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

Move module.exports assigment to the top #2059

Merged
merged 10 commits into from
Mar 14, 2022

Conversation

indutny
Copy link
Contributor

@indutny indutny commented Feb 26, 2022

When using cjs format make sure to move module.exports to the top of
the generated chunk, right after the __export() call:

var exports = {};
__export(exports, {
  // ...
});
module.exports = __toCommonJS(exports);

This guarantees that all circular requires following these calls would
see the correct module.exports object.

Additionally, add an extra argument for __reExport to copy the data to
the module.exports:

__reExport(exports, module.exports, ...)

This is required to support export * from '..' since now
module.exports wouldn't automatically receive properties otherwise.

Fix: #1894

When using cjs format make sure to move `module.exports` to the top of
the generated chunk, right after the `__export()` call:

    var exports = {};
    __export(exports, {
      // ...
    });
    module.exports = __toCommonJS(exports);

This guarantees that all circular requires following these calls would
see the correct `module.exports` object.

Additionally, add an extra argument for `__reExport` to copy the data to
the `module.exports`:

    __reExport(exports, module.exports, ...)

This is required to support `export * from '..'` since now
`module.exports` wouldn't automatically receive properties otherwise.

Fix: evanw#1894
@indutny
Copy link
Contributor Author

indutny commented Feb 26, 2022

cc @evanw

@indutny-signal
Copy link

Tested on our codebase with both test suite and application tests, in addition to the existing tests in the esbuild's test suite.

@indutny-signal
Copy link

@evanw being a maintainer of a large project like esbuild is very stressful and I totally understand if you don't have time or desire to look into things. Let me know, however, if there is anything I could do to make it easier for you to review this and other PRs. Maybe I need to add test cases, write better description of the changes? Any feedback is welcome, and thanks for building and support this project!

@evanw
Copy link
Owner

evanw commented Mar 14, 2022

My personal life is really busy right now and I currently have extremely limited time to work on esbuild. I've got a ton of stuff going on all at once. Maintaining esbuild is one of the least stressful things about my life right now haha. I'm hoping to have more free time in a month or so but right now esbuild-related stuff is just going to go slowly.

@indutny
Copy link
Contributor Author

indutny commented Mar 14, 2022 via email

@evanw evanw merged commit a599d45 into evanw:master Mar 14, 2022
@indutny-signal
Copy link

Thank you! 🤗

zhusjfaker pushed a commit to speedy-js/esbuild that referenced this pull request Mar 28, 2022
hardfist pushed a commit to speedy-js/esbuild that referenced this pull request Mar 28, 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

Successfully merging this pull request may close these issues.

Import issue with circular re-exports
3 participants