Skip to content

fix(transformer/class-properties): preserve RHS in logical-assignment to static private field#21950

Merged
graphite-app[bot] merged 1 commit intomainfrom
fix/transformer-class-properties-static-private-logical-assign
Apr 30, 2026
Merged

fix(transformer/class-properties): preserve RHS in logical-assignment to static private field#21950
graphite-app[bot] merged 1 commit intomainfrom
fix/transformer-class-properties-static-private-logical-assign

Conversation

@Dunqing
Copy link
Copy Markdown
Member

@Dunqing Dunqing commented Apr 29, 2026

fixes #21874

transform_static_assignment_expression took assign_expr.right once at the top of the function (correct for the = branch), then took it a second time inside the else branch that handles compound operators. The second take_in returned a NullLiteral dummy, so ??= / &&= / ||= (and the binary ops) on a static private field where the receiver isn't a direct class reference — e.g. this.#shared ??= 1 inside a static method — emitted null in place of the RHS:

// before
class Singleton {
  static #shared;
  static shared() {
    this.#shared ??= 1;
    return this.#shared;
  }
}

// transformer output (broken):
_assertClassBrand(Singleton, this, _shared)._ ?? (_shared._ = _assertClassBrand(Singleton, this, null));
//                                                                                              ^^^^ should be 1

Removes the redundant inner let class_ident = …; let value = assign_expr.right.take_in(…); shadowing so the compound-operator branches use the already-captured outer value (the real RHS) when building the _assertClassBrand(Class, object, value) call. Output now matches Babel.

Adds a private-static-logical-assignment conformance fixture covering ??=, &&=, and ||=, plus an exec.js to verify runtime behavior.

Copy link
Copy Markdown
Member Author

Dunqing commented Apr 29, 2026

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more


How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • 0-merge - adds this PR to the back of the merge queue
  • hotfix - for urgent changes, fast-track this PR to the front of the merge queue

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented Apr 29, 2026

Merging this PR will not alter performance

✅ 44 untouched benchmarks
⏩ 7 skipped benchmarks1


Comparing fix/transformer-class-properties-static-private-logical-assign (4ab46d0) with fix/transformer-typescript-parameter-property-fields (bf76f6c)

Open in CodSpeed

Footnotes

  1. 7 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@Dunqing Dunqing force-pushed the fix/transformer-class-properties-static-private-logical-assign branch from 86ff08b to 4ab46d0 Compare April 29, 2026 16:22
@Dunqing Dunqing marked this pull request as ready for review April 29, 2026 16:23
@Dunqing Dunqing requested a review from overlookmotel as a code owner April 29, 2026 16:23
@Dunqing Dunqing added the 0-merge Merge with Graphite Merge Queue label Apr 30, 2026
Copy link
Copy Markdown
Member Author

Dunqing commented Apr 30, 2026

Merge activity

… to static private field (#21950)

fixes #21874

`transform_static_assignment_expression` took `assign_expr.right` once at the top of the function (correct for the `=` branch), then took it a second time inside the `else` branch that handles compound operators. The second `take_in` returned a `NullLiteral` dummy, so `??=` / `&&=` / `||=` (and the binary ops) on a static private field where the receiver isn't a direct class reference — e.g. `this.#shared ??= 1` inside a static method — emitted `null` in place of the RHS:

```js
// before
class Singleton {
  static #shared;
  static shared() {
    this.#shared ??= 1;
    return this.#shared;
  }
}

// transformer output (broken):
_assertClassBrand(Singleton, this, _shared)._ ?? (_shared._ = _assertClassBrand(Singleton, this, null));
//                                                                                              ^^^^ should be 1
```

Removes the redundant inner `let class_ident = …; let value = assign_expr.right.take_in(…);` shadowing so the compound-operator branches use the already-captured outer `value` (the real RHS) when building the `_assertClassBrand(Class, object, value)` call. Output now matches Babel.

Adds a `private-static-logical-assignment` conformance fixture covering `??=`, `&&=`, and `||=`, plus an `exec.js` to verify runtime behavior.
@graphite-app graphite-app Bot force-pushed the fix/transformer-typescript-parameter-property-fields branch from bf76f6c to c59c199 Compare April 30, 2026 00:39
@graphite-app graphite-app Bot force-pushed the fix/transformer-class-properties-static-private-logical-assign branch from 4ab46d0 to e3399ec Compare April 30, 2026 00:40
@graphite-app graphite-app Bot changed the base branch from fix/transformer-typescript-parameter-property-fields to main April 30, 2026 00:43
@graphite-app graphite-app Bot removed the 0-merge Merge with Graphite Merge Queue label Apr 30, 2026
@graphite-app graphite-app Bot merged commit e3399ec into main Apr 30, 2026
28 checks passed
@graphite-app graphite-app Bot deleted the fix/transformer-class-properties-static-private-logical-assign branch April 30, 2026 00:44
camc314 added a commit that referenced this pull request May 5, 2026
### 💥 BREAKING CHANGES

- 0ffbe0d allocator: [**BREAKING**] Remove `Allocator::end_ptr` method
(#21871) (overlookmotel)

### 🚀 Features

- 9593ec8 transformer/jsx: Add jsxDEV source metadata for fragments
(#21932) (Ido Rosenthal)

### 🐛 Bug Fixes

- 429deac napi/parser: Export `visitorKeys` from `wasm` entrypoint
(#21996) (NullVoxPopuli)
- e852911 codegen: Preserve legal comments orphaned by upstream passes
(#21575) (Dunqing)
- e3399ec transformer/class-properties: Preserve RHS in
logical-assignment to static private field (#21950) (Dunqing)
- c59c199 transformer/typescript: Emit class fields for parameter
properties (#21831) (Dunqing)
- aaabde4 parser: Attach legal comments to following token (#21670)
(Dunqing)

### ⚡ Performance

- 0bf0cb9 allocator: Per-platform `Arena::new_fixed_size`
implementations (#22088) (overlookmotel)

### 📚 Documentation

- 62ec410 allocator: Correct doc comment for `Allocator::from_raw_parts`
(#22093) (overlookmotel)
- 3e152c6 allocator: Correct typos in comments (#22092) (overlookmotel)
- e220855 allocator: Correct doc comment for `Allocator::set_cursor_ptr`
(#21866) (overlookmotel)

---------

Co-authored-by: Boshen <1430279+Boshen@users.noreply.github.com>
Co-authored-by: Cameron Clark <cameron.clark@hey.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

transformer: ??= on a static #private field drops the RHS value

1 participant