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

feat(es/parser): Add disallowAssertKeywords option #8913

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

petamoriken
Copy link
Contributor

@petamoriken petamoriken commented May 3, 2024

Description:

V8 will remove support for the assert keywords, so Node.js and Chrome will follow suit.

Hey, quick update on this. Node.js is planning to remove support for assert in v22 (which will be released in April) and Chrome in v126 (which will be released in May).

denoland/deno#17944 (comment)

Disallow the assert keywords in Deno by adding the disallowAssertKeywords option in this PR.

BREAKING CHANGE:

N/A

Related issue (if exists):

#8893 (comment)

Copy link

changeset-bot bot commented May 3, 2024

⚠️ No Changeset found

Latest commit: af8d6f6

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@petamoriken petamoriken force-pushed the feat/disallow-assert-keywords branch from ccefe19 to c813957 Compare May 4, 2024 11:01
@petamoriken petamoriken force-pushed the feat/disallow-assert-keywords branch from c813957 to efbb583 Compare May 5, 2024 07:25
@petamoriken
Copy link
Contributor Author

@kdy1 Where should I write the test codes?

@kdy1
Copy link
Member

kdy1 commented May 5, 2024

You can add a test like

#[test]
fn illegal_language_mode_directive2() {
test_parser(
r#"let f = (a = 0) => { "use strict"; }"#,
Default::default(),
|p| {
let program = p.parse_program()?;
let errors = p.take_errors();
assert_eq!(
errors,
vec![Error::new(
Span {
lo: BytePos(22),
hi: BytePos(35),
ctxt: swc_common::SyntaxContext::empty()
},
crate::parser::SyntaxError::IllegalLanguageModeDirective
)]
);
Ok(program)
},
);
}

@kdy1 kdy1 added this to the Planned milestone May 5, 2024
Copy link
Member

@kdy1 kdy1 left a 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

@petamoriken petamoriken marked this pull request as ready for review May 6, 2024 10:11
@petamoriken petamoriken requested review from a team as code owners May 6, 2024 10:11
}) => disallow_assert_keywords,
#[cfg(feature = "typescript")]
Syntax::Typescript(TsConfig {
disallow_assert_keywords,
Copy link
Member

Choose a reason for hiding this comment

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

Please remove this

Copy link
Member

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

Copy link
Contributor Author

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.

@@ -315,6 +329,9 @@ pub struct TsConfig {
#[serde(default)]
pub decorators: bool,

#[serde(default)]
pub disallow_assert_keywords: bool,
Copy link
Member

Choose a reason for hiding this comment

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

and this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants