Eslint-plugin-import: Add `allow` option to `no-nodejs-modules`

[ljharb]

The no-nodejs-modules rule is very useful, but some modules are fine to import - like events - due to a good browserify shim.

I'd like the ability to add exceptions to this rule.

[jfmengels]

Wouldn't be very hard to implement. But does this occur very often? If not, a eslint-disable comment could be the best option.

[benmosher]

I had the same initial gut reaction as @jfmengels, though that said I can imagine making a browserify shared config with this rule configured with a default whitelist. (crypto comes to mind as well as one that works fine in the browser.)

Also, I'd move to call the option whitelist, barring precedent from other rules (in this plugin or elsewhere).

Would be an easy PR for someone. @jfmengels do you think it hurts to have this?

[jfmengels]

Just getting the feeling that for a lot of rules there are a lot of requests to have exceptions. It's a recurring theme.
But no, I think it's fine here.

:+1: for whitelist

[benmosher]

Yeah, I am getting that feeling too, in general.

This case is explicit, pragmatic, and obvious, though.

I can imagine one could argue for an eslint-plugin-browserify to handle this, maybe, but that would be on the pedantic side.

(linked because a quick web search found it to exist, by the law of npm that there is an npm package for every combination of two popular projects that support plugins, lol. 😎)

[benmosher]

Updated title to match discussion, @ljharb feel free to voice disagreement, just wanted to make sure possible PR submitters don't miss the discussion.

[ljharb]

I'd prefer a more neutral term that lacks the connotation of "whitelist".

[benmosher]

Fair enough. allow?

[ljharb]

Sounds great! Fwiw, this isn't just for browserify, anyone who'd set up a custom alias for bundling, in webpacking as well, may still want this ability.

[graingert]

This should allow by default anything in node-libs-browser

It would be good to have presets for this, based on the table at https://github.com/webpack/node-libs-browser:

hasImplementation: true (anything with a value for has implementation)
hasMock: false (anything with a value for has mock)

[benmosher]

@graingert certainly there should be an easy way to configure that, i.e. allow: 'node-libs-browser' though I think it might should ignore everything by default. Maybe not, though, if the majority use case is for browser-bundling.

[graingert]

might should maybe not? I don't understand :(

[benmosher]

restatement: it's a breaking change not to ignore everything by default, but I think you're probably right in that in v2 it should default to allowing node-libs-browser through, for convenience's sake.

[benmosher]

added the semver-major label so I remember to make an item to update the default behavior over in v2