Repository navigation
Conversation
`delete a.b` printed `delete undefined` when a define, a fold or a macro replaced `a.b` with the value undefined. That form deletes an identifier: a SyntaxError in strict code, `false` in sloppy code. esbuild prints `delete void 0` here, and so does bun with --minify-syntax. The printer now prints `void 0` for an undefined operand of `delete`, whatever the operand was in the source. The runtime transpiler cache version is 36.
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes. ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
WalkthroughThe printer now emits ChangesDelete Undefined Output
Suggested reviewers: Priority: ➖ Normal Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Title check |  |
The title clearly and concisely identifies the main change: printing void 0 for the value undefined in delete expressions. |
| Description check | |
The description explains the problem, fix, affected cases, limitations, performance impact, and verification. It provides more detail than the template requires and covers both requested areas. |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
|
Updated 8:45 AM PT - Oct 17th, 2026
✅ @robobun, your commit 9a772bd6bee074a1c20e7d35d8a41f2aa362fc20 passed in 🧪 To try this PR locally: bunx bun-pr 45526That installs a local version of the PR into your bun-45526 --bun |
|
Status: ready for review. How I reproduced it (release build of main 116dd64, and bun 1.4.2): $ printf 'delete process.env.DEBUG;\nconsole.log("loaded");\n' > entry.mjs
$ bun build ./entry.mjs --target=bun --define 'process.env.DEBUG=undefined' --outfile out.mjs && bun out.mjs
SyntaxError: Cannot delete unqualified property 'undefined' in strict mode.
PR: #45526 |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.
Beyond the inline findings, I also checked the other consumers of is_identifier_or_numeric_constant_or_property_access after EUndefined was dropped from it: the only callers are the delete arm at src/js_printer/lib.rs:4446 (where is_undefined_value is tested first) and its own EInlinedEnum recursion, so no other print path changes. The delete ns.missing namespace-import case and the cache-refill cost were also looked at and are pre-existing / expected, not regressions from this diff.
Extended reasoning...
The change is a small, focused edit to the prefix-unary print path in src/js_printer/lib.rs plus a transpiler cache version bump; it touches no security-sensitive surface. Three inline findings were posted (a sloppy-mode const-inlining result change, a test that cannot distinguish which producer regressed, and the pre-existing NaN/Infinity gap), so a human should weigh those before merging.
| // "delete undefined" deletes an identifier, whatever the operand was in the source | ||
| self.print_space_before_identifier(); | ||
| self.add_source_mapping(e.value.loc); | ||
| self.print(b"void 0"); |
There was a problem hiding this comment.
🟡 (optional) Sloppy CommonJS code transpiled with const inlining but without minify syntax now gets true from delete x where base and Node give false. Bun.Transpiler({inline:true}) and the runtime under --inspect inline const x = undefined at src/js_parser/p.rs:2210, so the delete operand becomes EUndefined; src/js_printer/lib.rs:4448-4452 then prints delete void 0 where the base printed delete undefined (false, matching the source). Fix: do not inline a const whose use is the delete target (gate at p.rs:2209 on opts.is_delete_target()), so every value class (undefined, numbers, strings) keeps the source's reference semantics under delete.
Why this was flagged
Trigger: sloppy-mode source const x = undefined; console.log(delete x) passed through Bun.Transpiler with inline:true and no minify.syntax, or through the runtime transpiler in a configuration with inlining on and minify_syntax off (the PR text names --inspect). handle_identifier at src/js_parser/p.rs:2209-2214 replaces the identifier with the const value even when opts.is_delete_target() is set, giving EUnary(delete, EUndefined) with the WAS_ORIGINALLY_DELETE flag. On base the printer emitted delete undefined, which in sloppy mode returns false (the global undefined is non-configurable), matching the source's false for deleting a const binding. After this change lib.rs:4448-4452 emits delete void 0, which returns true. The dismissal relied on the author calling it an accepted downside and on minify-syntax output already being void 0; but the non-minify inlining population is a real configuration and the result flips from correct to incorrect. Remedy: skip const inlining for a delete target, which also keeps delete x for inlined numbers/strings consistent.
Verification: Trigger: sloppy-mode source const x = undefined; delete x transpiled with inlining on and minify_syntax off. handle_identifier at src/js_parser/p.rs:2209-2214 returns the inlined value before any is_delete_target() check. On base the printer wrote delete undefined, which evaluates to false. After the diff, src/js_printer/lib.rs:4446-4452 writes delete void 0, which evaluates to true.
There was a problem hiding this comment.
Correct. The PR body lists this under Downsides. The cause is the const inlining in handle_identifier, which has no delete check. On main the default runtime already prints delete void 0 for this input and returns true, and const x = 1; delete x returns true there too. A gate on that inlining changes the default runtime result, so it is a separate change and not part of this printer rule.
| if Op::Code::is_prefix(e.op) { | ||
| // Never turn "typeof (0, x)" into "typeof x" or "delete (0, x)" into "delete x" | ||
| if (e.op == Op::Code::UnTypeof && !e.flags.contains(E::UnaryFlags::WAS_ORIGINALLY_TYPEOF_IDENTIFIER) && self.is_unbound_identifier(&e.value)) | ||
| || (e.op == Op::Code::UnDelete && !e.flags.contains(E::UnaryFlags::WAS_ORIGINALLY_DELETE_OF_IDENTIFIER_OR_PROPERTY_ACCESS) && is_identifier_or_numeric_constant_or_property_access(&e.value)) | ||
| || (e.op == Op::Code::UnDelete && (is_undefined_value(&e.value) || (!e.flags.contains(E::UnaryFlags::WAS_ORIGINALLY_DELETE_OF_IDENTIFIER_OR_PROPERTY_ACCESS) && is_identifier_or_numeric_constant_or_property_access(&e.value)))) |
There was a problem hiding this comment.
🟣 pre-existing, not blocking: pre-existing: a bundled ES module still fails to load when a define, macro or inlined enum puts the value NaN or Infinity under delete. The UnDelete arm at src/js_printer/lib.rs:4446 only wraps a non-finite ENumber when WAS_ORIGINALLY_DELETE_OF_IDENTIFIER_OR_PROPERTY_ACCESS is clear, so delete a.b with a macro result {b: NaN} prints delete NaN, a strict-mode SyntaxError. Fix: treat every operand the printer spells as a bare identifier (undefined, NaN, Infinity) the same way under delete regardless of the flag, e.g. extend the new value branch to non-finite ENumber and --define x=NaN/Infinity identifier defines, printing (0, NaN) or void 0.
A small fix can ride a push you are already making; otherwise a short reply is enough.
Why this was flagged
Input: delete config().v where the macro returns { v: NaN }, or delete a.b with --define a.b=NaN (src/bundler/defines.rs:365 keeps NaN as an EIdentifier), in an ESM output or any strict-mode file. The parser sets WAS_ORIGINALLY_DELETE_OF_IDENTIFIER_OR_PROPERTY_ACCESS at src/js_parser/parse/parse_prefix.rs:419 because the source operand was an EDot. In the printer, src/js_printer/lib.rs:4446 fires the wrapper only when is_undefined_value is true or the flag is clear, so the operand is printed bare via print_number (src/js_printer/lib.rs:6912 prints NaN, 6932 prints Infinity in non-minified bundles). Output is delete NaN, which JavaScriptCore rejects in strict code with a SyntaxError and which returns false in sloppy code, whereas the source expression returned true. The base branch behaves identically; this PR fixes only the undefined member of the same class, and the PR text lists the NaN/Infinity cases as deliberately left out citing esbuild parity.
Verification: pre-existing; acknowledged in diff: the PR description's "Not changed, with the reason" section states that NaN/Infinity define values under delete still print delete NaN; the base already fails by the same route and this PR neither widens nor fixes it. At src/js_printer/lib.rs:4446 the wrapper fires only for is_undefined_value or a clear flag, so ENumber(NaN) with the flag set falls to the bare self.print_expr(...) branch at lib.rs:4460.
There was a problem hiding this comment.
Left out on purpose. For these values esbuild prints delete NaN and delete Infinity too, so a fix leaves the bytes of the reference and needs a maintainer decision first. A number arm that ignores the flag also turns const x = NaN; delete x from false into true in the default runtime, until const inlining is gated under delete. The PR notes give the follow-up shape.
There was a problem hiding this comment.
The second push addresses the per-producer assertion point from my earlier review, and this run found no new bugs; a human look is still worthwhile because this changes printed bundler output for every delete operand the printer spells as undefined, and the PR knowingly accepts a runtime-semantics difference under const inlining (--inspect) that a maintainer should sign off on.
What was reviewed:
- The
UnDeletearm and the newis_undefined_valuehelper againstprint_undefinedand theESpecial::HotDisabledarm (which delegates toEUndefined), so the helper covers both shapes that print as the bare identifier; the only other caller of the trimmed predicate is this same arm, and the new check runs first, so no other output changes. - A fourth candidate producer, a bundled
delete ns.missingon a namespace import (anEImportIdentifierwithMissingstatus printsundefinedat src/js_printer/lib.rs:4293): ruled out, because the parser rejects any delete target bound to an import symbol withCannot assign to importbefore the linker can mark it missing, and dynamic-import locals leave delete targets asEDot. - The transpiler cache version bump to 36 is present; CODEOWNERS covers none of the changed paths.
Extended reasoning...
The change touches the JS printer's prefix-unary arm (src/js_printer/lib.rs), bumps the runtime transpiler cache version in src/jsc/RuntimeTranspilerCache.rs, and adds exact-output tests in test/bundler/esbuild/extra.test.ts and test/bundler/transpiler/transpiler.test.js covering define, import.meta define, macro and import.meta.hot producers across esm/cjs/iife and minify settings. It touches no security-sensitive surface. The second commit addressed my prior inline comment about asserting the printed line per producer; the other two prior comments were optional or pre-existing and the PR text declines them with reasons. Deferring rather than approving because it is a semantic change to bundler output with an author-acknowledged behavior difference under const inlining, which is a maintainer-level design call even though the diff itself is small and I found no bugs in it.
Still open from earlier reviews (2):
- Unresolved: 2 minor or pre-existing.
…te-define-stale-flag
Problem
bun build --define process.env.DEBUG=undefinedturnsdelete process.env.DEBUGintodelete undefined. An ES module with that line fails to load:SyntaxError: Cannot delete unqualified property 'undefined' in strict mode.Thedebugpackage has this line.deleteoperand bare when the flagWAS_ORIGINALLY_DELETE_OF_IDENTIFIER_OR_PROPERTY_ACCESSis set (src/js_printer/lib.rs:4446). The define made the operand the value undefined, printed as the identifierundefined.Fix
EUnaryarm printsvoid 0for an undefined operand ofdelete, with or without the flag. esbuild prints the same bytes.import.meta.hotfold, a macro result.test/bundler/transpiler/transpiler.test.js,test/bundler/esbuild/extra.test.ts. More suites in Notes.Background
delete xdeletes a binding. Strict code forbids it.delete <value>evaluates the value and returnstrue.undefinedis an identifier.delete (0, undefined)(js_printer: re-wrap delete of folded import/cjs-export identifier #36740).delete(a browser bundle keeps a liveprocessreference).Downsides
NaNandInfinitydefine values still printdelete NaNanddelete Infinity, as esbuild does. Strict code rejects both.--inspect, sloppyconst x = undefined; delete xnow givestrue. Node givesfalse. The default runtime already gavetrue.delete(of about 3,128), 0 allocations, +512 bytes, one transpiler cache refill.Notes
Repro
After the change
out.mjshasdelete void 0;and printsloaded. esbuild 0.21.5 and 0.28.2 printdelete void 0;for the same input. In cjs and iife output the old line loads and givesfalse, where the source givestrue.Output that changes (release builds of main 116dd64 and this PR,
Bun.Transpilerand bundles in esm and cjs)delete a.b, definea.b=undefineddelete undefineddelete void 0delete import.meta.hot, HMR offdelete undefineddelete void 0delete config().missing, macro resultdelete undefineddelete void 0delete void 0,delete (null ?? undefined),delete (null ?? a.b)delete (0, undefined)delete void 0--minify-syntaxand in the default runtimedelete (0, void 0)delete void 0The first three rows did not load in strict code. The last two rows loaded and gave
true, and still do. With--minify-syntaxthe first three rows already printeddelete void 0.Not changed, with the reason
NaN,Infinityand1e999define values underdeleteprintdelete NaN/delete Infinity, byte-equal to esbuild 0.28.2. Node rejects both in strict code. A fix leaves esbuild's bytes, so a maintainer has to accept it first. The shape would be: store those define values as numbers inDefineData::parse(src/bundler/defines.rs), as esbuild'sinstantiateDefineExprdoes, then wrap a non-finite number underdeletein this same printer arm.--define a.b=xprintsdelete x, as esbuild does.delete [x][0]anddelete [o.p][0]with--minify-syntaxand in the default runtime fold todelete xanddelete o.p. Open js_parser: keep a const of one switch case out of the other cases #30936 gates that fold for assignment and delete targets.delete NaN /* N */. Open js_printer: wrap cross-module enum inlined as a delete operand when non-finite #36744 and js_printer: inline a cross-module enum member only where the parser counted a read #42454 cover it.deletethat load but give another result than the source:delete "abc".length,delete "foo"[2],module.id,import.meta.main. Open js_parser: keep a const of one switch case out of the other cases #30936 covers them.delete(handle_identifier,src/js_parser/p.rs): the default runtime givestrueforconst x = 1; delete x, node givesfalse. This PR does not touch it. It is the cause of the second Downsides bullet.delete NaN/delete Infinity.Overlap with open PRs
thisin an ES module #32173 pinsexport {}; delete thisasdelete (0, undefined)anddelete (0, void 0). With this PR both aredelete void 0.import.meta.hotfold underdelete. The tests here assert the run result fordelete import.meta.hot, not its bytes.thisin an ES module #32173 also moveEXPECTED_VERSION.0 / 0andvoid 0where a local binding orwithobject shadowsNaN/undefined#36124 changesprint_undefinedfor files that shadowundefined. This PR does not touchprint_undefined.deleteoperand before and after the visit.Design history
The first version of this branch cleared the flag in the
UnDeletearm ofe_unarywhen the operand came back as a primitive value, with a classifier over each expression kind (106 source lines). The self-review rejected it: the printer is the only code that spells a value asundefined, it already has the predicate from #36740, and the visit pass cannot see print-time rewrites. This version is 24 added and 8 removed source lines, 22 and 7 of them in the printer.Self-review concerns not addressed: a generated position-by-operand differential test (it belongs with the assignment targets in #30936), an upstream esbuild report for
delete NaN, the const-inlining gate (a separate change), a redesign of the[x][n]fold, and a test through--inspect.Measurements (release builds of main 116dd64 and the PR, linux x64. instructions:u from a
perf_event_openwrapper, 10 interleaved runs each, run-to-run spread about 1,000)print_expr+98 to +148 bytes in each of 5 copies,print_undefinedandvisit_expr_in_outunchanged, text +512 bytes (nm,size)--no-bundle: -2,094 instructions of 950,788,294. With--minify-syntax: -2,001 of 973,894,077x = !o.p;: -32. 200,000x = typeof o.p;: +290delete o.p;: +999,936, so +5.0 per statement of about 3,128strace,perfandvalgrindare not installed)cmp): typescript.js, prettier, a React app with the default defines, esbuild'slib/main.jsSuites run with the debug build, all green
transpiler/transpiler.test.js,esbuild/extra.test.ts,bundler_minify,bundler_cjs2esm,bundler_drop,transpiler/react-compiler,transpiler/assign-to-import,transpiler/macro-test,transpiler/runtime-transpiler,esbuild/ts,esbuild/dce,cli/run/transpiler-cache, and theedgecase/Delete*cases ofbundler_edgecase.