Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions src/js_printer/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1216,6 +1216,17 @@
use js_ast::ExprData;
match &expr.data {
ExprData::EIdentifier(_) | ExprData::EDot(_) | ExprData::EIndex(_) => true,
Comment thread
robobun marked this conversation as resolved.
// Visit-pass rewrites that print as an identifier or property access.
ExprData::EImportIdentifier(_)
| ExprData::ECommonjsExportIdentifier(_)
| ExprData::ESpecial(_)
| ExprData::ERequireCallTarget
| ExprData::ERequireResolveCallTarget
| ExprData::ERequireMain
| ExprData::EImportMeta(_)
| ExprData::EImportMetaMain(_)

Check notice on line 1227 in src/js_printer/lib.rs

View check run for this annotation

Claude / Claude Code Review

Pre-existing: EImportMetaMain node-target branch ignores precedence level

Pre-existing (not introduced or widened here, just documented in the new test comment at bundler_edgecase.test.ts:2947-2949): the `EImportMetaMain` else-branch at src/js_printer/lib.rs:2883-2916 emits `__require.main == __require.module` as raw text without consulting `level`, so under `target: node`, `typeof import.meta.main` bundles to `typeof __require.main == __require.module` → parses as `(typeof __require.main) == __require.module` → `false` instead of `"boolean"`; same mis-association for
Comment thread
robobun marked this conversation as resolved.
| ExprData::EUndefined(_) => true,
ExprData::EInlinedEnum(e) => is_identifier_or_numeric_constant_or_property_access(&e.value),

Check notice on line 1229 in src/js_printer/lib.rs

View check run for this annotation

Claude / Claude Code Review

Pre-existing: print-time cross-module enum inlining emits 'delete NaN' on flag-set path

Pre-existing (not introduced or reachable via this PR's helper): the print-time cross-module enum inlining at `lib.rs:3274-3280` / `:3324-3331` produces the same `delete NaN` output on the flag-**set** path. `import { E } from './other'; delete E.N;` with `export enum E { N = 0/0 }` parses as `delete <EDot>`, so `WAS_ORIGINALLY_DELETE_OF_IDENTIFIER_OR_PROPERTY_ACCESS` is set and the guard at :4015 skips `is_identifier_or_numeric_constant_or_property_access`; the `EDot` print arm then uncondition
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Comment thread
robobun marked this conversation as resolved.
Comment thread
robobun marked this conversation as resolved.
ExprData::ENumber(e) => e.value().is_infinite() || e.value().is_nan(),
Comment thread
robobun marked this conversation as resolved.
_ => false,
}
Expand Down
49 changes: 49 additions & 0 deletions test/bundler/bundler_cjs2esm.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,55 @@ describe("bundler", () => {
stdout: '[[{"xyz":456},456],[{"xyz":123},123],[{"xyz":456},456],[{"xyz":123},123]]',
},
});
// `delete (null ?? exports.a)` evaluates to a value, so the result is `true`
// with no effect on the property. Under cjs2esm, `exports.a` is rewritten to
// an `ECommonjsExportIdentifier`; the printer has to re-wrap it as `(0, ...)`
// so `delete` still sees a value instead of the hoisted binding.
itBundled("cjs2esm/DeleteFoldedExportsPropertyRef", {
files: {
"/entry.js": /* js */ `
exports.a = 1;
console.log(delete (null ?? exports.a), exports.a);
console.log(delete (0, exports.a), exports.a);
`,
},
onAfterBundle: api => {
const code = api.readFile("out.js");
expect(code).not.toMatch(/^[^"]*delete\s+\$a\b/m);
},
run: { stdout: "true 1\ntrue 1" },
});
itBundled("cjs2esm/DeleteFoldedExportsPropertyRefConsumer", {
files: {
"/entry.js": /* js */ `
import { a } from "./lib.js";
console.log(a);
`,
"/lib.js": /* js */ `
exports.a = 1;
console.log(delete (null ?? exports.a), exports.a);
`,
},
run: { stdout: "true 1\n1" },
});
// `module.exports` under cjs2esm rewrites to ESpecial::ModuleExports, which
// prints as the `exports_*` namespace symbol. Output-shape check only: a bare
// `module.exports` value-read under cjs2esm currently emits a reference that
// has no declaration (a separate pre-existing issue), so the bundle cannot be
// executed here.
itBundled("cjs2esm/DeleteFoldedModuleExportsRef", {
files: {
"/entry.js": /* js */ `
exports.a = 1;
globalThis.r = delete (null ?? module.exports);
`,
},
onAfterBundle: api => {
const code = api.readFile("out.js");
expect(code).toMatch(/delete\s+\(0,\s*exports_\w+\)/);
expect(code).not.toMatch(/delete\s+exports_\w+\b/);
},
});
// https://github.com/oven-sh/bun/issues/4565
// `exports.x = ...` as the unbraced body of if/while/do/else must not be
// converted to `var $x = ...; export { $x as x };` because `export` is only
Expand Down
129 changes: 129 additions & 0 deletions test/bundler/bundler_edgecase.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2858,6 +2858,135 @@ describe("bundler", () => {
expect(out).not.toContain("require_foo\u2014bar");
},
});
// `delete (null ?? ns.x)` evaluates its operand to a value, so the result is
// `true` with no side effect. When bundling rewrites `ns.x` to the local
// binding (EImportIdentifier) after folding `??`, the printer must re-wrap
// the operand so `delete` still sees a value instead of the binding itself.
// Without the wrap the output is `delete x`, a strict-mode SyntaxError.
itBundled("edgecase/DeleteFoldedNamespacePropertyRef", {
files: {
"/entry.js": /* js */ `
import * as ns from "./m.js";
console.log(delete (null ?? ns.x), ns.x);
console.log(delete (0, ns.x), ns.x);
console.log(delete (true ? ns.x : 0), ns.x);
`,
"/m.js": /* js */ `
export let x = 1;
`,
},
onAfterBundle: api => {
const code = api.readFile("out.js");
expect(code).not.toMatch(/delete\s+x\b/);
expect(code).not.toMatch(/delete\s+ns\.x\b/);
},
run: { stdout: "true 1\ntrue 1\ntrue 1" },
});
itBundled("edgecase/DeleteFoldedNamespacePropertyRefMinify", {
files: {
"/entry.js": /* js */ `
import * as ns from "./m.js";
console.log(delete (null ?? ns.x), ns.x);
`,
"/m.js": /* js */ `
export let x = 1;
`,
},
minifySyntax: true,
minifyWhitespace: true,
run: { stdout: "true 1" },
});
// Same path via a direct named import: the identifier becomes an
// EImportIdentifier during the visit pass.
itBundled("edgecase/DeleteFoldedImportedBindingRef", {
files: {
"/entry.js": /* js */ `
import { x } from "./m.js";
console.log(delete (null ?? x), x);
`,
"/m.js": /* js */ `
export let x = 1;
`,
},
onAfterBundle: api => {
expect(api.readFile("out.js")).not.toMatch(/delete\s+x\b/);
},
run: { stdout: "true 1" },
});
// The bundler rewrites bare `require`/`require.main`/`require.resolve` to an
// ERequireCallTarget / ERequireMain / ERequireResolveCallTarget that prints
// as `__require` / `__require.main` / `__require.resolve`.
itBundled("edgecase/DeleteFoldedRequireRefs", {
files: {
"/entry.js": /* js */ `
console.log(delete (null ?? require));
console.log(delete (null ?? require.main));
console.log(delete (null ?? require.resolve));
`,
},
onAfterBundle: api => {
const code = api.readFile("out.js");
expect(code).not.toMatch(/delete\s+__require\b/);
expect(code).not.toMatch(/delete\s+require\b/);
},
run: { stdout: "true\ntrue\ntrue" },
});
// The visit pass substitutes unbound `undefined` to EUndefined, which
// `print_undefined` emits as the bare identifier when not minifying.
itBundled("edgecase/DeleteFoldedUndefinedRef", {
files: {
"/entry.js": /* js */ `
console.log(delete (null ?? undefined));
`,
},
onAfterBundle: api => {
expect(api.readFile("out.js")).not.toMatch(/delete\s+undefined\b/);
},
run: { stdout: "true" },
});
// `import.meta.main` is rewritten to EImportMetaMain; under `target: node`
// that prints as `__require.main == __require.module` without its own paren
// wrap, so an unwrapped `delete` would bind to `__require.main`.
itBundled("edgecase/DeleteFoldedImportMetaMainRef", {
files: {
"/entry.js": /* js */ `
console.log(delete (null ?? import.meta.main));
`,
},
onAfterBundle: api => {
expect(api.readFile("out.js")).not.toMatch(/delete\s+import\.meta\.main\b/);
},
run: { stdout: "true" },
});
itBundled("edgecase/DeleteFoldedImportMetaMainRefNode", {
files: {
"/entry.js": /* js */ `
console.log(delete (null ?? import.meta.main));
`,
},
target: "node",
onAfterBundle: api => {
expect(api.readFile("out.js")).not.toMatch(/delete\s+__require\.main\b/);
},
run: { runtime: "node", stdout: "true" },
});
// A same-file `const enum` member is inlined to an EInlinedEnum wrapping an
// ENumber during the visit pass, so the NaN/Infinity check has to look
// through the wrapper.
itBundled("edgecase/DeleteFoldedInlinedConstEnumNaN", {
files: {
"/entry.ts": /* ts */ `
const enum E { N = 0/0, I = 1/0, V = 1 }
console.log(delete (null ?? E.N), delete (null ?? E.I), delete (null ?? E.V));
`,
},
onAfterBundle: api => {
const code = api.readFile("out.js");
expect(code).not.toMatch(/delete\s+NaN\b/);
expect(code).not.toMatch(/delete\s+Infinity\b/);
},
run: { stdout: "true true true" },
});
});

for (const backend of ["api", "cli"] as const) {
Expand Down
Loading