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.

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

TypeScript type-only export specifiers cause esbuild to overwrite module.exports in esbuild >0.14.3 #1907

Closed
p0wl opened this issue Jan 5, 2022 · 2 comments

Comments

@p0wl
Copy link

p0wl commented Jan 5, 2022

Since esbuild 0.14.4 (0.14.5) a type-only export causes esbuild to apply the __toCommonJS() modifier to module.exports. this causes all other properties on module.exports to be overwritten.

I see that using export causes esbuild to apply the CJS transformation, but we were running into troubles because we use type only exports and those were removed without overwriting module.exports before v0.14.4.

Here is what happens with 0.14.3:

➜ npx esbuild --version; echo 'export type Area = 'ASSEMBLY' | 'BERLIN'; module.exports.bla = { a: 1 }' | npx esbuild --format=cjs --loader=ts
0.14.3
var __defProp = Object.defineProperty;
var __markAsModule = (target) => __defProp(target, "__esModule", { value: true });
__markAsModule(exports);
module.exports.bla = { a: 1 };

and since 0.14.5 the behavior changed:

0.14.5
var __defProp = Object.defineProperty;
...
var stdin_exports = {};
module.exports.bla = { a: 1 };
module.exports = __toCommonJS(stdin_exports);

I tried to spot the change in the diff, but I only found this: v0.14.3...v0.14.5#diff-7c136e0e2ecad99566cc25cf7268b0537dadfe387e46eea3b7d82886e8875d84R1539, which is checking for an import, not an export.

Thanks for your work 🙏, do you think this is a bug or is this another interop ambiguity?

@evanw
Copy link
Owner

evanw commented Jan 5, 2022

Mixed CommonJS and ESM exports in the same file is not something esbuild is designed for. I added a warning in #1887 but that only checks for people doing module.exports = { a: 1 } not module.exports.bla = { a: 1 }. I can extend the warning to cover this case too.

@p0wl
Copy link
Author

p0wl commented Jan 6, 2022

Awesome, thanks! We'll upgrade to only use ESM syntax, which wasn't possible before we used esbuild in our project 🎉

@p0wl p0wl closed this as completed Jan 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants