X Tutup
Skip to content

[babel 8] Rename TSImportType.argument to .source#17610

Merged
nicolo-ribaudo merged 4 commits intobabel:mainfrom
nicolo-ribaudo:tsimporttype-source
Nov 28, 2025
Merged

[babel 8] Rename TSImportType.argument to .source#17610
nicolo-ribaudo merged 4 commits intobabel:mainfrom
nicolo-ribaudo:tsimporttype-source

Conversation

@nicolo-ribaudo
Copy link
Member

@nicolo-ribaudo nicolo-ribaudo commented Nov 26, 2025

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

@nicolo-ribaudo nicolo-ribaudo added the PR: Breaking Change 💥 A type of pull request used for our changelog categories for next major release label Nov 26, 2025
@babel-bot
Copy link
Collaborator

babel-bot commented Nov 26, 2025 •
edited
Loading

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

@pkg-pr-new
Copy link

pkg-pr-new bot commented Nov 26, 2025 •
edited
Loading

Open in StackBlitz

commit: 7e4441f

@nicolo-ribaudo
Copy link
Member Author

nicolo-ribaudo commented Nov 26, 2025

@fisker Any chance you could update prettier to check for the new property too? :)

@fisker
Copy link
Contributor

fisker commented Nov 26, 2025

Prettier already use .source, prettier/prettier#18319
The failure is expected, since .argument doesn't exists in visitorKeys anymore, please remove this line https://github.com/prettier/prettier/blob/bbdec54a41e20c8e363fd9bf3aad163b156d509f/src/language-js/traverse/visitor-keys.evaluate.js#L49

@fisker
Copy link
Contributor

fisker commented Nov 26, 2025 •
edited
Loading

Or do you prefer add an env flag to bypass these checks when running test?

https://github.com/prettier/prettier/blob/bbdec54a41e20c8e363fd9bf3aad163b156d509f/src/language-js/traverse/utilities.js#L31-L33

https://github.com/prettier/prettier/blob/bbdec54a41e20c8e363fd9bf3aad163b156d509f/src/language-js/traverse/utilities.js#L37-L39

https://github.com/prettier/prettier/blob/bbdec54a41e20c8e363fd9bf3aad163b156d509f/src/language-js/traverse/utilities.js#L57-59

fisker
fisker reviewed Nov 26, 2025
View reviewed changes
packages/babel-parser/test/fixtures/estree/typescript/import-dot-this/output.json Outdated
"start":41,"end":59,"loc":{"start":{"line":2,"column":7},"end":{"line":2,"column":25}},
"argument": {
"source": {
"type": "TSLiteralType",
Copy link
Contributor

@fisker fisker Nov 26, 2025

Choose a reason for hiding this comment

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

.source should be StringLiteral

fisker
fisker reviewed Nov 26, 2025
View reviewed changes
packages/babel-types/src/definitions/typescript.ts
: validateType("StringLiteral"),
...(process.env.BABEL_8_BREAKING
? { source: validateType("TSLiteralType") }
: { argument: validateType("StringLiteral") }),
Copy link
Contributor

@fisker fisker Nov 26, 2025

Choose a reason for hiding this comment

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

Did some previous change in v8 created TSLiteralType? We need revert that change.

@nicolo-ribaudo
Copy link
Member Author

nicolo-ribaudo commented Nov 26, 2025

It was #17046, to align with TS-ESLint. I'll undo it now that TS-ESLint changed their AST shape.

fisker
fisker approved these changes Nov 26, 2025
View reviewed changes
Copy link
Contributor

@fisker fisker left a comment

Choose a reason for hiding this comment

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

Great work! Thanks!

@fisker
Copy link
Contributor

fisker commented Nov 26, 2025

Or do you prefer add an env flag to bypass these checks when running test?

prettier/prettier@bbdec54/src/language-js/traverse/utilities.js#L31-L33

prettier/prettier@bbdec54/src/language-js/traverse/utilities.js#L37-L39

prettier/prettier@bbdec54/src/language-js/traverse/utilities.js#L57-59

I don't want to cause trouble for you, I can make this change if you want. I added these checks to make sure our visitor keys logic is up to date.

fisker
fisker reviewed Nov 26, 2025
View reviewed changes
packages/babel-parser/src/plugins/typescript/index.ts
if (process.env.BABEL_8_BREAKING) {
// Consume as an non-conditional type so that we can recover from this error
node.argument = this.tsParseNonConditionalType() as any;
node.source = this.tsParseNonConditionalType() as any;
Copy link
Contributor

@fisker fisker Nov 26, 2025

Choose a reason for hiding this comment

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

This is for error revovery, right? Should it use the one same as below? super.parseExprAtom.

Copy link
Member Author

@nicolo-ribaudo nicolo-ribaudo Nov 26, 2025

Choose a reason for hiding this comment

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

It's mostly so that we parse in types mode rather than in values mode.

@nicolo-ribaudo
Copy link
Member Author

nicolo-ribaudo commented Nov 26, 2025

I don't want to cause trouble for you, I can make this change if you want. I added these checks to make sure our visitor keys logic is up to date.

Nah it's fine, we already patch prettier's tests anyway, I didn't realize it was easy for this one too.

liuxingbaoyu
liuxingbaoyu approved these changes Nov 27, 2025
View reviewed changes
Copy link
Member

@liuxingbaoyu liuxingbaoyu left a comment

Choose a reason for hiding this comment

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

It appears that typescript-eslint has been released.

@JLHwung
Copy link
Contributor

JLHwung commented Nov 27, 2025

It appears that typescript-eslint has been released.

Yes and we should bump the typescript-eslint version defined here:

babel/eslint/babel-eslint-parser/package.json

Line 54 in a03e2b6

"typescript-eslint": "8.39.1"

@nicolo-ribaudo nicolo-ribaudo force-pushed the tsimporttype-source branch 2 times, most recently from c4e6119 to 44f2b6a Compare November 28, 2025 11:02
@nicolo-ribaudo nicolo-ribaudo mentioned this pull request Nov 28, 2025
Merged
@nicolo-ribaudo nicolo-ribaudo merged commit beea88c into babel:main Nov 28, 2025
138 of 142 checks passed
@nicolo-ribaudo nicolo-ribaudo deleted the tsimporttype-source branch November 28, 2025 11:41
@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 Feb 28, 2026
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 28, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@JLHwung JLHwung JLHwung approved these changes

@liuxingbaoyu liuxingbaoyu liuxingbaoyu approved these changes

+1 more reviewer

@fisker fisker fisker approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Move TSImportType.argument to .source in v8

5 participants

@nicolo-ribaudo @babel-bot @fisker @JLHwung @liuxingbaoyu
X Tutup