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.

Sign up for GitHub

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

Jump to bottom

fix: Parse placeholder for TS namespace #16903

Merged
merged 4 commits into from
Oct 30, 2024
Merged

fix: Parse placeholder for TS namespace #16903

merged 4 commits into from
Oct 30, 2024

Conversation

liuxingbaoyu
Copy link
Member

@liuxingbaoyu liuxingbaoyu commented Oct 13, 2024 •
edited
Loading

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

@liuxingbaoyu liuxingbaoyu added PR: Bug Fix 🐛 A type of pull request used for our changelog categories pkg: parser labels Oct 13, 2024
@babel-bot
Copy link
Collaborator

babel-bot commented Oct 13, 2024 •
edited
Loading

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

@kbjr kbjr mentioned this pull request Oct 13, 2024
Closed
1 task
JLHwung
JLHwung reviewed Oct 15, 2024
View reviewed changes
packages/babel-parser/src/tokenizer/types.ts Outdated
@@ -351,7 +351,7 @@ export const tt = {
} as const;

export function tokenIsIdentifier(token: TokenType): boolean {
return token >= tt._as && token <= tt.name;
return (token >= tt._as && token <= tt.name) || token === tt.placeholder;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we move the placeholder token right after the name token so that we can compare with the placeholder token directly?

JLHwung
JLHwung reviewed Oct 17, 2024
View reviewed changes
packages/babel-parser/src/tokenizer/types.ts
@@ -328,6 +328,9 @@ export const tt = {
_opaque: createKeywordLike("opaque", { startsExpr }),
// end: isFlowInterfaceOrTypeOrOpaque
name: createToken("name", { startsExpr }),

// placeholder plugin
placeholder: createToken("%%", { startsExpr: true }),
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this change will also affect tokenIsLiteralPropertyName, can you add a parser test for placeholder used as a property name?

@liuxingbaoyu liuxingbaoyu force-pushed the fix-placeholder-namespace branch from 5686b19 to 5986582 Compare October 26, 2024 07:33
@nicolo-ribaudo nicolo-ribaudo merged commit 29242d5 into babel:main Oct 30, 2024
54 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

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

@JLHwung JLHwung JLHwung approved these changes

Assignees
No one assigned
Labels
pkg: parser PR: Bug Fix 🐛 A type of pull request used for our changelog categories
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Bug]: Template parser fails on syntactic placeholder for TS namespace name
4 participants
@liuxingbaoyu @babel-bot @JLHwung @nicolo-ribaudo