feat(lint): add Airbnb and Unicorn eslint packages

[mvorisek]

Add eslint plugin packages with recommended rules to be fixed in separate PRs.

[mvorisek]

This PR (part 1 of #2589) is complete.

[lubber-de]

This is still a confusing change. Please revert. addDerivedValues returns config and config is declared via let, so what's the purpose of creating a temporary const which extends the previous config.... This is confusing.

[mvorisek]

addDerivedValues() was written quite complited. When no/empty config was given, it created it. Why? When such fallback is removed, no need for return.

By the function name I would expect a void method that adds some config values. This is how it is coded now, is that ok?

The change also fixes the mutation of a config returned from an imported module, previously, it was not extended/cloned. IMHO it is a good practise to never mutate anything imported.

[lubber-de]

It is still confusing: Why is configDocs now declared outiside of the exports scope? For nothing else than being extended for the const once.... Why not move the let config = require('../config/docs'), into the module.exports block and keep the functionality as it was before?

[mvorisek]

because it is a good practise to keep imports at the top (and never mutate them) - otherwise eslint with recommended config will warn - otherwise we will need eslint-disable-line

[lubber-de]

ok, understood, but then it should be kept as var instead of let if we get into scope issues otherwise ;)
[Edit]"and never mutated them" -> yes, right, so this case really needs a refactoring, but lets do it clean instead of the const fix ;)

[mvorisek]

If imported on top (with var/let/const), then it is visible to the whole script scope.

[lubber-de]

Then i am still/again confused again why you renamed that into configDocs and did the additional const stuff in module.exports? Because of the undesired mutation? As said this needs a better refactoring. Maybe we are talking too much about a code which is not even used many times (its about serving docs)

Lets get this PR to some point where we can merge it, as said, i would have already merged lots of other rules if they werent bound altogether in combined PRs 😉

[mvorisek]

now part of #2597

see 117dc67, we import the config as configDocs, then create (local/cloned) config config.

[lubber-de]

@mvorisek

Don't get me wrong, i highly appreciate what you are doing here 👍🏼 But you are preparing lots of similiar PRs always containing several rules and if only one of them turns into discussion we never get close into getting most of them included :)

I actually dont want to rewrite ancient (but working) code just because airbnb rules now tell us we should code different.
linting rules are still opinion based. 😉

That said, i am totally fine with many other rules which do not need code structure changes , but please keep the whole var/let/const changes in a separate PR which does nothing else. Thanks.

I am fine with a lot of PRs having a single lint rule. This makes approvement/review much easier and we can concentrate on one topic and merging gets faster as we won't loose time (or me investigating what code, i never touched before, is doing and why the linter thinks it has to be changed).

You are doing a great job these days, thanks a lot! 🙂

[mvorisek]

But you are preparing lots of similiar PRs always containing several rules

I have a "master" PR #2589 which I split and split :)

The older PRs are to be merged later, they hold changes not compatible with Node 12/ (and E11).

Thankfully, I made the changes quite strictly rule by rule so I can reduce the PR size easily.

This PR now adds the linter packages only.

[lubber-de]

LGTM :)

[lubber-de]

@mvorisek
mmh, my local eslint throws an error now after merge..i wonder why the ci didnt?

Error: Failed to load config "airbnb-base" to extend from.
Referenced from: C:\gitrepos\Fomantic-UI\.eslintrc.js

Error: Failed to load config "airbnb-base" to extend from.
Referenced from: C:\gitrepos\Fomantic-UI\.eslintrc.js
    at configInvalidError (C:\gitrepos\Fomantic-UI\node_modules\@eslint\eslintrc\dist\eslintrc.cjs:2627:9)
    at ConfigArrayFactory._loadExtendedShareableConfig (C:\gitrepos\Fomantic-UI\node_modules\@eslint\eslintrc\dist\eslintrc.cjs:3258:23)
    at ConfigArrayFactory._loadExtends (C:\gitrepos\Fomantic-UI\node_modules\@eslint\eslintrc\dist\eslintrc.cjs:3135:25)
    at ConfigArrayFactory._normalizeObjectConfigDataBody (C:\gitrepos\Fomantic-UI\node_modules\@eslint\eslintrc\dist\eslintrc.cjs:3074:25)
    at _normalizeObjectConfigDataBody.next (<anonymous>)
    at ConfigArrayFactory._normalizeObjectConfigData (C:\gitrepos\Fomantic-UI\node_modules\@eslint\eslintrc\dist\eslintrc.cjs:3019:20)
    at _normalizeObjectConfigData.next (<anonymous>)
    at ConfigArrayFactory.loadInDirectory (C:\gitrepos\Fomantic-UI\node_modules\@eslint\eslintrc\dist\eslintrc.cjs:2865:28)
    at CascadingConfigArrayFactory._loadConfigInAncestors (C:\gitrepos\Fomantic-UI\node_modules\@eslint\eslintrc\dist\eslintrc.cjs:3848:46)
    at CascadingConfigArrayFactory.getConfigArrayForFile (C:\gitrepos\Fomantic-UI\node_modules\@eslint\eslintrc\dist\eslintrc.cjs:3769:18)

[lubber-de]

nevermind.... found the issue 😉

[lubber-de]

@mvorisek Another "issue".
Since the last merge my jetbrains IDE eslint moans about me using windows.

Expected linebreaks to be 'LF' but found 'CRLF'

If this is a default rule in airbnb-ruleset, how can we override that? linebreak-style is not set inside our own eslintrc.

my git config is configured to use LF only when pushing but CRLF when checking out on windows (standard git setting under windows "core.autolcrlf=true"). Of course i dont want to see every code line now being an error, so we should override that rule. Not everybody is using a mac or linux for web development :)

[lubber-de]

This seems to be a reliable override ?

rules: {
        'linebreak-style': ['error', process.platform === 'win32' ? 'windows' : 'unix'],
 }

[lubber-de]

If possible and you are in doubt, somebody could accidently push CRLF: Could we restrict the rule to LF inside CI only?

[mvorisek]

LF must be used in all source text files

if there is any text file stored as CRLF, please fix

if you have LF locally, I advise to reconfigure your IDE to LF which is used by 99% projects across many programming languages

if you have locally CRLF, it might come from git autocrlf, disable it, I would not advise to let git transform your text files locally, otheriwse you will hit many problems like eslint wanting LF :)

and I will take care of enforcing LF for JS and LESS as well