Skip to content

Fix static/proto initializers when there aren't class fields#14335

Merged
nicolo-ribaudo merged 3 commits intobabel:mainfrom
JLHwung:fix-14242
Mar 8, 2022
Merged

Fix static/proto initializers when there aren't class fields#14335
nicolo-ribaudo merged 3 commits intobabel:mainfrom
JLHwung:fix-14242

Conversation

@JLHwung
Copy link
Copy Markdown
Contributor

@JLHwung JLHwung commented Mar 7, 2022 •
edited
Loading

Q                       A
Fixed Issues? Fixes #14334 (comment)
Patch: Bug Fix? Yes
Major: Breaking Change?
Minor: New Feature?
Tests Added + Pass? Yes
Documentation PR Link
Any Dependency Changes?
License MIT

The helper behaviour is now strictly aligned with the transformer:

babel/packages/babel-plugin-proposal-decorators/src/transformer-2021-12.ts

Lines 734 to 740 in 0b29359

if (kind !== FIELD) {
if (isStatic) {
requiresStaticInit = true;
} else {
requiresProtoInit = true;
}
}

Also added a script per #14334 (comment). Here is an an example output when we have uncommitted changes after build

Please re-run "make build" and checkout the following changes to git
HEAD detached at pull/14335/merge
Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
	modified:   packages/babel-helpers/src/helpers-generated.ts

no changes added to commit (use "git add" and/or "git commit -a")

/cc @wvq Please try if this PR fixes the issues you found when using decorators. Thank you!

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

babel-bot commented Mar 7, 2022 •
edited
Loading

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

@JLHwung JLHwung force-pushed the fix-14242 branch 3 times, most recently from 313330f to 7a76b0b Compare March 7, 2022 16:42
nicolo-ribaudo
nicolo-ribaudo reviewed Mar 7, 2022
View reviewed changes
Comment thread packages/babel-helpers/src/helpers/applyDecs.js
Comment on lines +450 to +452
if (kind !== 0 /* FIELD */) {
staticInitializers = staticInitializers || [];
initializers = staticInitializers;
Copy link
Copy Markdown
Member

@nicolo-ribaudo nicolo-ribaudo Mar 7, 2022

Choose a reason for hiding this comment

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

Nit, but this probably produces a smaller output: initializers = staticInitializers = staticInitializers || [].

Copy link
Copy Markdown
Member

@nicolo-ribaudo nicolo-ribaudo Mar 7, 2022

Choose a reason for hiding this comment

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

Ah no it's ok, the minifier step already produces this code.

Copy link
Copy Markdown
Contributor Author

@JLHwung JLHwung Mar 7, 2022

Choose a reason for hiding this comment

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

initializers = staticInitializers = staticInitializers || []

This is the exact output in helpers-generated by terser, so I don't bother to manually minify here.

nicolo-ribaudo
nicolo-ribaudo approved these changes Mar 7, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@nicolo-ribaudo nicolo-ribaudo left a comment

Choose a reason for hiding this comment

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

Code looks good, let's see if the new CI check already catches some missing files 😬

@nicolo-ribaudo nicolo-ribaudo reopened this Mar 7, 2022
@JLHwung JLHwung force-pushed the fix-14242 branch 3 times, most recently from 5f57e57 to 965a1a6 Compare March 7, 2022 16:58
@JLHwung JLHwung mentioned this pull request Mar 7, 2022
Merged
@wvq
Copy link
Copy Markdown

wvq commented Mar 8, 2022

:) It's work's now.

@nicolo-ribaudo
Copy link
Copy Markdown
Member

nicolo-ribaudo commented Mar 8, 2022

I slightly updated the PR title so that it fits in a commit message even with (#14335) at the end :)

@nicolo-ribaudo nicolo-ribaudo changed the title Initialize static/proto Initializers when we seen non-field class members Fix static/proto initializers when there aren't class fields Mar 8, 2022
@nicolo-ribaudo nicolo-ribaudo merged commit 0951316 into babel:main Mar 8, 2022
@nicolo-ribaudo nicolo-ribaudo deleted the fix-14242 branch March 8, 2022 17:27
@github-actions github-actions bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Jun 8, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 8, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@existentialism existentialism existentialism approved these changes

@nicolo-ribaudo nicolo-ribaudo nicolo-ribaudo approved these changes

Assignees

No one assigned

Labels

outdated A closed issue/PR that is archived due to age. Recommended to make a new issue PR: Bug Fix 🐛 A type of pull request used for our changelog categories Spec: Decorators

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@JLHwung @babel-bot @wvq @nicolo-ribaudo @existentialism