fix(eslint-import): switch to recommended import settings

[ghost]

This switches to the recommended settings for the eslint import plugin. Their presets are rather conservative (but still very useful), so there shouldn't be anything in here that has to be overridden.

It doesn't add the no-cycle rule, as that rule can be quite resource intensive (it lints node_modules as well by default). We could enable it and ignore node_modules. Then apps could always enable linting that as well if they want it.

Furthermore, this removes the no-unused-modules rule. Viktor and I talked about it in #248. Our conclusion there was to require the user to set ignoreExports (the recommendation in the linked issue). However, if the user has to set that, due to the structure of the rule they basically have to redefine the entire rule anyway. So our shipping the rule isn't very convenient. I think it'd be nicer to just allow them to set it if they want it.

Closes #246

[amcgee]

@ismay would you mind putting in an example of what's covered by the defaults and how someone would override the rules you mention in their .eslintrc? I think a concrete example would help clarify this a little bit.

[ghost]

@ismay would you mind putting in an example of what's covered by the defaults and how someone would override the rules you mention in their .eslintrc? I think a concrete example would help clarify this a little bit.

Sure thing. This is what's in errors:

https://github.com/benmosher/eslint-plugin-import/blob/master/config/errors.js#L1-L14

/**
 * unopinionated config. just the things that are necessarily runtime errors
 * waiting to happen.
 * @type {Object}
 */
module.exports = {
  plugins: ['import'],
  rules: { 'import/no-unresolved': 2
         , 'import/named': 2
         , 'import/namespace': 2
         , 'import/default': 2
         , 'import/export': 2,
         },
}

And this in warnings:

https://github.com/benmosher/eslint-plugin-import/blob/master/config/warnings.js#L1-L12

/**
 * more opinionated config.
 * @type {Object}
 */
module.exports = {
  plugins: ['import'],
  rules: {
    'import/no-named-as-default': 1,
    'import/no-named-as-default-member': 1,
    'import/no-duplicates': 1,
  },
}

If you'd want to enable the no-unused-modules rule for a project, you'd add this to the local eslint rules:

        'import/no-unused-modules': [
            'error',
            {
                unusedExports: true,
                missingExports: true,
                ignoreExports: [
                    '**/*.test.js',
                ],
            },
        ],

Similarly, for disabling any of the rules in any of the presets, for example import/no-named-as-default:

'import/no-named-as-default': 0,

Or did you mean to add the above to the readme? Let me know.

[amcgee]

Thanks, that's very clear for me now!

Would be great to add at least the overflow example to the docs (the overrides page seems a good one, or in FAQ), if you don't mind!

[ghost]

Thanks, that's very clear for me now!

Would be great to add at least the overflow example to the docs (the overrides page seems a good one, or in FAQ), if you don't mind!

Yeah no problem. I could add a generic explanation to the overrides/eslint section, on enabling/disabling eslint rules. Shall I do that? That way it'll be applicable to any situation where people want to customize the rules.

[ghost]

I've added a generic example to the docs, let me know what you think.

[amcgee]

Awesome, looks great!

[dhis2-bot]

🎉 This PR is included in version 7.1.0-alpha.9 🎉

The release is available on:

• npm package (@alpha dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[dhis2-bot]

🎉 This PR is included in version 7.2.0-alpha.1 🎉

The release is available on:

• npm package (@alpha dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[ghost]

@varl @amcgee Should we merge this to master? Or maybe test the alpha on master in a couple of places? I'm not sure if it already is tested somewhere beyond the scheduler. In the scheduler MK II it seems to be doing all right.

[dhis2-bot]

🎉 This PR is included in version 7.2.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀