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

fix: "use strict" should not trigger strict mode in es3. #547

Merged
merged 2 commits into from Apr 28, 2022

Conversation

aladdin-add
Copy link
Member

No description provided.

@aladdin-add aladdin-add force-pushed the eslint-issue-15456 branch 2 times, most recently from 5b853b6 to 089a6fd Compare April 26, 2022 08:48
"value": "use strict",
"raw": "\"use strict\""
},
"directive": "use strict"
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure if this should be reported to Acorn as a possible bug. As far as I know, ES3 didn't have the concept of directive prologues, so maybe no nodes should be parsed as Directive nodes in ES3 mode.

Copy link
Member Author

Choose a reason for hiding this comment

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

👍 sounds a bug in acorn.

Copy link
Member

Choose a reason for hiding this comment

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

Agree. This seems like a bug in Acorn.

Copy link
Member

@mdjermanovic mdjermanovic left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

I think we should merge this (and release it next week) regardless of #547 (comment) to ensure consistent behavior of Espree in regard to whether parsing produces syntax errors or not.

Copy link
Member

@nzakas nzakas left a comment

Choose a reason for hiding this comment

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

Can you also add a test for a function-level directive?

@aladdin-add
Copy link
Member Author

@nzakas added in 7178c4f.

Copy link
Member

@nzakas nzakas left a comment

Choose a reason for hiding this comment

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

LGTM

@nzakas nzakas merged commit 6c718af into main Apr 28, 2022
@nzakas nzakas deleted the eslint-issue-15456 branch April 28, 2022 00:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants