Visitar URL original
Fix parser `strictMode` option by overlookmotel 路 Pull Request #13548 路 babel/babel 路 GitHub
Skip to content

Fix parser strictMode option - #13548

Merged
JLHwung merged 1 commit into
babel:mainfrom
overlookmotel:parser-strictMode-option
Jul 20, 2021
Merged

JLHwung merged 1 commit into
babel:mainfrom
overlookmotel:parser-strictMode-option

Conversation

@overlookmotel

@overlookmotel overlookmotel commented Jul 11, 2021 •

Copy link
Copy Markdown
Contributor
Q 聽 聽 聽 聽 聽 聽 聽 聽 聽 聽 聽 A
Fixed Issues? No issue
Patch: Bug Fix? Yes
Major: Breaking Change? No
Minor: New Feature? No
Tests Added + Pass? Yes
Documentation PR Link n/a
Any Dependency Changes? No
License MIT

The strictMode option to parse() does not behave as described in the docs.

The docs say:

strictMode: By default, ECMAScript code is parsed as strict only if a "use strict"; directive is present or if the parsed file is an ECMAScript module. Set this option to true to always parse files in strict mode.

The actual behavior is strict = options.strictMode === false ? false : options.sourceType === "module";

Setting the option to true has no effect - contradicting what it says in docs.

This PR fixes this so the option behaves as documented, while also preserving the current (undocumented) behavior of allowing strictMode: false to override sourceType: 'module'.

I have opted to check for true and false explicitly and ignore any other values for the option, rather than accepting other truthy/falsy values. This is intended to make this change less likely to cause unexpected breakage for incorrect configurations e.g. strictMode: "sloppy". But I think anyone who has explicitly said strictMode: true clearly wants that!

By the way, this option is useful in some cases. I ran into it because was trying to parse code that was being run in an eval() which is nested within strict mode code. So it should be parsed as strict mode, but not as a module as there's no import inside eval().

@codesandbox-ci

Copy link
Copy Markdown

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit ffe9d58:

Sandbox Source
babel-repl-custom-plugin Configuration
babel-plugin-multi-config Configuration

@nicolo-ribaudo nicolo-ribaudo added pkg: parser PR: Bug Fix 馃悰 A type of pull request used for our changelog categories labels Jul 19, 2021

@nicolo-ribaudo nicolo-ribaudo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't like this option, but yeah let's at least make it behave correctly.

@sosukesuzuki sosukesuzuki left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We should update TypeScript definition file for this option.

@nicolo-ribaudo

Copy link
Copy Markdown
Member

Uh well, it was already defined as an optional boolean.

@sosukesuzuki

Copy link
Copy Markdown
Member

Oh sorry I missed it

@JLHwung
JLHwung merged commit dd942f9 into babel:main Jul 20, 2021
@overlookmotel

Copy link
Copy Markdown
Contributor Author

Thanks for merging!

@overlookmotel
overlookmotel deleted the parser-strictMode-option branch July 21, 2021 11:31
nicolo-ribaudo pushed a commit to nicolo-ribaudo/babel that referenced this pull request Jul 30, 2021
@github-actions github-actions Bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Oct 21, 2021
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Oct 21, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

outdated A closed issue/PR that is archived due to age. Recommended to make a new issue pkg: parser PR: Bug Fix 馃悰 A type of pull request used for our changelog categories

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants