fix(core): always use project level tsconfigs with eslint #4657
Conversation
Nx Cloud ReportCI ran the following commands for commit 757fd7c. Click to see the status, the terminal output, and the build insights. 📂 See all runs for this branch Sent with 💌 from NxCloud. |
|
This improvement is substantial enough that we should think about adding a migration for it. Fortunately, because everything currently works as is, it is not critical to include it straight away, but @FrozenPandaz I will let you make the call on whether or not you want the migration included in this PR, or it should go in a follow up. |
Indeed! This shaved off 2s when linting
I agree, please add the migration in this PR so that we can test it together and it gets released together. |
| "parserOptions": Object { | ||
| "project": Array [ | ||
| "apps/my-app/tsconfig.*?.json", | ||
| expect(eslintConfig).toMatchInlineSnapshot(` |
There was a problem hiding this comment.
This diff is purely the change to make the snapshot to capture the whole config, instead of just overrides
| "parserOptions": Object { | ||
| "project": Array [ | ||
| "libs/my-lib/tsconfig.*?.json", | ||
| expect(eslintConfig).toMatchInlineSnapshot(` |
There was a problem hiding this comment.
This diff is purely the change to make the snapshot to capture the whole config, instead of just overrides
| expect(eslintrcJson.extends).toContain('plugin:cypress/recommended'); | ||
|
|
||
| const eslintrcJson = readJson(tree, 'apps/my-app-e2e/.eslintrc.json'); | ||
| expect(eslintrcJson).toMatchInlineSnapshot(` |
There was a problem hiding this comment.
This diff is purely the change to make the snapshot to capture the whole config, instead of just extends
| expect(packageJson).toMatchObject({ | ||
| devDependencies: { | ||
| 'eslint-plugin-react': expect.anything(), | ||
| 'eslint-plugin-react-hooks': expect.anything(), | ||
| }, | ||
| }); | ||
|
|
||
| const eslintJson = readJsonInTree(tree, '/apps/my-app/.eslintrc.json'); | ||
| expect(eslintJson).toMatchInlineSnapshot(` |
There was a problem hiding this comment.
This diff is purely the change to make the snapshot to capture the whole config, instead of just extends
|
Ready for final review! |
|
What about packages/cypress/src/generators/cypress-project/cypress-project.spec.ts:265, seems a hardcoded value |
Sorry, I'm not sure what you mean.. it's hardcoded but it's a snapshot so jest can update it easily. |
|
This pull request has already been merged/closed. If you experience issues related to these changes, please open a new issue referencing this pull request. |
Current Behavior
Currently, for non-Angular projects, we do not set the
"project"parser option fortypescript-eslinton an Nx project level, and therefore when ESLint runs on a project it is delegating up to the setting at the root.As as result the TypeScript Program that gets created behind the scenes for the linting of applicable Nx projects scales with the size of the workspace instead of the size of the project. For large workspaces this is a significant overhead.
Expected Behavior
This PR changes things to always apply the
"project"parser option on a project level, thus ensuring more efficient lint runs in terms of speed and memory pressure.In testing this on 1 generated repo and 1 real-world (larger but still only "medium sized") workspace I have seen speed improvements of linting all projects be between 25-300%. For large workspaces the improvements should be even greater.
ADDITIONAL NOTES:
As you can see in the commits, before I began to make changes, I also increased the coverage of eslint config file contents so that we have greater confidence in both this change and any future tweaks we make.
In order to get this to work for all project types I also had to fix an existing inconsistency in the
tsconfig.app.jsonfiles of Gatsby and Next projects vs theirtsconfig.spec.jsonand workspace.json configs. For those two types (and only those two types), they included globs for .js(x) everywhere except theirtsconfig.app.json. I walked Victor through this and he agreed we should make it consistent.Related Issue(s)
Fixes #