Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: ensure that bundle is generated to es2020 target #815

Merged
merged 1 commit into from
Apr 17, 2022
Merged

fix: ensure that bundle is generated to es2020 target #815

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/perfect-tomatoes-invent.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"wrangler": patch
---

fix: ensure that bundle is generated to es2020 target

The default tsconfig generated by tsc uses `target: "es5"`, which we don't support. This fix ensures that we output es2020 modules, even if tsconfig asks otherwise.
157 changes: 112 additions & 45 deletions packages/wrangler/src/__tests__/publish.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3720,56 +3720,123 @@ export default{
`"Deprecation warning: detected a legacy module import in \\"./index.js\\". This will stop working in the future. Replace references to \\"text.file\\" with \\"./text.file\\";"`
);
});
});

it("should work with legacy module specifiers, with a deprecation warning (2)", async () => {
writeWranglerToml();
fs.writeFileSync(
"./index.js",
`import WASM from 'index.wasm'; export default {};`
);
fs.writeFileSync("./index.wasm", "SOME WASM CONTENT");
mockSubDomainRequest();
mockUploadWorkerRequest({
expectedModules: {
"./94b240d0d692281e6467aa42043986e5c7eea034-index.wasm":
"SOME WASM CONTENT",
},
it("should work with legacy module specifiers, with a deprecation warning (2)", async () => {
writeWranglerToml();
fs.writeFileSync(
"./index.js",
`import WASM from 'index.wasm'; export default {};`
);
fs.writeFileSync("./index.wasm", "SOME WASM CONTENT");
mockSubDomainRequest();
mockUploadWorkerRequest({
expectedModules: {
"./94b240d0d692281e6467aa42043986e5c7eea034-index.wasm":
"SOME WASM CONTENT",
},
});
await runWrangler("publish index.js");
expect(std.out).toMatchInlineSnapshot(`
"Uploaded test-name (TIMINGS)
Published test-name (TIMINGS)
test-name.test-sub-domain.workers.dev"
`);
expect(std.err).toMatchInlineSnapshot(`""`);
expect(std.warn).toMatchInlineSnapshot(
`"Deprecation warning: detected a legacy module import in \\"./index.js\\". This will stop working in the future. Replace references to \\"index.wasm\\" with \\"./index.wasm\\";"`
);
});
await runWrangler("publish index.js");
expect(std.out).toMatchInlineSnapshot(`
"Uploaded test-name (TIMINGS)
Published test-name (TIMINGS)
test-name.test-sub-domain.workers.dev"
`);
expect(std.err).toMatchInlineSnapshot(`""`);
expect(std.warn).toMatchInlineSnapshot(
`"Deprecation warning: detected a legacy module import in \\"./index.js\\". This will stop working in the future. Replace references to \\"index.wasm\\" with \\"./index.wasm\\";"`
);
});

it("should not match regular module specifiers when there aren't any possible legacy module matches", async () => {
// see https://github.com/cloudflare/wrangler2/issues/655 for bug details
it("should not match regular module specifiers when there aren't any possible legacy module matches", async () => {
// see https://github.com/cloudflare/wrangler2/issues/655 for bug details

fs.writeFileSync(
"./index.js",
`import inner from './inner/index.js'; export default {};`
);
fs.mkdirSync("./inner", { recursive: true });
fs.writeFileSync("./inner/index.js", `export default 123`);
mockSubDomainRequest();
mockUploadWorkerRequest();
fs.writeFileSync(
"./index.js",
`import inner from './inner/index.js'; export default {};`
);
fs.mkdirSync("./inner", { recursive: true });
fs.writeFileSync("./inner/index.js", `export default 123`);
mockSubDomainRequest();
mockUploadWorkerRequest();

await runWrangler(
"publish index.js --compatibility-date 2022-03-17 --name test-name"
);
expect(std.out).toMatchInlineSnapshot(`
"Uploaded test-name (TIMINGS)
Published test-name (TIMINGS)
test-name.test-sub-domain.workers.dev"
`);
expect(std.err).toMatchInlineSnapshot(`""`);
expect(std.warn).toMatchInlineSnapshot(`""`);
await runWrangler(
"publish index.js --compatibility-date 2022-03-17 --name test-name"
);
expect(std.out).toMatchInlineSnapshot(`
"Uploaded test-name (TIMINGS)
Published test-name (TIMINGS)
test-name.test-sub-domain.workers.dev"
`);
expect(std.err).toMatchInlineSnapshot(`""`);
expect(std.warn).toMatchInlineSnapshot(`""`);
});
});

describe("tsconfig", () => {
it("should use compilerOptions.paths to resolve modules", async () => {
writeWranglerToml({
main: "index.ts",
});
fs.writeFileSync(
"index.ts",
`import { foo } from '~lib/foo'; export default { fetch() { return new Response(foo)} }`
);
fs.mkdirSync("lib", { recursive: true });
fs.writeFileSync("lib/foo.ts", `export const foo = 123;`);
fs.writeFileSync(
"tsconfig.json",
JSON.stringify({
compilerOptions: {
baseUrl: ".",
paths: {
"~lib/*": ["lib/*"],
},
},
})
);
mockSubDomainRequest();
mockUploadWorkerRequest({
expectedEntry: "var foo = 123;", // make sure it imported the module correctly
});
await runWrangler("publish index.ts");
expect(std).toMatchInlineSnapshot(`
Object {
"err": "",
"out": "Uploaded test-name (TIMINGS)
Published test-name (TIMINGS)
test-name.test-sub-domain.workers.dev",
"warn": "",
}
`);
});

it("should output to target es2020 even if tsconfig says otherwise", async () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we display a warning in this case?

It could be confusing to developers if that have specified ES5 but it keeps outputting ES2020... They might think that we are not reading tsconfig.json at all?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They never see the output so it maaay be ok.

writeWranglerToml();
writeWorkerSource();
fs.writeFileSync(
"tsconfig.json",
JSON.stringify({
compilerOptions: {
target: "es5",
module: "commonjs",
},
})
);
mockSubDomainRequest();
mockUploadWorkerRequest({
expectedEntry: "export {", // just check that the export is preserved
});
await runWrangler("publish index.js"); // this would throw if we tried to compile with es5
expect(std).toMatchInlineSnapshot(`
Object {
"err": "",
"out": "Uploaded test-name (TIMINGS)
Published test-name (TIMINGS)
test-name.test-sub-domain.workers.dev",
"warn": "",
}
`);
});
});
});

Expand Down
1 change: 1 addition & 0 deletions packages/wrangler/src/bundle.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ export async function bundleWorker(
outdir: destination,
external: ["__STATIC_CONTENT_MANIFEST"],
format: "esm",
target: "es2020",
sourcemap: true,
metafile: true,
conditions: ["worker", "browser"],
Expand Down
1 change: 1 addition & 0 deletions packages/wrangler/src/entry.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ export default async function guessWorkerFormat(
metafile: true,
bundle: false,
format: "esm",
target: "es2020",
write: false,
loader: {
".js": "jsx",
Expand Down