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

documentation: no-unsafe-optional-chaining - add needs ecmaVersion: 2020 #14029

Closed
aavmurphy opened this issue Jan 23, 2021 · 10 comments
Closed

documentation: no-unsafe-optional-chaining - add needs ecmaVersion: 2020 #14029

aavmurphy opened this issue Jan 23, 2021 · 10 comments

Comments

@aavmurphy
Copy link

@aavmurphy aavmurphy commented Jan 23, 2021

The version of ESLint you are using.
7.18.0 (n/a- website doco)

The problem you want to solve.
The no-unsafe-option-chaining rule page ( https://eslint.org/docs/rules/no-unsafe-optional-chaining ) should mention you also need :
parserOptions.ecmaVersion: 2020
(as mentioned here https://eslint.org/blog/2020/07/eslint-v7.5.0-released )

And while you're there, please add to correct examples a simple
obj.foo?.bar

Your take on the correct solution to problem.

Are you willing to submit a pull request to implement this change?

@mdjermanovic
Copy link
Member

@mdjermanovic mdjermanovic commented Jan 23, 2021

Hi @aavmurphy, thanks for the issue!

The no-unsafe-option-chaining rule page ( https://eslint.org/docs/rules/no-unsafe-optional-chaining ) should mention you also need :
parserOptions.ecmaVersion: 2020

The rule itself doesn't require ecmaVersion: 2020, the optional chaining syntax requires that. Also, ESLint can be used with third-party parsers like @babel/eslint-parser and @typescript-eslint/parser which support the latest syntax by default, so there is no need to specify ecmaVersion.

I think that mentioning ecmaVersion in the documentation for a rule makes sense only if the rule auto-fixes code to a newer syntax. For example: prefer-object-spread#when-not-to-use-it

And while you're there, please add to correct examples a simple
obj.foo?.bar

PR to add this example is welcome!

@arminyahya
Copy link
Contributor

@arminyahya arminyahya commented Jan 27, 2021

Hi everyone
I couldn't find any case no-unsafe-optional-chaining auto-fix code.
Do you know any?

@mdjermanovic
Copy link
Member

@mdjermanovic mdjermanovic commented Jan 27, 2021

Hi @arminyahya! This rule isn't auto-fixable, since it warns about possible runtime errors and auto-fixes should never change the behavior of the code. I also don't think there are many cases where the rule could provide a suggestion fix.

@arminyahya
Copy link
Contributor

@arminyahya arminyahya commented Jan 27, 2021

In that case, there is nothing to document. Am i right?

@mdjermanovic
Copy link
Member

@mdjermanovic mdjermanovic commented Jan 27, 2021

Nothing about the autofixing, but it makes sense to add obj.foo?.bar as a correct example for this rule, as @aavmurphy suggested, since it's probably the most common usage of optional chaining.

@arminyahya
Copy link
Contributor

@arminyahya arminyahya commented Jan 28, 2021

Working on this.

@aavmurphy
Copy link
Author

@aavmurphy aavmurphy commented Jan 28, 2021

Thanks for adding the a.b?.c example.

I still think it worth mentioning: use parserOptions.ecmaVersion: 2020 if you're using the default parser.

I was unaware of this config option, so didn't understand why my correct code was erroring with a syntax error until google came up with the eslint blog post which mentions ecmaversion which in turn explains it.

Aside: Some of the other 'correct' options are beginning to look a bit like line noise (and this from a perl programmer). Could almost do with a complexity/obfuscated warning.

mdjermanovic pushed a commit that referenced this issue Jan 29, 2021
…) (#14050)
@nzakas
Copy link
Member

@nzakas nzakas commented Feb 9, 2021

@aavmurphy Thanks, we've already decided against adding that for the reasons already stated. If you have other feedback for the documentation, can you open a separate issue? It will make it easier for us to track. Thanks!

@nzakas
Copy link
Member

@nzakas nzakas commented Feb 9, 2021

@mdjermanovic can this be closed now? It looks like the PR was merged but didn't have a "fixes" tag.

@nzakas nzakas added this to Evaluating in Triage Feb 9, 2021
@mdjermanovic
Copy link
Member

@mdjermanovic mdjermanovic commented Feb 10, 2021

Yes, the accepted documentation update has been merged (we had "refs" instead of "fixes" probably because the PR addresses only part of this request).

@eslint-github-bot eslint-github-bot bot locked and limited conversation to collaborators Aug 10, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Triage
Complete
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants