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

refactor(parser): add TSImportAttributesKeyword as the attributes keyword #5334

Conversation

Dunqing
Copy link
Member

@Dunqing Dunqing commented Aug 30, 2024

related: #4610 (comment)

The attributes keyword has only two determined values: assert and with.

Copy link

graphite-app bot commented Aug 30, 2024

Your org has enabled the Graphite merge queue for merging into main

Add the label “merge” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge. Or use the label “hotfix” to add to the merge queue as a hot fix.

You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link.

@github-actions github-actions bot added A-parser Area - Parser A-ast Area - AST A-codegen Area - Code Generation labels Aug 30, 2024
@Dunqing Dunqing changed the title refactor(parser): add TSImportAttributesKeyword as the attributes keyword refactor(parser): add TSImportAttributesKeyword as the attributes keyword Aug 30, 2024
Copy link
Member Author

Dunqing commented Aug 30, 2024

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @Dunqing and the rest of your teammates on Graphite Graphite

Copy link

codspeed-hq bot commented Aug 30, 2024

CodSpeed Performance Report

Merging #5334 will not alter performance

Comparing 08-30-refactor_parser_add_tsimportattributeskeyword_as_the_attributes_keyword (0fff430) with main (3ae94b8)

Summary

✅ 28 untouched benchmarks

@Boshen
Copy link
Member

Boshen commented Aug 30, 2024

https://github.com/tc39/proposal-import-attributes only allows with, it's going to be another ast change once it lands in stage 4. I'm in favor of keeping this as is until stage 4.

@Dunqing
Copy link
Member Author

Dunqing commented Aug 30, 2024

https://github.com/tc39/proposal-import-attributes only allows with, it's going to be another ast change once it lands in stage 4. I'm in favor of keeping this as is until stage 4.

Yes, I see your point. But TypeScritp at least keep it until TypeScript 6. It could be a long time. I am fine with any decision, feel free to close/merge it

@Boshen
Copy link
Member

Boshen commented Aug 30, 2024

The current behavior is fine, let's have less AST nodes.

@Boshen Boshen closed this Aug 30, 2024
@Boshen Boshen deleted the 08-30-refactor_parser_add_tsimportattributeskeyword_as_the_attributes_keyword branch August 30, 2024 08:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ast Area - AST A-codegen Area - Code Generation A-parser Area - Parser
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants