Conversation
…izers and static blocks A class field initializer and a class static block run as a method call, so new.target is always undefined in them. An arrow function in them takes the same value. JavaScriptCore throws "ReferenceError: Can't find private variable: PrivateSymbol.newTargetLocal" on entry to an arrow function that has no function around it when a class inside it has new.target in a field initializer or a static block. The bundler's __esm wrapper is such an arrow function, so a module that runs unbundled throws once bundled. With a function around the arrow function, a static block throws too. Lowering also moves the initializer of an auto-accessor into the constructor, where new.target is the class.
A cached file that was transpiled before this change still has new.target in a class field initializer or a class static block.
… into the constructor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Essentials Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 10 included reviews per hour; 1 remains after this review. WalkthroughThe parser now tracks class field initializer and static block contexts where Changesnew.target context handling
Suggested reviewers: Priority: ➖ Normal Merge Risk: ⚪ Minimal · up to The parser rewrite is scoped to the intended class-field and static-block cases, with cache versioning and regression coverage in place. 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
|
Updated 1:45 PM PT - Sep 13th, 2026
✅ @robobun, your commit 5a92b150bacd989195e80bc1ad7455a87f4fb9bc passed in 🧪 To try this PR locally: bunx bun-pr 42653That installs a local version of the PR into your bun-42653 --bun |
|
Status Reproduced on bun 1.4.3-canary.1+b99371011 and on a debug build of main (09bb546): // repro.mjs
const run = f => { try { return f(); } catch (e) { return String(e); } };
console.log(run(() => { class A { x = typeof new.target; } return new A().x; }));
console.log(run(() => { let r; class A { static { r = typeof new.target; } } return r; }));
The engine fix is oven-sh/WebKit#647. This PR does not depend on it. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — small, spec-correct rewrite with the cache version bumped and thorough positive/negative coverage.
What was reviewed:
FnOnlyDataVisitflag placement: inherited by arrows (e_arrowdoesn't touch it), reset byvisit_funcvia..Default::default(), so functions/methods/constructors inside a field keepnew.target.- Save/restore around field initializers matches the neighboring
is_this_nestedpattern; static-block struct literal is exhaustive (only two fields). - Negative cases (computed keys,
extends, constructor, function-in-field) are pinned by printer snapshots and runtime assertions; nested class-in-field key correctly inherits the outer field'sundefined. - Tests follow harness conventions (tempDir, concurrent pipe drain, stdout/stderr before exitCode,
.toEqualon one object).
Extended reasoning...
Overview
This PR adds a single boolean is_new_target_undefined to FnOnlyDataVisit in the JS parser's visit pass, sets it while visiting class field initializers and class static blocks, and rewrites E::NewTarget → E::Undefined when the flag is set. It bumps RuntimeTranspilerCache::EXPECTED_VERSION 32→33 and adds tests to four existing files (bundler edgecase, runtime transpiler, printer snapshots, and useDefineForClassFields: false). The parser change is ~5 lines of logic plus a struct field.
Security risks
None. This is a compile-time AST rewrite of a meta-property whose value is spec-defined as undefined in these contexts. No untrusted input parsing, no allocation, no FFI, no auth/crypto surface.
Level of scrutiny
Low-to-moderate. The correctness hinges on flag scoping, which I traced: FnOnlyDataVisit is documented as saved/restored around nested functions but not arrows; visit_func at mod.rs:96 resets it via ..Default::default() (so is_new_target_undefined becomes false inside any real function/method/constructor), and e_arrow never touches it (so arrows inherit). The field-initializer path saves/restores the old value around visit_expr, mirroring the existing is_this_nested pattern at lines 1122/1223. The static-block path replaces ..Default::default() with an explicit two-field literal, which is exhaustive since the struct now has exactly two fields. Computed keys and extends are visited before the flag is set (mod.rs:1048 for extends; keys are visited outside the initializer block), so they correctly retain new.target — verified by the printer test at transpiler.test.js:3852.
Other factors
REVIEW.md's cache-version-bump rule is satisfied with a doc line matching the existing convention. The upstream WebKit PR is linked in the code comment as required. Test coverage hits the whole variant matrix (instance/static/private fields, static blocks, arrows-in-fields, class expressions, derived classes, auto-accessors) and the negative contract (constructor, function-in-field, function-in-static-block, computed keys, extends clause). Tests follow harness conventions: tempDir, bunExe()/bunEnv, Promise.all for pipe draining, stdout/stderr asserted before exitCode, .toEqual on a single result object, test.concurrent for the subprocess test. No CODEOWNERS cover the touched paths. Two earlier bot inline threads were followed by two subsequent commits, and the current diff is clean at those lines. Exit reason was dry_streak.
|
Closing: this change is now a commit of #30936 ( |
1 participant
Problem
new.targetin a class field initializer or a class static block throwsReferenceError: Can't find private variable: PrivateSymbol.newTargetLocalwhen the class is inside an arrow function that has no function around it. Node printsundefined.new.targetto the code around the class, and an arrow function then loads a value that no function stored. [JSC] new.target in a class field initializer or a class static block throws a ReferenceError when the class is in an arrow function WebKit#647 fixes the engine.__esm(() => { ... }). A module that runs unbundled throws once bundled. Lowering also moves the initializer ofaccessor x = new.targetinto the constructor, where it is the class.Fix
undefinedfornew.targetin a class field initializer and a class static block (e_new_targetinsrc/js_parser/visit/visit_expr.rs). The flag is inFnOnlyDataVisit, the visit-pass state that an arrow function inherits andvisit_funcresets.undefined. A key, anextendsclause and a decorator belong to the code around the class and keepnew.target.EXPECTED_VERSIONgoes to 33.runtime-transpiler.test.ts,transpiler.test.jsandts-use-define-for-class-fields.test.tsintest/bundler/transpiler/, andedgecase/EsmWrapNewTargetInClassFieldInitializerintest/bundler/bundler_edgecase.test.ts. A debug build of main fails all four.Background
new.targetis the constructor thatnewran. An arrow function takes it from the function around it.__esmis the bundler's lazy wrapper for a module thatrequire()orimport()loads.Notes
Nobody reported this. Another change to decorator lowering found it in a test, and no code in the wild is known to hit it.
tscrejectsnew.targetin a field initializer with TS17013. It is a conformance fix, and it keepsbun buildfrom turning a module that runs into one that throws.Repro (
bun repro.mjs, compare withnode repro.mjs):bun 1.4.3-canary.1+b99371011 prints the ReferenceError four times. Node prints
undefinedfour times. An async arrow function throws from the call, it does not return a rejected promise.Why the transpiler and not only the engine:
bun buildoutput runs on other JavaScriptCore versions. The bundle ofexport class A { x = new.target }behindawait import()throws on the bun that built it and runs on node.new.targetis the class: an auto-accessor (class Foo { accessor x = new.target },new Foo().xis[class Foo]on canary) and TypeScript withuseDefineForClassFields: false.tscemitsthis.x = new.targetfor the second one, next to the TS17013 error. The value that ECMAScript gives isundefined. The engine fix cannot repair these.What the substitution does not reach: code that the transpiler does not see (
eval,new Function,node:vm).(0, eval)("class A { x = new.target }; new A().x")still throws until the engine fix lands.Positions, as the printed-output test pins them:
new.target), and the key,extendsclause and decorators of the class itself.new.target. That is right unless the class is itself inside a class element, where the value stays correct and only the substitution is missed.E::Undefinedprints asundefined, orvoid 0with--minify-syntax, like every other place that produces it.delete new.targetbecomesdelete (0, undefined).#42595 adds a line to the same
e_new_targetbody. The two changes do not depend on each other. The second one to land needs a rebase there. #42585 and #42588 also take cache version 33. The later ones take the next free number.Test runs, linux x64, debug build with ASAN: the four files above fail on a debug build of main (09bb546) and pass on this branch. Also
es-decorators.test.ts(418),es-decorators-esbuild.test.ts(147),decorators.test.ts(24),decorator-metadata.test.ts(5),ts-use-define-for-class-fields.test.ts(12), all oftranspiler.test.js(215 pass, 21 todo) and all ofbundler_edgecase.test.ts(181 pass, 11 todo).Self-reviewed: 4 concerns raised, 3 addressed. The cache version bump, the engine PR link in the code comment and a test for the
useDefineForClassFields: falsepath are in. The parameter decorator case above is left as it is, because it needs TypeScript experimental parameter decorators withnew.targetin a class inside a class element.[human-review] gate passed · iteration 0 · 8 files touched
fails on main (without fix)
passes on PR (with fix)
diff hotspot
gate history · 1 passed · 0 rejected · iteration 0
evidence per changed file