fix(linter): do not infer lint tasks for projects without files to lint #22944
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
☁️ Nx Cloud ReportCI is running/has finished running commands for commit 088b594. As they complete they will appear below. Click to see the status, the terminal output, and the build insights. 📂 See all runs for this CI Pipeline Execution ✅ Successfully ran 6 targets
Sent with 💌 from NxCloud. |
| if (configDir !== '.' && configDir !== dirname(configFilePath)) { | ||
| filesToExclude.push(`${configDir}/**/*`); | ||
| } |
There was a problem hiding this comment.
I believe there's a possibility where a parent configDir of the configFilePath will get added to the filesToExclude paths. This would ignore everything including the files in the directory as well.
If configFiles is for libs/a/.eslintrc.json
[
`.eslintrc.json`, // This isn't added because it's the root...
`libs/.eslintrc.json`, // This one does get added...
`libs/a/`.eslintrc.json`, // This is the actual config file.
]There was a problem hiding this comment.
Good catch! I only need to ignore the nested ones. I'll amend it and add tests to cover that.
| const lintableFiles = globWithWorkspaceContext(context.workspaceRoot, [ | ||
| join(dir, `**/*.{${options.extensions.join(',')}}`), | ||
| ]); | ||
| return lintableFiles.length > 0; |
There was a problem hiding this comment.
So this never handled if there were lintable files.. but they were all under a child config path right?
There was a problem hiding this comment.
Exactly. It never handled the ESLint ignored file configuration. It did glob for children with specific file extensions, which might or might not have been lintable for the project.
| ...(existsSync(join(workspaceRoot, projectRoot, '.eslintignore')) | ||
| ? ['{projectRoot}/.eslintignore'] | ||
| : []), |
There was a problem hiding this comment.
This includes proj/.eslintignore as an input?
Should it also include anything in a parent directory.. as well as anything in a child directory?
There was a problem hiding this comment.
Only the .eslintignore file at the project root (cwd) applies (https://eslint.org/docs/latest/use/configure/ignore-deprecated#the-eslintignore-file). Other .eslintignore are not used by ESLint.
leosvelperez
force-pushed
the
eslint/plugin-respect-ignored-files
branch
from
78abcf8
to
088b594
Compare
| for (const file of lintableFiles) { | ||
| if (!(await eslint.isPathIgnored(join(context.workspaceRoot, file)))) { | ||
| childProjectRoots.add(childProjectRoot); | ||
| break; | ||
| } | ||
| } |
There was a problem hiding this comment.
We should probably use a Promise.all here. But you can address it in a followup PR.
| ].map((f) => join(configDir, f)), | ||
| nestedEslintRootPatterns.length ? nestedEslintRootPatterns : undefined | ||
| ); | ||
| for (const projectFile of projectFiles) { |
There was a problem hiding this comment.
This loop could also be a Promise.all
|
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
@nx/eslint/plugininfers tasks for projects without files to lint. It doesn't properly consider the ESLint ignored files configuration.Expected Behavior
@nx/eslint/pluginshould not infer tasks for projects without files to lint.Related Issue(s)
Fixes #