jhusain / eslint-plugin-immutable

ESLint plugin to disable all mutation in JavaScript.
Apache License 2.0
912 stars 17 forks source link

not compatible with commonjs #6

Closed despairblue closed 8 years ago

despairblue commented 8 years ago

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 commented 8 years ago

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. :stuck_out_tongue_closed_eyes:

despairblue commented 8 years ago

Well Object.assign(module, { exports: 'foo' }) works the roundabout way :wink: (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 commented 8 years ago

@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 commented 8 years ago

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

despairblue commented 8 years ago

Made a PR fixing this.

jfmengels commented 8 years ago

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