-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
feat(es/parser): Add disallowAssertKeywords option
#8913
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
feat(es/parser): Add disallowAssertKeywords option
#8913
Conversation
|
ccefe19 to
c813957
Compare
c813957 to
efbb583
Compare
|
@kdy1 Where should I write the test codes? |
|
You can add a test like swc/crates/swc_ecma_parser/src/parser/tests.rs Lines 321 to 345 in c1115a2
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
swc-bump:
- swc_ecma_parser --breaking
| }) => disallow_assert_keywords, | ||
| #[cfg(feature = "typescript")] | ||
| Syntax::Typescript(TsConfig { | ||
| disallow_assert_keywords, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should unconditionally follow the behavior of tsc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I see. The issue I want to solve with this PR is that it prevents the assert keywords from being used in TypeScript, so I'll try to confirm if tsc can handle it.
| pub decorators: bool, | ||
|
|
||
| #[serde(default)] | ||
| pub disallow_assert_keywords: bool, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and this
|
Since tsc has not decided on a direction, this pull request is closed for the time being. |
Description:
V8 will remove support for the
assertkeywords, so Node.js and Chrome will follow suit.denoland/deno#17944 (comment)
Disallow the
assertkeywords in Deno by adding thedisallowAssertKeywordsoption in this PR.BREAKING CHANGE:
N/A
Related issue (if exists):
#8893 (comment)