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

module exports inside circular dependency #758

Closed
mdoi2 opened this issue Feb 6, 2021 · 2 comments
Closed

module exports inside circular dependency #758

mdoi2 opened this issue Feb 6, 2021 · 2 comments

Comments

@mdoi2
Copy link

mdoi2 commented Feb 6, 2021

I found that when I build certain TypeScript sources using esbuild, I get an error caused by module exports inside circular dependency. This error does not occur when building with tsc.

I discovered this when using TypeORM.
A sample of the code is presented here.
https://github.com/m-doi2/esbuild-circular-dependency-error-sample

In summary, it seems to be caused by the result of code conversion when exporting multiple times.
It is possible to work around this by modifying the way the TypeScript code is written, so I don't feel it is a serious problem at this point.

However, during some experiments, I confirmed some results that were somewhat redundant and disturbing compared to tsc, for example, the __exportStar method was generated even when star was not used, and the code to execute multiple __exportStar methods was generated.

I'm sorry I can't offer a complete solution, but I hope this report helps.

@evanw
Copy link
Owner

evanw commented Feb 8, 2021

Thanks for the test case. It looks like the difference that makes TypeScript work is that TypeScript first sets exports.__esModule = true and then calls require() before passing it to __exportStar() while esbuild calls require() and passes it to __exportStar() which then sets exports.__esModule = true (so esbuild does the reverse order of TypeScript). This ends up making a difference if the module that is in the process of evaluating a cross-compiled export * as re-export statement is re-imported in a cycle.

@evanw evanw closed this as completed in 81a1117 Feb 8, 2021
@evanw
Copy link
Owner

evanw commented Mar 4, 2021

Heads up that I'm probably going to have to break this due to how node has decided to implement default imports from CommonJS modules. TypeScript adopts the __esModule convention from Babel but for whatever reason node decided to not adopt this convention. So for node compatibility, esbuild will need to no longer respect the __esModule convention exactly. Specifically require() of a module that sets __esModule will now return a shim object instead of the actual exports object so that esbuild can emulate node's default import behavior. That means that in circular import scenarios like this one, new exports added to a module that is already in the middle of being imported may no longer be visible to the first importer of that module (the export * as in this case).

I think I am ok with this breakage because esbuild is primarily a bundler, compatibility with node's ESM behavior is important, and this is a pretty narrow edge case. This circular import scenario should still work when the code is bundled, but it will no longer work when each file is converted from ESM to CommonJS independently. There is more discussion about the default import issue in #532 and #909.

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 a pull request may close this issue.

2 participants