From 4eea0ce86f16ce83070f61fd0c2675c36e861003 Mon Sep 17 00:00:00 2001 From: Carmen Popoviciu Date: Mon, 3 Apr 2023 17:28:55 +0200 Subject: [PATCH] [wrangler]: Remove unused txt/html esbuild loaders, configured for bundling Pages Functions (#2900) Up until now, Pages handled `.txt` and `.html` file imports in Functions via esbuild's [loader](https://esbuild.github.io/api/#loader) option. With the landing of the Wasm module support in Pages Functions, `.txt` and `.html` file imports are now handled as external modules imports, and the existing esbuild `loader` configuration we have in place for such file types, is completely ignored. This commit cleans up the no longer used esbuild txt and html loaders, from both Functions and _worker.js bundling configurations. It also adds relevant tests and fixtures for these specific file type imports, that should have been part of our Wasm support PR, but unfortunately weren't. Co-authored-by: Carmen Popoviciu --- .../{wasm => external-modules}/add.wasm | Bin .../{wasm => external-modules}/add.wat | 0 .../external-modules/meaning-of-life.html | 5 + .../external-modules/meaning-of-life.txt | 1 + .../functions/meaning-of-life-html.js | 7 ++ .../functions/meaning-of-life-text.js | 7 ++ ...ing-of-life.js => meaning-of-life-wasm.js} | 4 +- .../tests/index.test.ts | 22 ++++- .../external-modules/meaning-of-life.html | 5 + .../pages-workerjs-wasm-app/public/_worker.js | 13 ++- .../tests/index.test.ts | 16 +++- .../src/__tests__/pages/publish.test.ts | 90 +++++++++++++----- .../src/pages/functions/buildWorker.ts | 8 -- 13 files changed, 135 insertions(+), 43 deletions(-) rename fixtures/pages-functions-wasm-app/{wasm => external-modules}/add.wasm (100%) rename fixtures/pages-functions-wasm-app/{wasm => external-modules}/add.wat (100%) create mode 100644 fixtures/pages-functions-wasm-app/external-modules/meaning-of-life.html create mode 100644 fixtures/pages-functions-wasm-app/external-modules/meaning-of-life.txt create mode 100644 fixtures/pages-functions-wasm-app/functions/meaning-of-life-html.js create mode 100644 fixtures/pages-functions-wasm-app/functions/meaning-of-life-text.js rename fixtures/pages-functions-wasm-app/functions/{meaning-of-life.js => meaning-of-life-wasm.js} (50%) create mode 100644 fixtures/pages-workerjs-wasm-app/external-modules/meaning-of-life.html diff --git a/fixtures/pages-functions-wasm-app/wasm/add.wasm b/fixtures/pages-functions-wasm-app/external-modules/add.wasm similarity index 100% rename from fixtures/pages-functions-wasm-app/wasm/add.wasm rename to fixtures/pages-functions-wasm-app/external-modules/add.wasm diff --git a/fixtures/pages-functions-wasm-app/wasm/add.wat b/fixtures/pages-functions-wasm-app/external-modules/add.wat similarity index 100% rename from fixtures/pages-functions-wasm-app/wasm/add.wat rename to fixtures/pages-functions-wasm-app/external-modules/add.wat diff --git a/fixtures/pages-functions-wasm-app/external-modules/meaning-of-life.html b/fixtures/pages-functions-wasm-app/external-modules/meaning-of-life.html new file mode 100644 index 000000000000..9ee3bcd390fe --- /dev/null +++ b/fixtures/pages-functions-wasm-app/external-modules/meaning-of-life.html @@ -0,0 +1,5 @@ + + + [.html]: The meaning of life is 21 + + diff --git a/fixtures/pages-functions-wasm-app/external-modules/meaning-of-life.txt b/fixtures/pages-functions-wasm-app/external-modules/meaning-of-life.txt new file mode 100644 index 000000000000..ee67156dbcb5 --- /dev/null +++ b/fixtures/pages-functions-wasm-app/external-modules/meaning-of-life.txt @@ -0,0 +1 @@ +[.txt]: The meaning of life is 21 \ No newline at end of file diff --git a/fixtures/pages-functions-wasm-app/functions/meaning-of-life-html.js b/fixtures/pages-functions-wasm-app/functions/meaning-of-life-html.js new file mode 100644 index 000000000000..08cdf637470d --- /dev/null +++ b/fixtures/pages-functions-wasm-app/functions/meaning-of-life-html.js @@ -0,0 +1,7 @@ +import html from "./../external-modules/meaning-of-life.html"; + +export async function onRequest() { + return new Response(html, { + headers: { "Content-Type": "text/html" }, + }); +} diff --git a/fixtures/pages-functions-wasm-app/functions/meaning-of-life-text.js b/fixtures/pages-functions-wasm-app/functions/meaning-of-life-text.js new file mode 100644 index 000000000000..9d9d7772018b --- /dev/null +++ b/fixtures/pages-functions-wasm-app/functions/meaning-of-life-text.js @@ -0,0 +1,7 @@ +import text from "./../external-modules/meaning-of-life.txt"; + +export async function onRequest() { + return new Response(text, { + headers: { "Content-Type": "text/plain" }, + }); +} diff --git a/fixtures/pages-functions-wasm-app/functions/meaning-of-life.js b/fixtures/pages-functions-wasm-app/functions/meaning-of-life-wasm.js similarity index 50% rename from fixtures/pages-functions-wasm-app/functions/meaning-of-life.js rename to fixtures/pages-functions-wasm-app/functions/meaning-of-life-wasm.js index 96ba38339d37..9a32cc6bc121 100644 --- a/fixtures/pages-functions-wasm-app/functions/meaning-of-life.js +++ b/fixtures/pages-functions-wasm-app/functions/meaning-of-life-wasm.js @@ -1,8 +1,8 @@ -import add from "../wasm/add.wasm"; +import add from "../external-modules/add.wasm"; export async function onRequest() { const addModule = await WebAssembly.instantiate(add); return new Response( - `Hello WASM World! The meaning of life is ${addModule.exports.add(20, 1)}` + `[.wasm]: The meaning of life is ${addModule.exports.add(20, 1)}` ); } diff --git a/fixtures/pages-functions-wasm-app/tests/index.test.ts b/fixtures/pages-functions-wasm-app/tests/index.test.ts index 363f26f6dd76..7f37d5fee36d 100644 --- a/fixtures/pages-functions-wasm-app/tests/index.test.ts +++ b/fixtures/pages-functions-wasm-app/tests/index.test.ts @@ -22,11 +22,27 @@ describe.concurrent("Pages Functions with wasm module imports", () => { expect(text).toContain("Hello from pages-functions-wasm-app!"); }); - it("should resolve any wasm module imports and render /meaning-of-life", async ({ + it("should resolve wasm module imports and render /meaning-of-life-wasm", async ({ expect, }) => { - const response = await fetch(`http://${ip}:${port}/meaning-of-life`); + const response = await fetch(`http://${ip}:${port}/meaning-of-life-wasm`); const text = await response.text(); - expect(text).toEqual("Hello WASM World! The meaning of life is 21"); + expect(text).toEqual("[.wasm]: The meaning of life is 21"); + }); + + it("should resolve text module imports and render /meaning-of-life-wasm-text", async ({ + expect, + }) => { + const response = await fetch(`http://${ip}:${port}/meaning-of-life-text`); + const text = await response.text(); + expect(text).toEqual("[.txt]: The meaning of life is 21"); + }); + + it("should resolve html module imports and render /meaning-of-life-wasm-html", async ({ + expect, + }) => { + const response = await fetch(`http://${ip}:${port}/meaning-of-life-html`); + const text = await response.text(); + expect(text).toContain(`[.html]: The meaning of life is 21`); }); }); diff --git a/fixtures/pages-workerjs-wasm-app/external-modules/meaning-of-life.html b/fixtures/pages-workerjs-wasm-app/external-modules/meaning-of-life.html new file mode 100644 index 000000000000..9ee3bcd390fe --- /dev/null +++ b/fixtures/pages-workerjs-wasm-app/external-modules/meaning-of-life.html @@ -0,0 +1,5 @@ + + + [.html]: The meaning of life is 21 + + diff --git a/fixtures/pages-workerjs-wasm-app/public/_worker.js b/fixtures/pages-workerjs-wasm-app/public/_worker.js index 4c0be0be4395..e783c02872f6 100644 --- a/fixtures/pages-workerjs-wasm-app/public/_worker.js +++ b/fixtures/pages-workerjs-wasm-app/public/_worker.js @@ -1,19 +1,28 @@ import multiply from "./../wasm/multiply.wasm"; +import html from "./../external-modules/meaning-of-life.html"; export default { async fetch(request, env) { const url = new URL(request.url); const multiplyModule = await WebAssembly.instantiate(multiply); - if (url.pathname === "/meaning-of-life") { + if (url.pathname === "/meaning-of-life-wasm") { return new Response( - `Hello _worker.js WASM World! The meaning of life is ${multiplyModule.exports.multiply( + `[.wasm]: The meaning of life is ${multiplyModule.exports.multiply( 7, 3 )}` ); } + if (url.pathname === "/meaning-of-life-html") { + return new Response(html, { + headers: { + "Content-Type": "text/html", + }, + }); + } + return env.ASSETS.fetch(request); }, }; diff --git a/fixtures/pages-workerjs-wasm-app/tests/index.test.ts b/fixtures/pages-workerjs-wasm-app/tests/index.test.ts index 60cca8961a25..7ab945d18a66 100644 --- a/fixtures/pages-workerjs-wasm-app/tests/index.test.ts +++ b/fixtures/pages-workerjs-wasm-app/tests/index.test.ts @@ -22,13 +22,19 @@ describe.concurrent("Pages Advanced Mode with wasm module imports", () => { expect(text).toContain("Hello from pages-workerjs-wasm-app!"); }); - it("should resolve any wasm module imports and render the correct response", async ({ + it("should resolve wasm module imports and correctly render /meaning-of-life", async ({ expect, }) => { - const response = await fetch(`http://${ip}:${port}/meaning-of-life`); + const response = await fetch(`http://${ip}:${port}/meaning-of-life-wasm`); const text = await response.text(); - expect(text).toEqual( - "Hello _worker.js WASM World! The meaning of life is 21" - ); + expect(text).toEqual("[.wasm]: The meaning of life is 21"); + }); + + it("should resolve text module imports and correctly render /meaning-of-life-html", async ({ + expect, + }) => { + const response = await fetch(`http://${ip}:${port}/meaning-of-life-html`); + const text = await response.text(); + expect(text).toContain(`[.html]: The meaning of life is 21`); }); }); diff --git a/packages/wrangler/src/__tests__/pages/publish.test.ts b/packages/wrangler/src/__tests__/pages/publish.test.ts index 852c6e75174d..af30d5e30162 100644 --- a/packages/wrangler/src/__tests__/pages/publish.test.ts +++ b/packages/wrangler/src/__tests__/pages/publish.test.ts @@ -2245,20 +2245,22 @@ and that at least one include rule is provided. mkdirSync("public"); writeFileSync("public/README.md", "This is a readme"); - // set up hello.wasm - mkdirSync("wasm"); - writeFileSync("wasm/hello.wasm", "HELLO WORLD"); + // set up some "external" modules + mkdirSync("external"); + writeFileSync("external/hello.wasm", "Hello Wasm modules world!"); + writeFileSync("external/hello.txt", "Hello Text modules world!"); // set up Functions mkdirSync("functions"); writeFileSync( "functions/hello.js", ` - import hello from "./../wasm/hello.wasm"; + import wasm from "./../external/hello.wasm"; + import text from "./../external/hello.txt" export async function onRequest() { - const helloModule = await WebAssembly.instantiate(hello); - const greeting = helloModule.exports.hello; - return new Response(greeting); + const helloModule = await WebAssembly.instantiate(wasm); + const wasmGreeting = helloModule.exports.hello; + return new Response(wasmGreeting + text); } ` ); @@ -2363,6 +2365,10 @@ and that at least one include rule is provided. /[0-9a-z]*-hello.wasm/g, "test-hello.wasm" ); + workerBundleWithConstantData = workerBundleWithConstantData.replace( + /[0-9a-z]*-hello.txt/g, + "test-hello.txt" + ); // check we appended the metadata expect(workerBundleWithConstantData).toContain( @@ -2377,18 +2383,29 @@ and that at least one include rule is provided. `Content-Disposition: form-data; name="functionsWorker-0.test.js"; filename="functionsWorker-0.test.js"` ); expect(workerBundleWithConstantData).toContain(` -import hello from "./test-hello.wasm"; +import wasm from "./test-hello.wasm"; +import text from "./test-hello.txt"; async function onRequest() { - const helloModule = await WebAssembly.instantiate(hello); - const greeting = helloModule.exports.hello; - return new Response(greeting); + const helloModule = await WebAssembly.instantiate(wasm); + const wasmGreeting = helloModule.exports.hello; + return new Response(wasmGreeting + text); }`); // check we appended the wasm module expect(workerBundleWithConstantData).toContain( `Content-Disposition: form-data; name="./test-hello.wasm"; filename="./test-hello.wasm"` ); - expect(workerBundleWithConstantData).toContain(`HELLO WORLD`); + expect(workerBundleWithConstantData).toContain( + `Hello Wasm modules world!` + ); + + // check we appended the text module + expect(workerBundleWithConstantData).toContain( + `Content-Disposition: form-data; name="./test-hello.txt"; filename="./test-hello.txt"` + ); + expect(workerBundleWithConstantData).toContain( + `Hello Text modules world!` + ); return res.once( ctx.status(200), @@ -2445,20 +2462,31 @@ async function onRequest() { writeFileSync("public/README.md", "This is a readme"); // set up hello.wasm - mkdirSync("wasm"); - writeFileSync("wasm/hello.wasm", "HELLO"); + mkdirSync("external"); + writeFileSync("external/hello.wasm", "Hello wasm modules"); + writeFileSync( + "external/hello.html", + "Hello text modules" + ); // set up _worker.js writeFileSync( "public/_worker.js", ` - import hello from "./../wasm/hello.wasm"; + import wasm from "./../external/hello.wasm"; + import html from "./../external/hello.html"; export default { async fetch(request, env) { const url = new URL(request.url); - const helloModule = await WebAssembly.instantiate(hello); - const greeting = helloModule.exports.hello; - return url.pathname.startsWith('/hello') ? new Response(greeting) : env.ASSETS.fetch(request); + const helloModule = await WebAssembly.instantiate(wasm); + const wasmGreeting = helloModule.exports.hello; + if(url.pathname.startsWith('/hello-wasm')) { + return new Response(wasmGreeting); + } + if(url.pathname.startsWith('/hello-text')) { + return new Response(html); + } + return env.ASSETS.fetch(request); } }; ` @@ -2558,6 +2586,10 @@ async function onRequest() { /[0-9a-z]*-hello.wasm/g, "test-hello.wasm" ); + workerBundleWithConstantData = workerBundleWithConstantData.replace( + /[0-9a-z]*-hello.html/g, + "test-hello.html" + ); // we care about a couple of things here, like the presence of `metadata`, // `bundledWorker`, the wasm import, etc., and since `workerBundle` is @@ -2572,13 +2604,20 @@ async function onRequest() { Content-Type: application/javascript+module // _worker.js - import hello from \\"./test-hello.wasm\\"; + import wasm from \\"./test-hello.wasm\\"; + import html from \\"./test-hello.html\\"; var worker_default = { async fetch(request, env) { const url = new URL(request.url); - const helloModule = await WebAssembly.instantiate(hello); - const greeting = helloModule.exports.hello; - return url.pathname.startsWith(\\"/hello\\") ? new Response(greeting) : env.ASSETS.fetch(request); + const helloModule = await WebAssembly.instantiate(wasm); + const wasmGreeting = helloModule.exports.hello; + if (url.pathname.startsWith(\\"/hello-wasm\\")) { + return new Response(wasmGreeting); + } + if (url.pathname.startsWith(\\"/hello-text\\")) { + return new Response(html); + } + return env.ASSETS.fetch(request); } }; export { @@ -2590,7 +2629,12 @@ async function onRequest() { Content-Disposition: form-data; name=\\"./test-hello.wasm\\"; filename=\\"./test-hello.wasm\\" Content-Type: application/wasm - HELLO + Hello wasm modules + ------formdata-undici-0.test + Content-Disposition: form-data; name=\\"./test-hello.html\\"; filename=\\"./test-hello.html\\" + Content-Type: text/plain + + Hello text modules ------formdata-undici-0.test--" `); diff --git a/packages/wrangler/src/pages/functions/buildWorker.ts b/packages/wrangler/src/pages/functions/buildWorker.ts index b6f3e59a6893..fb742862cb9e 100644 --- a/packages/wrangler/src/pages/functions/buildWorker.ts +++ b/packages/wrangler/src/pages/functions/buildWorker.ts @@ -59,10 +59,6 @@ export function buildWorker({ watch, legacyNodeCompat, nodejsCompat, - loader: { - ".txt": "text", - ".html": "text", - }, define: { __FALLBACK_SERVICE__: JSON.stringify(fallbackService), }, @@ -212,10 +208,6 @@ export function buildRawWorker({ watch, legacyNodeCompat, nodejsCompat, - loader: { - ".txt": "text", - ".html": "text", - }, define: {}, betaD1Shims: (betaD1Shims || []).map( (binding) => `${D1_BETA_PREFIX}${binding}`