refactor: move linting from root to each lib #24424
Conversation
|
Thanks for taking the time to open a PR!
|
jordanpowell88
changed the title
jordanpowell88
force-pushed
the
jordanpowell88/ref-linting
branch
2 times, most recently
from
75b410b
to
9c7dc54
Compare
| @@ -12,7 +12,8 @@ | |||
| "cypress:open": "yarn cypress:run-cypress-in-cypress gulp open --project .", | |||
| "watch": "tsc -w", | |||
| "test": "yarn test-unit", | |||
| "test-unit": "mocha -r ts-node/register/transpile-only --config ./test/.mocharc.js" | |||
| "test-unit": "mocha -r ts-node/register/transpile-only --config ./test/.mocharc.js", | |||
| "lint": "eslint --ext .js,.ts,.json, ." | |||
There was a problem hiding this comment.
Most of these .eslintignore files are 1:1. Can you do eslint --ignore-path -- ../../.eslintignore when these are shared and using custom .eslintignore files when needed?
There was a problem hiding this comment.
Certainly could be ignored from a shared .eslintignore file at the root. My first goal was two-fold:
- move to linting to individual the lib level
- avoid having a massive
.eslintignorefile at the root of the workspace
There was a problem hiding this comment.
What is the benefit of having lots of tiny files?
Ideally these are in individual libs to narrow the scope of executing the lint script
Does this actually mean eslint will have to execute on less files? Is there some perf improvement here?
I don't really see the benefit of having more files - I actually prefer one file, so I can just add all my linting rules there. Right now every package has a slightly different set of rules, if anything wouldn't want to move towards 1 eslint file and one eslintignore file for everything?
There was a problem hiding this comment.
@emilyrohrbough @lmiller1990 The benefit is that later, when we update Lerna (to utilize Nx) we can utilize caching. This way we only have to lint thing that have been affected by changes. Right now we do this once at the root level and therefore we will have to lint essentially every single time on any change
There was a problem hiding this comment.
@jordanpowell88 to use nx we need the additional files or do we just need to lint command in each subpackage/workspace?
There was a problem hiding this comment.
I am not familiar with the intricacies of Nx caching - if this gets us closer to a better monorepo DX, happy to bikeshed my opinion of "all the linting rules in one file" in favor of moving forward.
I wonder if we can have some way to extend a base set of ignored files? Eg a linting package or something could expose it.
There was a problem hiding this comment.
Okay, followed up with @jordanpowell88. We don't actually need separate . eslintignore files for each workspace to have it's own linting script & to use nx caching. Sounds like there are performance implications.
if you change the root estlintignore file it would invalidate the cache and essentially require a re-run of everything to lint (not a huge deal if it doesn't happen often) but if I just change one line for one lib then it still affects performance
|
All for making our linting less complex, but I'm not sure I see the benefit here - if anything, spreading things out seems like more complexity to manage? I could be missing something, though, like some perf or dependency benefits. |
jordanpowell88
force-pushed
the
jordanpowell88/ref-linting
branch
from
a596b6a
to
03c71d9
Compare
User facing changelog
n/a
Additional details
.eslintignorefile. Ideally these are in individual libs to narrow the scope of executing the lint script.Steps to test
yarn lintHow has the user experience changed?
n/a
PR Tasks
cypress-documentation?type definitions?