Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix protoInit call injection timing #16235

Merged
merged 2 commits into from Jan 26, 2024

Conversation

JLHwung
Copy link
Contributor

@JLHwung JLHwung commented Jan 25, 2024

Q                       A
Fixed Issues? Babel runs method initializers added by context.addInitializers after undecorated field initializer (REPL), fixes #16188
Patch: Bug Fix? Yes
Major: Breaking Change?
Minor: New Feature?
Tests Added + Pass? Yes
Documentation PR Link
Any Dependency Changes?
License MIT

This PR includes commits from #16234, I will rebase once that PR gets merged.

Per spec: InitializeInstanceElements

The method initializers should run before any fields are defined.

In the REPL example,

let initCalled;

function dec(value, context) {
  context.addInitializer(() => initCalled = true);
}

class C {
  @dec m() {}
  p = initCalled;
}

console.log((new C).p)

When the property p is defined, the initCalled should be true. Previously we inject the protoInit calls in the first decorated field, in this PR we move the field initializer injection out of the hasDecorators branch.

@JLHwung JLHwung added PR: Bug Fix 🐛 A type of pull request used for our changelog categories Spec: Decorators labels Jan 25, 2024
@babel-bot
Copy link
Collaborator

babel-bot commented Jan 25, 2024

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/56192/

Previously they are injected at the first _decorated_ field, which is probably an oversight.
@JLHwung JLHwung force-pushed the fix-proto-init-call-injection-timing branch from eb49b20 to 846153c Compare January 25, 2024 17:50
"after," +
"ctor:start," +
"m3,m4,m5,m6,g3,g4,g5,g6,s3,s4,s5,s6," + // instanceExtraInitializers
"f," + // InitializeFieldOrAccessor
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the current main branch, f is printed before m3,m4,m5,m6,g3,g4,g5,g6,s3,s4,s5,s6.

@JLHwung JLHwung merged commit 2870d0b into babel:main Jan 26, 2024
50 checks passed
@JLHwung JLHwung deleted the fix-proto-init-call-injection-timing branch January 26, 2024 14:32
liuxingbaoyu pushed a commit to liuxingbaoyu/babel that referenced this pull request Mar 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: Bug Fix 🐛 A type of pull request used for our changelog categories Spec: Decorators
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: decorators 2023-05: addInitializer runs initializer at the wrong time
4 participants