Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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
14 changes: 14 additions & 0 deletions src/js_printer/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1216,6 +1216,20 @@ fn is_identifier_or_numeric_constant_or_property_access(expr: &js_ast::Expr) ->
use js_ast::ExprData;
match &expr.data {
ExprData::EIdentifier(_) | ExprData::EDot(_) | ExprData::EIndex(_) => true,
Comment thread
robobun marked this conversation as resolved.
// These are produced by the visit pass (namespace-import rewrite, cjs2esm,
// `module.exports`/`import.meta.hot`/`require` inlining) and in at least
// one output mode print as a bare identifier or a property access. Without
// the `(0, ...)` wrap the printed `delete` sees a Reference and its
// result/effect differ from the source. Over-wrapping the variants that can
// also print as a value literal is harmless.
Comment thread
robobun marked this conversation as resolved.
Outdated
ExprData::EImportIdentifier(_)
| ExprData::ECommonjsExportIdentifier(_)
| ExprData::ESpecial(_)
| ExprData::ERequireCallTarget
| ExprData::ERequireResolveCallTarget
| ExprData::ERequireMain
| ExprData::EImportMeta(_) => true,
ExprData::EInlinedEnum(e) => is_identifier_or_numeric_constant_or_property_access(&e.value),
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
90 changes: 90 additions & 0 deletions test/bundler/bundler_edgecase.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2858,6 +2858,96 @@ 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" },
});
// 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