Repository navigation
js_parser: do not bind module or exports in a file whose module type is ESM #46037
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
base: main
Are you sure you want to change the base?
Changes from all commits
d9ddc30
7aefcec
bb79754
155c3d8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3344,10 +3344,22 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O | |
| .generated | ||
| .ensure_unused_capacity(generated_symbols_count as usize * 3); | ||
|
|
||
| self.exports_ref = | ||
| self.declare_common_js_symbol(js_ast::symbol::Kind::Hoisted, b"exports")?; | ||
| self.module_ref = | ||
| self.declare_common_js_symbol(js_ast::symbol::Kind::Hoisted, b"module")?; | ||
| if self.is_file_considered_to_have_esm_exports { | ||
| // An ES module has no `exports` or `module` binding: a source | ||
| // reference to either name is an unbound global, as in Node. The | ||
| // symbols still back generated code (the ESM namespace object, | ||
| // `export =`, the HMR wrapper), so they live on `generated` only. | ||
| self.exports_ref = self.new_symbol(js_ast::symbol::Kind::Hoisted, b"exports"); | ||
| self.module_ref = self.new_symbol(js_ast::symbol::Kind::Hoisted, b"module"); | ||
| let (exports_ref, module_ref) = (self.exports_ref, self.module_ref); | ||
| VecExt::append(&mut self.module_scope_mut().generated, exports_ref); | ||
| VecExt::append(&mut self.module_scope_mut().generated, module_ref); | ||
| } else { | ||
| self.exports_ref = | ||
| self.declare_common_js_symbol(js_ast::symbol::Kind::Hoisted, b"exports")?; | ||
| self.module_ref = | ||
| self.declare_common_js_symbol(js_ast::symbol::Kind::Hoisted, b"module")?; | ||
| } | ||
|
|
||
| self.require_ref = | ||
| self.declare_common_js_symbol(js_ast::symbol::Kind::Unbound, b"require")?; | ||
|
|
@@ -5976,7 +5988,9 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O | |
| if self.options.repl_mode { | ||
| return None; | ||
| } | ||
| if self.has_es_module_syntax && self.commonjs_named_exports.count() == 0 { | ||
| if (self.has_es_module_syntax || self.is_file_considered_to_have_esm_exports) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 (optional) A Why this was flaggedTrigger: a Verification: Triggered when a |
||
| && self.commonjs_named_exports.count() == 0 | ||
| { | ||
| // In an ES6 module, "this" is supposed to be undefined. Instead of | ||
| // doing this at runtime using "fn.call(undefined)", we do it at | ||
| // compile time using expression substitution here. | ||
|
|
@@ -6031,6 +6045,18 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O | |
| } | ||
| } | ||
|
|
||
| /// `require.main === module` becomes `import.meta.main`, so the `module` | ||
| /// operand (the CommonJS binding, or the unbound global in an ES module) | ||
| /// is no longer used. | ||
| pub(crate) fn ignore_usage_of_require_main_module( | ||
| &mut self, | ||
| module_operand: &js_ast::ExprData, | ||
| ) { | ||
| if let js_ast::ExprData::EIdentifier(id) = module_operand { | ||
| self.ignore_usage(id.ref_); | ||
| } | ||
| } | ||
|
|
||
| pub(crate) fn ignore_usage_of_runtime_require(&mut self) { | ||
| if self.options.features.auto_polyfill_require { | ||
| debug_assert!(!self.runtime_imports.__require.is_empty()); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 (optional) Users running a
.js/.tsfile under"type": "module"that usesmodule.exportsnow get a bareReferenceError: module is not defined; the base ran the file as CommonJS. src/js_parser/p.rs:3347 leavesmodule/exportsunbound when the hint is ESM but logs nothing, so nothing says which package.json or extension made the file ESM. Node appends "in ES module scope" plus a note naming the package.json and suggesting.cjs; esbuild warns at build time. Fix: whenoptions.module_type == Esmand the module scope resolvesmoduleorexportsas Unbound, emit a note with the reason and the.cjsremedy on every path (runtime,bun build, plugin), or decorate the runtime ReferenceError as Node does.Why this was flagged
A project created by
bun initgets"type": "module"(src/runtime/cli/init_command.rs:664). It hasutil.jscontainingmodule.exports = { f }and an entry thatrequires it. On the base branchmodulewas declared bydeclare_common_js_symbol,uses_module_refat src/js_parser/parse/parse_entry.rs:1875 madeexports_kindCjs and the file ran under the CommonJS wrapper. After the changemodule_type_hintreturns Esm for.jsunder that package.json (src/runtime/jsc_hooks.rs:3928-3934), src/js_parser/p.rs:3347-3356 createsmoduleonly as a generated symbol, the source reference becomes Unbound, parse_entry.rs:1900 picks Esm, and evaluation throwsReferenceError: module is not definedwith no note. Node for the same file printsReferenceError: module is not defined in ES module scopefollowed by a line saying the file is treated as an ES module because of the.jsextension and the package.json"type": "module", and to rename it to.cjs. Bun emits nothing on any path: no parser note is added where the symbol is left unbound, and no runtime decoration exists.Verification: Triggered whenever a
.js/.tsfile under a package.json with"type": "module"assignsmodule.exports. src/js_parser/p.rs:3347-3356 createsmodule_refvianew_symboland appends it togeneratedonly, so a sourcemoduleresolves to an unbound global and JSC throws its generic ReferenceError. On the base, parse_entry.rs:1864-1877 ran the file as CommonJS. No Bun-side augmentation exists.