-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Add allow
option to no-nodejs-modules
rule (fixes #452)
#509
Conversation
Thanks, LGTM! |
1 similar comment
@@ -2,7 +2,7 @@ import importType from '../core/importType' | |||
import isStaticRequire from '../core/staticRequire' | |||
|
|||
function reportIfMissing(context, node, allowed, name) { | |||
if (!allowed.includes(name) && importType(name, context) === 'builtin') { | |||
if (allowed.indexOf(name) === -1 && importType(name, context) === 'builtin') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, bummer, I also thought Node v4 had includes
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not that surprising actually, since it's not even ES2015, but an ES2016 feature. I thought Babel would actually take care of this, but I guess was too hopeful (or it's just something to add). Anyway indexOf
is fine.
I was thinking presets (like @graingert suggested) would be a good idea, but that can be a responsibility of shared configs. LGTM. 😎 |
Add
allow
option tono-nodejs-modules
rule (fixes #452)cc @ljharb