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

"Annotate" exported object to fix named / namespace imports of our API in Node ESM #57133

Merged
merged 20 commits into from
Mar 4, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Fix again, exports is entirely reassigned, not augmented
  • Loading branch information
jakebailey committed Feb 29, 2024
commit 66ee8fe77cec85d2bb359284721f670f3d5e4bbe
4 changes: 2 additions & 2 deletions Herebyfile.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -209,10 +209,10 @@ function createBundler(entrypoint, outfile, taskOptions = {}) {
};

if (taskOptions.exportIsTsObject) {
// Monaco bundles us as ESM by wrapping our code with something that defines `module.exports`
// Monaco bundles us as ESM by wrapping our code with something that defines module.exports
// but then does not use it, instead using the `ts` variable. Ensure that if we think we're CJS
// that we still set `ts` to the module.exports object.
options.footer = { js: `})(typeof module !== "undefined" && module.exports ? (ts = module.exports, module) : { exports: ts });` };
options.footer = { js: `})(typeof module !== "undefined" && module.exports ? module : { exports: ts });\nif (typeof module !== "undefined" && module.exports) { ts = module.exports; }` };

// esbuild converts calls to "require" to "__require"; this function
// calls the real require if it exists, or throws if it does not (rather than
Expand Down
3 changes: 1 addition & 2 deletions src/compiler/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2749,6 +2749,5 @@ export function isNodeLikeSystem(): boolean {
// and definitely exist.
return typeof process !== "undefined"
&& !!process.nextTick
&& !(process as any).browser
&& typeof module === "object";
Copy link
Member Author

Choose a reason for hiding this comment

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

In the new version, module is a parameter and is always provided. This check would no longer do anything. I'm not sure that it really matters that we don't have this.

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually, I'm going to switch this back to the same definition it used to be, e.g. checking for require. The comment I left above about require stopped being correct at some point as I refined our output bundle format to rewrite require temporarily.

Copy link
Member Author

Choose a reason for hiding this comment

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

(I don't believe this will end up mattering in practice, but that's why we merge these sorts of things early in the cycle...)

&& !(process as any).browser;
}
Loading