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

Enforce comparing `typeof` expressions against string literals #629

Closed
feross opened this Issue Sep 16, 2016 · 8 comments

Comments

3 participants
@feross
Copy link
Member

commented Sep 16, 2016

I propose adding { "requireStringLiterals": true } to the valid-typeof rule that we already enforce.

This requires typeof expressions to only be compared to string literals, and disallows comparisons to any other value.

Examples of incorrect code:

typeof foo === undefined
typeof bar == Object
typeof baz === 'strnig'
typeof qux === 'some invalid type'
typeof baz === anotherVariable
typeof foo == 5
typeof bar === typeof qux

Examples of correct code:

typeof foo === 'undefined'
typeof bar == 'object'
typeof baz === 'string'

http://eslint.org/docs/rules/valid-typeof

@feross feross added the enhancement label Sep 16, 2016

@feross

This comment has been minimized.

Copy link
Member Author

commented Sep 16, 2016

Essentially no ecosystem impact:

# tests 422
# pass  421
# fail  1

@feross feross added this to the standard v9 milestone Sep 16, 2016

@Flet

This comment has been minimized.

Copy link
Member

commented Sep 18, 2016

:shipit:

@dcousens

This comment has been minimized.

Copy link
Member

commented Sep 18, 2016

Make it so

@dcousens

This comment has been minimized.

Copy link
Member

commented Sep 18, 2016

Why not standard <9 too?

@feross

This comment has been minimized.

Copy link
Member Author

commented Sep 18, 2016

@dcousens I fear users will get fatigue from too many releases of standard. Rule changes in a non-major version are especially onerous.

Also, frankly, it's a lot of work to do a release. It takes at least an hour to properly test everything, write the changelog, update the various packages, etc. I'd prefer to hold back most of these rule changes until standard v9.

@dcousens

This comment has been minimized.

Copy link
Member

commented Sep 19, 2016

@feross I feel like we don't thank you enough. Thanks so much for the work, totally understandable. Looking forward to v9.0 😄 👍

@feross

This comment has been minimized.

Copy link
Member Author

commented Sep 19, 2016

@dcousens Thanks. I appreciate your contributions, too! It's really nice to not be in this all alone. 😄

feross added a commit to standard/eslint-config-standard that referenced this issue Feb 9, 2017

Enforce comparing `typeof` expressions against string literals (valid…
…-typeof)

Adds { "requireStringLiterals": true } to the 'valid-typeof' rule that
we already enforce.

Fixes: standard/standard#629
@feross

This comment has been minimized.

Copy link
Member Author

commented Feb 9, 2017

This will be part of standard v9. Only one repo was impacted and it was doing this:

const OBJ = 'object'

module.exports = function merge (a, b, checked) {
  if (checked || typeof a === OBJ) {

Can't see a reason that's preferable to typeof a === 'object', so not too worried about this case.

@feross feross closed this Feb 9, 2017

feross added a commit to standard/eslint-config-standard that referenced this issue Feb 9, 2017

Enforce comparing `typeof` expressions against string literals (valid…
…-typeof)

Adds { "requireStringLiterals": true } to the 'valid-typeof' rule that
we already enforce.

Fixes: standard/standard#629

@lock lock bot locked as resolved and limited conversation to collaborators May 10, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.