Skip to content

[Babel 8] Create TSEnumBody for TSEnumDeclaration#16979

Merged
JLHwung merged 20 commits into
babel:mainfrom
JLHwung:ts-enum-body
Jan 9, 2025
Merged

[Babel 8] Create TSEnumBody for TSEnumDeclaration#16979
JLHwung merged 20 commits into
babel:mainfrom
JLHwung:ts-enum-body

Conversation

@JLHwung

@JLHwung JLHwung commented Nov 27, 2024

Copy link
Copy Markdown
Contributor
Q                       A
Fixed Issues? #16679
Patch: Bug Fix?
Major: Breaking Change?
Minor: New Feature?
Tests Added + Pass? Yes
Documentation PR Link babel/website#3018
Any Dependency Changes?
License MIT

Updated the TSEnum AST per typescript-eslint/typescript-eslint#8920.

Added a new script to automatically create Babel 7 AST test cases. I can't recall whether we already have such script in the repo: I have tried to search within the codebase but no luck, so I come up with a new script.

To fix the typing errors, this PR also introduces the TSEnumBody AST type to Babel 7. However, the parser will not generate TSEnumBody and the generator will not recognize TSEnumBody even if users manually assign body to TSEnumDeclaration. So this PR should not break Babel 7 users.

@JLHwung JLHwung added the PR: Breaking Change 💥 A type of pull request used for our changelog categories for next major release label Nov 27, 2024
Comment thread packages/babel-parser/scripts/generate-babel-7-ast-tests.mjs
@babel-bot

babel-bot commented Nov 27, 2024

Copy link
Copy Markdown
Collaborator

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

if (process.env.BABEL_8_BREAKING) {
node.body = this.tsParseEnumBody();
} else {
this.expect(tt.braceL);

@JLHwung JLHwung Nov 27, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We didn't reuse the tsParseEnumBody function for Babel 7 because it will otherwise remove comments between the enum identifier and the enum body as an EnumBody node will be created but never used.

Comment thread scripts/generate-babel-7-tests.mjs
@JLHwung JLHwung marked this pull request as draft November 28, 2024 16:48
@nicolo-ribaudo

Copy link
Copy Markdown
Member

I always did it manually, this script seems useful :)

@JLHwung JLHwung marked this pull request as ready for review November 28, 2024 17:42
@JLHwung

JLHwung commented Nov 28, 2024

Copy link
Copy Markdown
Contributor Author

I always did it manually, this script seems useful :)

Thanks, I should have just created this script from the day one we introduced BABEL_8_BREAKING env. To the lost copy-paste time.

rm tests/format/typescript/conformance/internalModules/importDeclarations/format.test.js
rm tests/format/typescript/keywords/format.test.js
rm tests/format/typescript/declare/format.test.js
rm tests/format/typescript/const/format.test.js

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

/cc @fisker These tests will be impacted by this AST change.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks!

@nicolo-ribaudo nicolo-ribaudo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Left a minor comment, but overall looks good

Comment thread packages/babel-generator/src/generators/typescript.ts Outdated
this.printList(node.members, {
indent: true,
statement: true,
// TODO: Default to false for consistency with everything else

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

While we are changing this code, could you make this default to process.env.BABEL_8_BREAKING ? false : true? :)

const: validateOptional(bool),
id: validateType("Identifier"),
members: validateArrayOfType("TSEnumMember"),
initializer: validateOptionalType("Expression"),

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The initializer field is also removed in Babel 8. The TS enum declaration does not support initializer (enum { ... } = initializer) and the babel parser does not use this field either. It probably refers to the initializer property in the TSEnumMember.

@JLHwung

JLHwung commented Dec 6, 2024

Copy link
Copy Markdown
Contributor Author

Rebased without any further changes. I will test this branch on typescript-eslint and see if we have any other AST differences.

@nicolo-ribaudo

nicolo-ribaudo commented Dec 6, 2024

Copy link
Copy Markdown
Member

You can use typescript-eslint/typescript-eslint#10452 to help with that. There are also some difference about optional properties, but those are probably fine.

@JLHwung

JLHwung commented Dec 6, 2024

Copy link
Copy Markdown
Contributor Author

Rebased without any further changes. I will test this branch on typescript-eslint and see if we have any other AST differences.

Added 3 todo items in #16679. I will add more later today. Please subscribe that issue.

@JLHwung JLHwung changed the title Create TSEnumBody for TS EnumDeclaration [Babel 8] Create TSEnumBody for TSEnumDeclaration Jan 9, 2025
@JLHwung JLHwung merged commit d35794e into babel:main Jan 9, 2025
@JLHwung JLHwung deleted the ts-enum-body branch January 9, 2025 20:34
laine-hallot pushed a commit to laine-hallot/uwu-parser that referenced this pull request Mar 31, 2025
* update babel-types definitions

* breaking: create TSEnumBody for Babel 8

* update test fixtures

* breaking: remove unused initializer field

* fix typings

* print TSEnumBody

* support Babel 8 AST in typescript transform

* fix: inherit enum member's comments

* add TSEnumBody printer method

* polish: adjust inner comments for TSEnumDeclaration

* fix printer test

* define TSEnumBody for Babel 7

* update artifacts

* fix: adapt to Babel 8 AST

* purge failing prettier test due to the AST change

* Update packages/babel-generator/src/generators/typescript.ts

Co-authored-by: Nicolò Ribaudo <hello@nicr.dev>

* fix typings

* print trailing comma only according to original source

* update test fixtures

* modify according to the changed interface

---------

Co-authored-by: Nicolò Ribaudo <hello@nicr.dev>
@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 Apr 11, 2025
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Apr 11, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

outdated A closed issue/PR that is archived due to age. Recommended to make a new issue PR: Breaking Change 💥 A type of pull request used for our changelog categories for next major release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants