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

Runtime support for __esModule annotations #3393

Merged
merged 3 commits into from
Jun 26, 2023
Merged

Conversation

Jarred-Sumner
Copy link
Collaborator

@Jarred-Sumner Jarred-Sumner commented Jun 24, 2023

This interprets the __esModule annotation at runtime:

  • If default and the __esModule property is defined on module.exports, then the default ESM export
    becomes the value of module.exports.default (like Babel), unless the enclosing package.json's "type" field is set to "module" (like esbuild)
  • If the __esModule annotation is not defined on module.exports, then we
    set the default ESM export to module.exports or if "package.json"'s "type"field is set to"module"` (like Node.js)

Note that this interpretation is slightly different than esbuild and other tooling:

  • We do not disable this based on the caller's file extension. In a JS runtime, there is only one ModuleNamespaceObject per ES module
  • We ignore the value of the __esModule annotation. We only look for the existence of the __esModule property on module.exports. This is for performance reasons (bun loops through the module.exports objects at runtime).

Relevant links:

Fixes #3383

cc @andrewbranch @colinhacks @paperdave

@github-actions
Copy link
Contributor

github-actions bot commented Jun 24, 2023

@Jarred-Sumner 9 files with test failures on bun-darwin-aarch64:

  • test/cli/install/bun-remove.test.ts
  • test/cli/install/bunx.test.ts
  • test/js/bun/net/socket.test.ts
  • test/js/bun/test/bun-test.test.ts
  • test/js/bun/test/test-test.test.ts
  • test/js/bun/websocket/websocket-server.test.ts
  • test/js/node/fs/fs.test.ts
  • test/js/web/fetch/fetch-leak.test.js
  • test/js/web/websocket/websocket.test.js

View test output

#2fcd657c552d012469af451ccfba9cd4e9424339

@github-actions
Copy link
Contributor

github-actions bot commented Jun 24, 2023

@Jarred-Sumner 6 files with test failures on linux-x64:

  • test/cli/install/bunx.test.ts
  • test/js/bun/websocket/websocket-server.test.ts
  • test/js/node/fs/fs.test.ts
  • test/js/third_party/prisma/prisma.test.ts
  • test/js/web/fetch/fetch-leak.test.js
  • test/js/web/websocket/websocket.test.js

View test output

#2fcd657c552d012469af451ccfba9cd4e9424339

@github-actions
Copy link
Contributor

github-actions bot commented Jun 24, 2023

@Jarred-Sumner 6 files with test failures on linux-x64-baseline:

  • test/cli/install/bunx.test.ts
  • test/js/bun/websocket/websocket-server.test.ts
  • test/js/node/fs/fs.test.ts
  • test/js/third_party/prisma/prisma.test.ts
  • test/js/web/fetch/fetch-leak.test.js
  • test/js/web/websocket/websocket.test.js

View test output

#2fcd657c552d012469af451ccfba9cd4e9424339

@github-actions
Copy link
Contributor

github-actions bot commented Jun 24, 2023

@Jarred-Sumner 8 files with test failures on bun-darwin-x64-baseline:

  • test/cli/install/bunx.test.ts
  • test/js/bun/spawn/spawn-streaming-stdin.test.ts
  • test/js/bun/sqlite/sqlite.test.js
  • test/js/bun/util/sleepSync.test.ts
  • test/js/bun/websocket/websocket-server.test.ts
  • test/js/node/fs/fs.test.ts
  • test/js/web/timers/setTimeout.test.js
  • test/js/web/websocket/websocket.test.js

View test output

#2fcd657c552d012469af451ccfba9cd4e9424339

@guybedford
Copy link

Note that "other tooling" here also includes Node.js itself. If you really thing the ecosystem should move in this direction, can we at least propose a new flag to Node.js --es-module-interop or something to not just diverge by default? Otherwise Node.js and Bun will always be incompatible?

@guybedford
Copy link

Or at least figure out how to distinguish Node.js usage from browser style interop?

@Jarred-Sumner
Copy link
Collaborator Author

Note that "other tooling" here also includes Node.js itself. If you really thing the ecosystem should move in this direction, can we at least propose a new flag to Node.js --es-module-interop or something to not just diverge by default? Otherwise Node.js and Bun will always be incompatible?

It doesn’t diverge by default, Node.js-like is what this keeps unless the CommonJS module has __esModule exported. The other difference is __esModule is excluded from the ESM namespace export (we could skip doing that though)

Where I feel the least good about this PR is not paying attention to .mjs or ”type”: “module” as opt-outs.

I think supporting __esModule overall will make more packages work the way people expect.

@Jarred-Sumner
Copy link
Collaborator Author

I have updated it to respect "type": "module".

@guybedford
Copy link

Completely understood supporting current bundler behaviours is needed, this sounds like it might be the right kind of compromise as Webpack worked out.

@Jarred-Sumner Jarred-Sumner merged commit a732999 into main Jun 26, 2023
@Jarred-Sumner Jarred-Sumner deleted the jarred/__esModule branch June 26, 2023 19:49
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.

pathlib-js fails with "Object is not a constructor"
2 participants