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

not compatible with commonjs #6

Closed
despairblue opened this issue Mar 14, 2016 · 6 comments · Fixed by #7 or #15
Closed

not compatible with commonjs #6

despairblue opened this issue Mar 14, 2016 · 6 comments · Fixed by #7 or #15

Comments

@despairblue
Copy link
Contributor

This module won't lint itself for example (this line)

I don't see a way to use this in node or with a commonjs module bundler, since exporting a module is basically mutating the module or exports object. There seems to be no way around that.

// not allowed because they are mutating
modules.exports = 'foo'
exports.foo = 'bar'

// does not export anything at all
exports = 'bar' 

Disabling one rule in every module, on just one line, also seems like a dirty solution.

Maybe I'm missing something.

@dtinth
Copy link

dtinth commented Mar 14, 2016

With CommonJS you export stuff by mutating the exports, while ES6 modules you declare it to be exported. So I think it’s normal. If I were to use this ESLint plugin, I would only use it for a certain directory or file patterns, and try to put as much stuff in it as possible. And only use it with ES6 modules.

I think right now you might be able to get away with const exports = Object.assign(module, { exports: 'foo' }).exports but I guess that’s even a dirtier solution. 😝

@despairblue
Copy link
Contributor Author

Well Object.assign(module, { exports: 'foo' }) works the roundabout way 😉 (the other does not work because of no-undef).

But Object.assign should be prohibited just like array mutation.

module.exports should probably be an exception to the rule, since there is no way around mutating it (except using es6 imports and thus using babel, nothing wrong with it, but using it simply to be able to use an eslint plugin is kind of overhead).

@davidgtonge
Copy link

@despairblue good point about Object.assign. Their should probably me a list of all mutative methods that should at least cause a warning message if not an error.

@ljharb
Copy link

ljharb commented Mar 14, 2016

module.exports = should definitely be an exception to the rule. exports.foo = should not.

@despairblue
Copy link
Contributor Author

Made a PR fixing this.

@jfmengels
Copy link

Seeing as there is no maintenance of the plugin, I've written my own where you can activate this exception.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants