Skip to content
This repository has been archived by the owner on Feb 18, 2024. It is now read-only.

Monorepo linting improvements #949

Merged
merged 7 commits into from
Jun 13, 2018
Merged

Monorepo linting improvements #949

merged 7 commits into from
Jun 13, 2018

Conversation

edmorley
Copy link
Member

@edmorley edmorley commented Jun 9, 2018

  • Reduces the number of non-style AirBnB rules that are disabled.
  • Removes redundant include from .neutrinorc.js.
  • Adds tests, create-project templates and the repo root .eslintrc.js to the list of monorepo files being linted.
  • Enables --report-unused-disable-directives.

See individual commits for more details.

At some point after this PR we should also deal with the pre-existing issue of having the style AirBnB rules disabled (due to the extends: ['prettier']), but yet we don't actually have Prettier enabled to replace them. However doing so causes a lot more diff churn / need for discussion, so I've left that out of this PR.

edmorley added 3 commits June 9, 2018 15:07
Since `include` is only used by `eslint-loader`, and all linting
for this monorepo occurs via the ESLint CLI, bypassing it.
This also means the `.eslintignore` entry for `.neutrinorc.js` can
be removed, since ESLint doesn't ignore dotfiles specified via globs.

The `lint` script's glob has to be double quoted and escaped, to
ensure it works correctly on Windows.
@edmorley edmorley self-assigned this Jun 9, 2018
edmorley added 4 commits June 9, 2018 20:18
The `vue` dependency was added to fix:
`Unable to resolve path to module 'vue'  import/no-unresolved`

...but will also have the benefit of meaning we get a Renovate PR
when it's time to update the pinned version of Vue in create-project's
version matrix.
Files with extension `.jsx` are now linted, which gives coverage of
the various JSX create-project template files.

(Doing the same for `.vue` turned out to be much more of a pain, since
it requires a different parser/plugin/rules than the ones used by
the AirBnb preset - so not bothering for now.)
@edmorley edmorley requested a review from eliperelman June 9, 2018 19:35
@edmorley edmorley merged commit be78c06 into neutrinojs:master Jun 13, 2018
@edmorley edmorley deleted the monorepo-linting-improvements branch June 13, 2018 16:52
acconrad pushed a commit to acconrad/neutrino-dev that referenced this pull request Jul 13, 2018
* Reduce number of airbnb-base rules that are globally disabled

* Remove redundant lint `include` from .neutrinorc.js

Since `include` is only used by `eslint-loader`, and all linting
for this monorepo occurs via the ESLint CLI, bypassing it.

* Lint repo root .eslintrc.js too (and any other future dotfiles)

This also means the `.eslintignore` entry for `.neutrinorc.js` can
be removed, since ESLint doesn't ignore dotfiles specified via globs.

The `lint` script's glob has to be double quoted and escaped, to
ensure it works correctly on Windows.

* Lint tests too

* Lint the create-projects templates directory too

The `vue` dependency was added to fix:
`Unable to resolve path to module 'vue'  import/no-unresolved`

...but will also have the benefit of meaning we get a Renovate PR
when it's time to update the pinned version of Vue in create-project's
version matrix.

* Lint JSX too

Files with extension `.jsx` are now linted, which gives coverage of
the various JSX create-project template files.

(Doing the same for `.vue` turned out to be much more of a pain, since
it requires a different parser/plugin/rules than the ones used by
the AirBnb preset - so not bothering for now.)

* Use --report-unused-disable-directives
edmorley added a commit that referenced this pull request Jun 20, 2019
…1413)

For React components projects, it's expected that they will not have a
direct dependency on React/React DOM/prop-types, but instead have them
as peer dependencies.

However until now, create-project has not actually configured them in
`peerDependencies` correctly, which causes false positive ESLint errors
from the `import/no-extraneous-dependencies` rule. These false positives
were previously wallpapered over using `eslint-disable`, however that
workaround was regressed in #949, and wasn't the correct fix.

This wasn't caught in CI, since we did not test the combination of
react-components with AirBnB, only with StandardJS.

Note: `eslint-plugin-imports` cache handling is fragile, so existing
projects that have `peerDependencies` manually added will require the
cache (`.eslintcache`) to be removed for the changes to be detected.

Fixes #1388.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

Successfully merging this pull request may close these issues.

2 participants