Consolidating `strict` rule definition between airbnb and airbnb-base

[edmorley]

Hi!

I've spotted some inconsistency/duplication with the strict rule and wondered if this was intentional?

Currently:

• There's the eslint-config-airbnb-base/rules/strict file that declares strict: ['error', 'never'].
• That file is only used in eslint-config-airbnb and not eslint-config-airbnb-base.
• Instead eslint-config-airbnb-base sets strict: 'error' manually in its rules directive (which results in the ESLint default of strict: ['error', 'safe']).
• However both eslint-config-airbnb-base and eslint-config-airbnb set sourceType: 'module', which causes the ESLint strict rule to "disallow[s] strict mode directives, no matter which option is specified" regardless of those options - ie: strict: ['error', 'never'] (docs).

Issues:

• It was my understanding that for the most part, the generic (ie non react, non-AirBnb-as-as-company specific) rules/configuration belong in eslint-config-airbnb-base rather than eslint-config-airbnb, but yet the eslint-config-airbnb-base/rules/strict extends is contrary to that, and confusing to the end user if it actually resulted in different strict mode behaviour.
• However unless people override sourceType: 'module', both eslint-config-airbnb-base and eslint-config-airbnb actually result in the same strict mode behaviour (ie: equivalent of strict: ['error', 'never']) anyway, so in most cases the rule being listed twice is just duplication rather than having any effect.

I think this situation might just be an accidental leftover from the split to the airbnb-base package (see here) and then #637.

As such, to increase consistency I would like to propose:

• Moving eslint-config-airbnb-base/rules/strict from eslint-config-airbnb's extends to eslint-config-airbnb-base's (this won't affect legacy).
• Deleting the manual strict rule definition from eslint-config-airbnb-base's rules.
• Probably bumping the eslint-config-airbnb-base major version in case there's anyone using it who sets sourceType: 'script' manually rather than using legacy, since they would see a difference in behaviour.

If this sounds acceptable, I'm happy to open a PR :-)

[edmorley]

If this sounds acceptable, I'm happy to open a PR :-)

Any thoughts before I open the PR? :-)

[ljharb]

I believe it is intentional as-is.

When using babel, every file is a module, and thus auto-strict - so you never should use a "use strict" directive.

When not using babel, you should have the directive everywhere. When using the base config, it's much more likely that someone might not be using babel (since they won't be using jsx).

(Also, eslint-config-airbnb-base is not "generic", it's "airbnb minus react")

[edmorley]

When not using babel, you should have the directive everywhere. When using the base config, it's much more likely that someone might not be using babel (since they won't be using jsx).

True, but from the OP:

However both eslint-config-airbnb-base and eslint-config-airbnb set sourceType: 'module'

...so even people using base already have module mode enabled.

[ljharb]

That is true - hmm.

OK, so, here's the changes I think should be made:

1. [base] the legacy entry point should enable strict with safe (semver-major)
2. [base] the index entry point should import rules/strict. (semver-major)
3. the main package no longer needs to import rules/strict (semver-patch)

A PR that does the base and main changes each in one commit (commit message convention: [eslint config] $message and [eslint config] [base] $message) would be great :-)

[edmorley]

A PR that does the base and main changes each in one commit (commit message convention: [eslint config] $message and [eslint config] [base] $message) would be great :-)

Sorry for the delay; have opened #1962 :-)

[marcospgp]

@ljharb said

When not using babel, you should have the directive everywhere. When using the base config, it's much more likely that someone might not be using babel (since they won't be using jsx).

Which is true. I use the base config and was surprised to see "use strict"; was no longer allowed after updating to the latest major version.

But this made me realize that the airbnb style guide

[...] assumes you are using Babel, and requires that you use babel-preset-airbnb or the equivalent. It also assumes you are installing shims/polyfills in your app, with airbnb-browser-shims or the equivalent.

So I get the idea it shouldn't really be used for Node.js and am thus going to switch to something else for that.

Posting here so you guys are aware of this part of your user base 😄