revert(es/decorators): revert decorator initializer ordering#11901
Conversation
![kodiakhq[bot]](https://avatars.githubusercontent.com/in/29196?s=60&v=4){kind=small}
🦋 Changeset detectedLatest commit: 3273a08 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3273a083a7
ℹ️ 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".
| - name: Install wasm-pack | ||
| # The rustwasm shell installer can crash on GitHub Windows runners after | ||
| # installing. Use the npm package so every CI OS follows one install path. | ||
| run: npm install -g wasm-pack | ||
| run: curl https://rustwasm.github.io/wasm-pack/installer/init.sh -sSf | sh |
There was a problem hiding this comment.
In the node-test job I checked the matrix above this step and it still runs on windows-latest as well as macOS, so this reinstates the rustwasm shell installer on Windows. The workflow section being reverted explicitly used the npm package because this shell installer can crash on GitHub Windows runners after installing; with this line the Windows node-bindings CI can fail before pnpm build:dev/tests run. Keep the npm-based install here, or gate the shell installer to non-Windows runners.
Useful? React with 👍 / 👎.
| && i + 1 < body_len | ||
| && matches!( | ||
| &n.body[i + 1], | ||
| ClassMember::Method(m) if m.kind == MethodKind::Getter | ||
| ) |
There was a problem hiding this comment.
For 2022-03 classes, this predicate now accepts any initialized private field immediately followed by a public getter, not just the dummy private storage synthesized for auto-accessors. In a class with an instance decorator that creates _initProto and a source #field = ...; get x() { ... }, _initProto(this) is injected into the #field initializer instead of falling back to the constructor, so addInitializer callbacks can run while that private field is still being initialized and throw or observe the wrong order when they access it. Restore the dummy-span plus getter/setter backing-storage checks that were removed here.
Useful? React with 👍 / 👎.
| # without the repository-scoped GitHub token, which GitHub can reject | ||
| # when fetching an unadvertised submodule commit directly by SHA. | ||
| run: git submodule update --init --force --depth=1 | ||
| submodules: true |
There was a problem hiding this comment.
This same submodules: true replacement appears in the cargo-clippy, cargo-check, and cargo-test checkout steps, but the removed step explicitly fetched public submodules after checkout so they were not fetched with the repository-scoped GitHub token. Moving submodule initialization back into actions/checkout reintroduces the unadvertised-submodule-SHA failure mode that comment was guarding against, causing these CI jobs to fail before running Rust checks when a public submodule commit cannot be fetched via the workflow token.
Useful? React with 👍 / 👎.
| && matches!( | ||
| &n.body[i + 1], | ||
| ClassMember::Method(m) if m.kind == MethodKind::Getter | ||
| ) |
There was a problem hiding this comment.
This 2022-03 backing-field check now only recognizes a public getter after the synthesized storage field, so private auto-accessors (whose generated accessors are ClassMember::PrivateMethod) no longer get _initProto injected into the backing-field initializer. In the ES2015 output for a class with only decorated private accessors, the generated _init_a(this) field initialization now runs before _initProto(this), which reverses the existing ordering tests' expectation that accessor addInitializer callbacks run before accessor init effects. Include the private getter/setter pattern here as well.
Useful? React with 👍 / 👎.
Merging this PR will not alter performance
Comparing 
Footnotes
|
1 participant
Description:
Reverts #11847 because the helper changes shipped in
@swc/helpers@0.5.22caused a decorator helper regression.This restores the previous 2022-03 decorator initializer behavior, removes the regression fixtures added by #11847, reverts the matching external helper source, and adds a patch changeset for the touched Rust crates.
@swc/helpers@0.5.22has been deprecated on npm with an upgrade note for0.5.23.Validation:
git submodule update --init --recursivecargo fmt --allcargo test -p swc_ecma_transforms_basecargo test -p swc_ecma_transforms_proposalcargo clippy --all --all-targets -- -D warningspnpm --filter @swc/helpers buildwas also attempted, but localnode_modulesis not installed in this checkout, sozxwas unavailable.BREAKING CHANGE:
None.
Related issue (if exists):
Reverts #11847