-
-
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
Ignore CommonJS exports in the named
rule
#268
Comments
I'd like to see the |
So So you should be able to set the following setting in an ---
settings:
import/ignore:
- node_modules # this is the default but will be removed if the setting is explicitly defined
- helpers/server It is a setting instead of a rule option so that it applies to all rules that parse external modules. |
Also, it's currenly implemented as a regex on the fully resolved path; I think it might make more sense as something more glob-like. I think you may have some good perspective on this, looking forward to your feedback. |
@benmosher I don't want to manually ignore paths. I want to ignore anything CommonJS. With the ignore option I would have to add ignore info to every project instead of just to my shareable config. |
I'm working on including this plugin in XO which is a almost zero-config linter. Requiring config for every project would mean I would have to disable this rule for good. |
Yeah, I hear that. Initially it was done like this because parsing is expensive, and things like I don't know if you've been following the discussions in node-eps about determining whether a file should be parsed as a module or as a script, but all the excitement there is around exactly this problem. That said, I recently added a regex that checks for anything that looks like an ES6 export to do the ignore- |
Yeah, that would be good enough for now until the node-eps discussion is resolved. I define all my |
We’ve stumbled upon the same problem where this plugin complains when I var k = require('fs').readFileSync(path, 'utf8')
if (/module\.exports/.test(k)) {
console.log('Skipping', path, 'as it is CommonJS module');return true
} |
@dtinth yep, that's more or less what I expect to implement to resolve this. Just need to test and figure out how to introduce as a non-breaking change. |
FWIW, planning to use UnambiguousJavascriptGrammar-style module detection in v2. So instead of looking for CJS, if your module has no imports or exports, it will be ignored. Would love to hear dissenting opinions, if any. Feels like an easy win, so I'm not sure if I'm missing something obvious in my excitement. |
@benmosher Sounds great! |
AVA comes with ES2015 using Babel built-in for tests, but we don't transpile the source files. This results in the tests using
import
, while helpers and sources usesrequire()
andmodule.exports
.I ran the rule in my
got
project which usesmodule.exports
, butimport
in the tests:The import: https://github.com/sindresorhus/got/blob/67ee190881f4ba9c498708dc41c1d71c5b6039a2/test/arguments.js#L3
The export: https://github.com/sindresorhus/got/blob/67ee190881f4ba9c498708dc41c1d71c5b6039a2/test/helpers/server.js#L8
I think it would make sense to either ignore
exports.foo
/module.exports
or provide an option for it.Previous discussion: #30
// @jfmengels @jamestalmage
The text was updated successfully, but these errors were encountered: