fix(react_compiler): lower delete obj.prop to Property/ComputedDelete#24123
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2b7c62c101
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Merging this PR will not alter performance
Comparing Footnotes
|
2b7c62c to
529d741
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 529d7411d5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
529d741 to
9e37b22
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9e37b221e8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
9e37b22 to
3af72d0
Compare
Merge activity
|
…te (#24123) Closes Group C of the "oxc compiles where the fork errors" gap — deleting a property of a frozen or global value. ## Root cause `build_hir` lowered `delete` on a member expression to a `Primitive(undefined)` no-op behind a leftover `TODO(stage1a-arms)`. So `delete obj.prop` was **silently dropped** — the deletion never reached the output, and it never produced a mutation effect. The downstream machinery was already in place: mutation inference emits `Mutate { object }` for `PropertyDelete`/`ComputedDelete` (`infer_mutation_aliasing_effects.rs:1953`) and codegen emits `delete ...` for them (`codegen_reactive_function.rs:2034`). Only the lowering was missing. ## Fix Lower `delete obj.prop` → `PropertyDelete` and `delete obj[key]` → `ComputedDelete`, reusing the existing `lower_member_expression` helper (matches TS `BuildHIR`). Non-member `delete` keeps the prior no-op. ## Result (9 snapshots, all improvements) - **3 error fixtures now error + decline** with `This value cannot be modified`, matching the fork: `error.invalid-delete-property-of-frozen-value`, `error.invalid-delete-computed-property-of-frozen-value`, `error.mutate-property-from-global`. - **6 fixtures that compile with a `delete`** now emit `delete ...` instead of the silently-dropped `undefined` (e.g. `delete-property` now matches the fork exactly; `round3_effect_read_vs_capture_v2` — previously a mis-compile — now emits `delete profile[key]`). Verified: full `cargo test -p oxc_react_compiler` (snapshot + 15 unit tests) green, `cargo clippy --all-features -D warnings` and `cargo fmt --check` clean.
3af72d0 to
83c94ef
Compare
…te (#24123) Closes Group C of the "oxc compiles where the fork errors" gap — deleting a property of a frozen or global value. ## Root cause `build_hir` lowered `delete` on a member expression to a `Primitive(undefined)` no-op behind a leftover `TODO(stage1a-arms)`. So `delete obj.prop` was **silently dropped** — the deletion never reached the output, and it never produced a mutation effect. The downstream machinery was already in place: mutation inference emits `Mutate { object }` for `PropertyDelete`/`ComputedDelete` (`infer_mutation_aliasing_effects.rs:1953`) and codegen emits `delete ...` for them (`codegen_reactive_function.rs:2034`). Only the lowering was missing. ## Fix Lower `delete obj.prop` → `PropertyDelete` and `delete obj[key]` → `ComputedDelete`, reusing the existing `lower_member_expression` helper (matches TS `BuildHIR`). Non-member `delete` keeps the prior no-op. ## Result (9 snapshots, all improvements) - **3 error fixtures now error + decline** with `This value cannot be modified`, matching the fork: `error.invalid-delete-property-of-frozen-value`, `error.invalid-delete-computed-property-of-frozen-value`, `error.mutate-property-from-global`. - **6 fixtures that compile with a `delete`** now emit `delete ...` instead of the silently-dropped `undefined` (e.g. `delete-property` now matches the fork exactly; `round3_effect_read_vs_capture_v2` — previously a mis-compile — now emits `delete profile[key]`). Verified: full `cargo test -p oxc_react_compiler` (snapshot + 15 unit tests) green, `cargo clippy --all-features -D warnings` and `cargo fmt --check` clean.
83c94ef to
c06d013
Compare
…te (#24123) Closes Group C of the "oxc compiles where the fork errors" gap — deleting a property of a frozen or global value. ## Root cause `build_hir` lowered `delete` on a member expression to a `Primitive(undefined)` no-op behind a leftover `TODO(stage1a-arms)`. So `delete obj.prop` was **silently dropped** — the deletion never reached the output, and it never produced a mutation effect. The downstream machinery was already in place: mutation inference emits `Mutate { object }` for `PropertyDelete`/`ComputedDelete` (`infer_mutation_aliasing_effects.rs:1953`) and codegen emits `delete ...` for them (`codegen_reactive_function.rs:2034`). Only the lowering was missing. ## Fix Lower `delete obj.prop` → `PropertyDelete` and `delete obj[key]` → `ComputedDelete`, reusing the existing `lower_member_expression` helper (matches TS `BuildHIR`). Non-member `delete` keeps the prior no-op. ## Result (9 snapshots, all improvements) - **3 error fixtures now error + decline** with `This value cannot be modified`, matching the fork: `error.invalid-delete-property-of-frozen-value`, `error.invalid-delete-computed-property-of-frozen-value`, `error.mutate-property-from-global`. - **6 fixtures that compile with a `delete`** now emit `delete ...` instead of the silently-dropped `undefined` (e.g. `delete-property` now matches the fork exactly; `round3_effect_read_vs_capture_v2` — previously a mis-compile — now emits `delete profile[key]`). Verified: full `cargo test -p oxc_react_compiler` (snapshot + 15 unit tests) green, `cargo clippy --all-features -D warnings` and `cargo fmt --check` clean.
c06d013 to
4e9194f
Compare
### 🚀 Features - 260425f semantic/examples: Include unresolved references (#24214) (camc314) - 2d9b0b3 minifier: Fold boolean-literal ternary branches in value contexts (#24110) (Dunqing) - 61fbf10 ast: Implement `ReplaceWith` on all AST types (#24013) (overlookmotel) - 7db7a29 allocator: Add `ReplaceWith` trait (#24012) (overlookmotel) - 4eb074e mangler: Add `reserved` option for names that must not be mangled (#24041) (Dunqing) - 2e62012 data_structures: Add `StringExt` trait (#24006) (overlookmotel) - 60e7160 minifier: Drop side-effect-free IIFEs whose result is unused (#23967) (Dunqing) - 26dd9e2 ast: Add method to widen inherited enum ref to parent ref (#23961) (overlookmotel) ### 🐛 Bug Fixes - e8b50ee transformer: Clean up semantics for stripped TypeScript syntax (#24180) (camc314) - d966d0b react_compiler: Remove clippy allows (#24168) (Boshen) - 854ef8d react_compiler: Compile generic functions instead of over-bailing on type-param hoisting (#24158) (Boshen) - 093586c react_compiler: Align memoization cache-slot allocation with Babel (#24157) (Boshen) - 09c8f59 react_compiler: Normalize snapshot fixture paths (#24142) (camc314) - f13df97 react_compiler: Drop stray empty statement from catch bindings (#24133) (Boshen) - cb2a505 react_compiler: Codegen destructuring reassignment targets (#24131) (Boshen) - b82c394 react_compiler: Propagate codegen invariants instead of emitting empty bodies (#24128) (Boshen) - 5771982 react_compiler: Render unchanged programs as source in fixture snapshots (#24129) (Boshen) - 4b16e1a transformer/async-to-generator: Preserve direct eval scope flags (#24136) (camc314) - 4e9194f react_compiler: Lower `delete obj.prop` to Property/ComputedDelete (#24123) (Boshen) - 0b25582 ast: Type binding node `typeAnnotation` as `TSTypeAnnotation | null` (#23113) (Boshen) - 018c0e5 transformer: Hoist lowered async declarations (#22770) (camc314) - 652fbaf mangler: Keep names of destructured exported bindings (#24036) (Dunqing) - e274415 minifier: Don't drop global calls that throw despite pure arguments (#23917) (Dunqing) - 59abb30 minifier: Only merge string literals in `try_fold_add` when the inner operator is `+` (#23622) (Jerry Zhao) ### ⚡ Performance - c5ca77b transformer: Avoid cloning refresh options (#24191) (camc314) - bf1a151 react_compiler: Compile out debug printers (#24184) (Boshen) - abb44a0 transformer: Build fixed object-rest arguments (#24190) (camc314) - a4db731 isolated_declarations: Use `ReplaceWith` instead of `TakeIn` (#24016) (overlookmotel) - ff10855 transformer: Use `ReplaceWith` instead of `TakeIn` (#24015) (overlookmotel) - bd49aff ecmascript: Avoid heap-allocating Math.min/max/imul operands (#23941) (Lawrence Lin) - e4b708b react_compiler: Skip compiled files before prefilters (#24171) (Boshen) - c59f2fe rust: Return impl ExactSizeIterator from slice-backed accessors (#24144) (Boshen) - 5d6d04a codegen: SWAR-skip boring byte runs in sourcemap line/column scan (#24023) (Boshen) - a55e0be traverse: Reduce string operations in `get_var_name_from_node` (#24007) (overlookmotel) - e6d48e1 transformer/nullish_coalescing: Move cold path into separate function (#23989) (overlookmotel) - c4e35b5 transformer/object_rest_spread: Pre-allocate capacity in `Vec` (#23988) (overlookmotel) - 527b8e5 transformer/decorators: Narrow type earlier (#23987) (overlookmotel) ### 📚 Documentation - 30d17f5 allocator: Clarify docs for `TakeIn::take_in_box` (#24093) (overlookmotel) - 675e6a8 ast: Correct doc comment for `PrivateFieldExpression` (#24008) (overlookmotel) - e4c30e6 minifier: Explain what `dce` mode means (#23994) (Dunqing) - 37cbf88 ast_macros: Document fields of `StructDetails` (#23959) (overlookmotel) - 4de3e54 ast: Correct doc comment (#23948) (overlookmotel) Co-authored-by: Boshen <[email protected]>
Closes Group C of the "oxc compiles where the fork errors" gap — deleting a property of a frozen or global value.
Root cause
build_hirlowereddeleteon a member expression to aPrimitive(undefined)no-op behind a leftoverTODO(stage1a-arms). Sodelete obj.propwas silently dropped — the deletion never reached the output, and it never produced a mutation effect. The downstream machinery was already in place: mutation inference emitsMutate { object }forPropertyDelete/ComputedDelete(infer_mutation_aliasing_effects.rs:1953) and codegen emitsdelete ...for them (codegen_reactive_function.rs:2034). Only the lowering was missing.Fix
Lower
delete obj.prop→PropertyDeleteanddelete obj[key]→ComputedDelete, reusing the existinglower_member_expressionhelper (matches TSBuildHIR). Non-memberdeletekeeps the prior no-op.Result (9 snapshots, all improvements)
This value cannot be modified, matching the fork:error.invalid-delete-property-of-frozen-value,error.invalid-delete-computed-property-of-frozen-value,error.mutate-property-from-global.deletenow emitdelete ...instead of the silently-droppedundefined(e.g.delete-propertynow matches the fork exactly;round3_effect_read_vs_capture_v2— previously a mis-compile — now emitsdelete profile[key]).Verified: full
cargo test -p oxc_react_compiler(snapshot + 15 unit tests) green,cargo clippy --all-features -D warningsandcargo fmt --checkclean.