Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make config modular #22

Open
rosshadden opened this issue Jan 6, 2020 · 2 comments
Open

Make config modular #22

rosshadden opened this issue Jan 6, 2020 · 2 comments
Assignees

Comments

@rosshadden
Copy link

Problem

Right now our main and only config in this project is both react-specific and test-specific. It uses the plugins jest, jsx-ally, react, and react-hooks. It also sets the jest, mocha, browser, and node environments to true.

The above has the following implications:

  • all projects that use this config must install peer dependencies, regardless of whether they are relevant to their project
  • all testing globals, like describe and test, are allowed in all code, testing or otherwise
  • all browser globals, like window and document, are allowed in all code, browser or otherwise
  • all node globals, like require and process, are allowed in all code, node or otherwise

Solution

We can make our config modular. We will have a base config file that is universally applicable and env-agnostic. No node config, no browser config, no tests config. We can then have specific environment files that extend these. One for browser, one for specifically react which would extend browser, one for node, one for tests.

The official eslint docs for doing so can be found here.

An example node environment config:

// environments/node.js
module.exports = {
  extends: [
    "@reactioncommerce",
    "plugin:node/recommended"
  ],
  env: {
    "node": true,
    "shared-node-browser": false
  }
};

Using this same pattern we could also make different "modules" which would have config for certain agnostic libraries or scenarios. Things like MongoDB, jsdoc, jest, react, or lodash. For example, consider the popular library lodash. A lodash module would specify the lodash plugin, extend the recommended config, and then override any rules we may desire. This example would look something like this:

// modules/lodash.js
module.exports = {
  plugins: [ "lodash" ],
  extends: [ "plugin:lodash/recommended" ],
  rules: {
    "lodash/prefer-constant": [ 2, false, true ],
    "lodash/chain-style": [ 2, "explicit" ]
  }
};

Projects (including this project which can eat its own dogfood) could then cherry-pick the modules they want, or use our pre-configured environments which should be good for most of our uses.

Breaking changes

This will have ramifications. My desired plan is to rely on semver, where people should not blindly update over a major version without doing their research first.

  • The biggest change will be that react-specific environments will need to specify a slightly longer config path.
  • Another change is that the situation-specific plugins like eslint-plugin-react-hooks will be changed to dependencies in this project and they will never need to manually be installed in other projects. Eslint recommends that plugins be peer dependencies, however when modularizing eslint configs it is more common to make plugins dependencies of the lint config, not peer dependencies. For some examples see Shopify, Facebook, React App, Canonical, Alloy, Supermind, and ES.
@rosshadden rosshadden self-assigned this Jan 6, 2020
@aldeed
Copy link
Contributor

aldeed commented Jan 6, 2020

Seems like a good plan @rosshadden. Some additional thoughts:

  • Using a .eslintrc.js file (https://eslint.org/docs/user-guide/configuring#configuration-file-formats) might be helpful in that it supports dynamic config (for example check whether NODE_ENV is "test" to turn on test globals). I assume there is an equivalent for dynamic config in a plugin package.
  • I vaguely think there was a good reason we did peer deps, but since I can't remember it and others do regular deps, I'm fine with that change. Whatever it was may have changed in the last 2 years.
  • Most likely you could create all new packages for the more specific config presets, and then update this package to depend on all of those, such that this package would have no breaking changes. Deps would move from peer to regular but I don't consider that a breaking change.

@brent-hoover
Copy link
Collaborator

I went to open this ticket but it was already here. This change would be really helpful and for pure node linting the current code has a lot of bloat.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants