Repository navigation
Conversation
A computed `{ ["__proto__"]: v }` key and the shorthand `{ __proto__ }`
define an own property named "__proto__". Lowering mapped both to a
static ObjectPropertyKey::String, which codegen emits as `__proto__: v`,
the form that sets the object's prototype instead (and is ignored for a
non-object value). Lower these two forms to a computed key so codegen
emits `["__proto__"]: v`. `{ __proto__: v }` and `{ "__proto__": v }`
still lower to a static key and keep setting the prototype.
|
Warning Review limit reached
Next review available in: 6 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
Comment |
|
Status: reproduced on bun 1.4.0 and on main ( |
There was a problem hiding this comment.
LGTM — narrowly targeted fix that preserves own-property semantics for __proto__ keys through React Compiler HIR lowering.
What was reviewed:
- The
(computed || WasShorthand) && key == "__proto__"condition is exactly the complement of the parser's prototype-setter predicate invisit_expr.rs:1667-1676(methods diverted earlier), so setter forms and every other key still go throughlower_object_property_keyunchanged. codegen_object_expression(codegen.rs:2402) only setsIsComputedforObjectPropertyKey::Computed, confirming the fix routes the own-property forms to the right emission.- The new test runs the bundle (own keys + prototype checked for all six forms) and asserts on emitted text; it also guards that
Compwas actually compiled via the memo-cache sentinel, so a silent bailout would fail.
Extended reasoning...
Overview
Two files: a ~15-line change to the EObject arm of lower_expression in src/react_compiler/lowering/build_hir/expr.rs, and a new itBundled case in test/bundler/transpiler/react-compiler.test.ts. When an object property key is the literal string __proto__ and the source used the computed or shorthand form (both own-property forms per Annex B), the key is now lowered to ObjectPropertyKey::Computed (a string-primitive temporary) instead of ObjectPropertyKey::String, so codegen emits ["__proto__"]: v rather than the prototype-setter __proto__: v.
Security risks
None. Build-time HIR lowering of object-literal keys; no parsing of untrusted input, no allocation-size arithmetic, no I/O.
Level of scrutiny
Medium — the React Compiler regenerates component ASTs, so a lowering mistake changes runtime semantics. The change is tightly gated: only fires when the key is exactly the EString "__proto__" (via eql_comptime, the same helper the parser and own_key_property_flags use) and one of IsComputed/WasShorthand is set. All other keys, all method properties (handled by lower_object_method before this block), and the two setter spellings ({ __proto__: v }, { "__proto__": v }) fall through to the existing path unchanged. I cross-checked lower_object_property_key in helpers.rs and codegen_object_expression in codegen.rs — the former does return String for every EString regardless of computed, and the latter only sets Property::IsComputed for the Computed variant, so both the diagnosis and the routing are correct. The template-literal form is covered because Bun's parser folds a substitution-free template key to EString (and if it didn't, the fallback _ if computed arm in lower_object_property_key already yields Computed).
Other factors
The test is strong: it bundles and executes, asserting Object.keys and Object.getPrototypeOf for all six forms (three own-property, one method, two setter), plus textual assertions counting three ["__proto__"] keys and two bare __proto__: keys, and a guard that the component was actually compiled (react.memo_cache_sentinel present) so a bailout can't mask a regression. The PR body reports the rest of react-compiler.test.ts and the 3293-fixture suite pass unchanged. No CODEOWNERS cover these paths and there are no outstanding reviewer comments.
Problem
{ ["__proto__"]: v }is emitted as{ __proto__: v }. The source defines an own property named"__proto__"; the output sets the object's prototype instead (and does nothing whenvis not an object), soObject.keys(o)goes from["__proto__"]to[]. The same build withoutreactCompilerprints the key unchanged.{ __proto__ }has the same problem once the value is no longer an identifier named__proto__. Withconst __proto__ = "own"the compiler inlines the constant and emits{ __proto__: "own" }, which drops the property. The identifier case only survives today because the printer happens to collapse__proto__: __proto__back to shorthand.EObjectarm oflower_expression(src/react_compiler/lowering/build_hir/expr.rs) lowers every string key throughlower_object_property_key(build_hir/helpers.rs), which returnsObjectPropertyKey::Stringregardless of thecomputedargument, so the HIR no longer records that the key was computed or shorthand.codegen_object_expression(src/react_compiler/codegen.rs) only setsProperty::IsComputedforObjectPropertyKey::Computed, and a bare__proto__key is the prototype-setter form. For any other name the two spellings are equivalent;__proto__is the one name where they are not.lowerObjectPropertyKeymatchesStringLiteralbefore it looks atcomputed), so the fixture suite does not cover it.Fix
EObjectarm oflower_expression, a property whose key is the string__proto__and which is computed or was written as shorthand is lowered toObjectPropertyKey::Computed(the key literal becomes a stringPrimitivetemporary), so codegen emits["__proto__"]: v. Everything else still goes throughlower_object_property_key.visit_expr.rs, the duplicate__proto__check: not computed, not shorthand, not a method).{ __proto__: v }and{ "__proto__": v }still lower to a static key and still set the prototype; methods takelower_object_methodand are never setters, so they are untouched.const k = "__proto__"; ({ [k]: v })lowers to a computed key whose place is constant-folded to the string, and today's output for it is already{ ["__proto__"]: v }(see the upstream fixtureobject-expression-computed-key-constant-string). The emitted computed key is also immune to later symbol renaming in the bundle, unlike a shorthand property.bun bd test:test/bundler/transpiler/react-compiler.test.ts,react-compiler/OwnProtoKeyStaysOwnProperty: one compiled component builds the computed, template-literal, shorthand and method own-property forms plus the identifier and quoted setter forms, and the bundle is run and prints each object's own keys and whether its prototype is the supplied object. It also checks the emitted text: three["__proto__"]keys and two bare__proto__:keys. On the unfixed build the computed, template and shorthand objects have no own keys and the bundle contains no computed key.react-compiler.test.ts(37 tests) andreact-compiler-fixtures.test.ts(3293 upstream fixtures, unchanged output) pass.__proto__for uncompiled code, bundler: keep a "__proto__" export name an own property in generated exports #33052 the object literals the bundler itself generates for exports. This PR is about the object literals the React Compiler regenerates from HIR; neither of those changes what the compiler emits.Background
__proto__in object literals: per Annex B,{ __proto__: v }and{ "__proto__": v }set the new object's[[Prototype]]tov(ignored whenvis neither an object nornull). The computed{ ["__proto__"]: v }, the shorthand{ __proto__ }and the method{ __proto__() {} }forms create an ordinary own property. Bun's AST stores identifier and string keys both asE::String; theIsComputedandWasShorthandproperty flags are what distinguish the forms.src/react_compilerrewrites each component or hook by lowering its body to HIR and generating a fresh AST from that HIR, so any distinction that is not carried into HIR is gone from the output.ObjectPropertyKeyhasString,Identifier,NumberandComputedvariants;Computedholds aPlace(an HIR temporary) that codegen prints inside brackets, and a temporary holding a string constant is printed inline as the literal.Repro on the unfixed build
The test's component, bundled with
reactCompiler: trueand run, prints on the unfixed build:and with this change:
Emitted keys after the change:
["__proto__"]: id,[`__proto__`]: id,["__proto__"]: "own",__proto__() {},__proto__: proto,__proto__: proto.