Update lint hook to only lint files that have been changed for the current branch
- Dominant language
- TypeScript
- Stars
- 413
- Forks
- 308
- Avg merge
- 1d 4h
- Merged PRs (30d)
- 253
Description
The `lint` command takes around 10 seconds to run. This is fine when it's run on CI, but it feels a bit uncomfortable when run as a part of a push.
Ideally, what we want is something like this:
* Full run (check or fix):
* Run ESLint on all `*.js` and `*.ts` files, omitting those in `.gitignore`
* Run Prettier on all `*.json`, `*.md`, and `*.yml` files, omitting those in `.gitignore`, as well as `CHANGELOG.md`s in any packages, `CHANGELOG.old.md`, and `.yarnrc.yml`
* Partial run (changed files only):
* Run ESLint on all _changed_ `*.js` and `*.ts` files, omitting those in `.gitignore`
* Run Prettier on all _changed_ `*.json`, `*.md`, and `*.yml` files, omitting those in `.gitignore`, as well as `CHANGELOG.md`s in any packages, `CHANGELOG.old.md`, and `.yarnrc.yml`
More details below:
---
The Snaps monorepo attempts to reduce the runtime by using [`lint-staged`](https://github.com/okonet/lint-staged) to [only linting files that have been staged](https://github.com/MetaMask/snaps-monorepo/blob/b9c6aec01954e8dfd5bd094e9776b87ca59fabc9/package.json#L37). There are two wrinkles with the approach that the Snaps monorepo takes that would prevent us from copying it to this repo, however.
First, `lint-staged` is designed to be paired with a `pre-commit` hook, but we use a `pre-push` hook instead, so we'd need a different solution. So minimally, we could use something like [`lint-changed`](https://github.com/artsy/lint-changed) instead.
However, `lint-changed` (and `lint-staged`, for that matter) assume that you don't want to customize the set of files that are processed when running a full lint. For ESLint, that's not a problem, as we can get away with asking it to lint the whole project and then use `.gitignore` to filter out undesired files. Prettier, however, doesn't use `.gitignore` by default, so [we tell it to use `.gitignore`, but then we also filter out more files on top of that](https://github.com/MetaMask/core/blob/65af9a440ff957dafdc681cc6ef2929831f62365/package.json#L25).
So if we were to start using `lint-changed`, we'd have to have a separate way of running Prettier. We'd need a way to run file paths that `lint-changed` has reported as changed, but continue to filter out undesired files that aren't covered by `.gitignore`.
The Prettier team has made this somewhat easier by allowing multiple ignore file paths to be passed: https://github.com/prettier/prettier/pull/14332. So we could pull out the list of files that we're excluding to a `.prettierignore` and then instruct the `prettier` command to use that in addition to `.gitignore`.
However, one problem would remain: we'd have to accommodate two ways of calling — and configuring — our lint commands.
Say we were able to reduce `package.json` to something like this:
``` json
{
"scripts": {
"lint": "yarn lint:js && yarn lint:misc --check && yarn constraints",
"lint:fix": "yarn lint:js --fix && yarn lint:misc --write && yarn constraints --fix",
"lint:js": "eslint . --cache --ext js,ts",
"lint:misc": "prettier '**/*.json' '**/*.md' '**/*.yml' --ignore-path .gitignore --ignore-path .prettierignore"
}
}
```
Now say we were to add `lint-changed`. That might look something like this:
``` json
{
"scripts": {
"lint": "yarn lint:js --check && yarn lint:misc --check && yarn constraints",
"lint:fix": "yarn lint:js --fix && yarn lint:misc --write && yarn constraints --fix",
"lint:js": "eslint --cache --ext js,ts .",
"lint:misc": "prettier --ignore-path .gitignore --ignore-path .prettierignore '**/*.json' '**/*.md' '**/*.yml'"
},
"lint-staged": {
"*.{js,ts}": "eslint --cache --ext js,ts",
"*.{json,md,yml}": "prettier --ignore-path .gitignore --ignore-path .prettierignore"
}
}
```
(We don't specify any file paths for `lint-staged` because it fills them in for us.)
Note how we have to specify two ways of running the `eslint` and `prettier` commands, and we have to specify acceptable file extensions twice. It'd be nice if we could just say:
``` json
{
"scripts": {
"lint": "scripts/lint.sh --check",
"lint:fix": "scripts/lint.sh --fix"
},
"lint-staged": {
"*": "scripts/lint.sh --check"
}
}
```
To get this to work, such a lint script would have to:
* figure out which file extensions go with which command, and specify them in the way that the tool understands
* accept a set of file patterns, or else default to `.` if no file patterns were specified, and pass the result off to the tool
So this ticket depends on the release of Prettier 3.0, but it also depends on writing such a lint script.
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.