feat: remove support for ignore files in FlatESLint #16355

mdjermanovic · 2022-09-27T21:48:24Z

Prerequisites checklist

I have read the contributing guidelines.

What is the purpose of this pull request? (put an "X" next to an item)

[ ] Documentation update
[ ] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofix to a rule
[ ] Add a CLI option
[ ] Add something to the core
[x] Other, please explain:

Refs #16264. Removes everything mentioned except for the ignorePatterns constructor option. I can do that in a separate PR when we are certain that the option should be removed.

What changes did you make? (Give an overview)

Removed all support for separate ignore files.
- Removed loading .eslintignore files.
- Removed ignorePath constructor option.
- Removed --ignore-path CLI option.
Removed interpreting --ignore-patterns and ignorePatterns as .gitignore-style patterns.

Is there anything you'd like reviewers to focus on?

netlify · 2022-09-27T21:48:29Z

✅ Deploy Preview for docs-eslint ready!

Name	Link
🔨 Latest commit	`ebaec07`
🔍 Latest deploy log	https://app.netlify.com/sites/docs-eslint/deploys/63336fa9d85290000ab1c728
😎 Deploy Preview	https://deploy-preview-16355--docs-eslint.netlify.app/user-guide/configuring/configuration-files-new
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

mdjermanovic · 2022-09-27T21:55:46Z

Makefile.js

     * There is a separate command `target.lintDocsJS` for linting JavaScript files in the `docs` directory.
     */
    echo("Validating JavaScript files");
-    lastReturn = exec(`${ESLINT}${fix ? "--fix" : ""} . --ignore-pattern docs`);
+    lastReturn = exec(`${ESLINT}${fix ? "--fix" : ""} . --ignore-pattern "docs/**"`);


I've updated this to be how we expect ignore patterns to work, but it would work the same without this change because of #16354.

mdjermanovic · 2022-09-27T22:01:53Z

tests/lib/cli.js

                    it(`should not process any files with configType:${configType}`, async () => {
                        const ignoredFile = getFixturePath("cli/syntax-error.js");
+                        const ignorePathOption = useFlatConfig
+                            ? ""
+                            : "--ignore-path .eslintignore_empty";
                        const filePath = getFixturePath("cli/passing.js");
-                        const exit = await cli.execute(`--ignore-pattern cli/ ${ignoredFile} ${filePath}`, null, useFlatConfig);
+                        const ignorePattern = useFlatConfig ? "cli/**" : "cli/";
+                        const exit = await cli.execute(
+                            `--ignore-pattern ${ignorePattern} ${ignorePathOption} ${ignoredFile} ${filePath}`, null, useFlatConfig
+                        );


This test was not working as intended because all the files were already ignored by .eslintignore, so --ignore-pattern actually had no effect.

mdjermanovic · 2022-09-27T22:09:51Z

tests/lib/eslint/flat-eslint.js

-        describe("with .eslintignore file or package.json file", () => {
-            it("should load .eslintignore from cwd when explicitly passed", async () => {
-                const cwd = getFixturePath("ignored-paths");
-                const engine = new FlatESLint({ cwd });
-
-                // `${cwd}/.eslintignore` includes `sampleignorepattern`.
-                assert(await engine.isPathIgnored("sampleignorepattern"));
-            });
-
-        });


This test was not working as intended, because sampleignorepattern file does not match any config entries, so it was ignored regardless of .eslintignore.

nzakas

LGTM. Just want to give others a chance to review before merging.

nzakas · 2022-09-29T18:41:03Z

eslint.config.js

+            "tmp/**",
+            "tools/internal-rules/node_modules/**",
+            "**/test.js",
+            "!**/.eslintrc.js"


Just double-checking: this still doesn't unignore .eslintrc.js correct?

Correct. When doing a glob search with a pattern that doesn't have an explicit dot that matches leading dots in file names, .eslintrc.js will still be ignored because globby doesn't support negative patterns in ignore (in fact, I believe it removes leading ! so this actually ignores **/.eslintrc.js), and in general globby's dotfiles matching behavior seems to depend only on the content of search patterns.

On the other hand, when directly using our config array to evaluate whether or not .eslintrc.js is ignored (e.g., when passing a file path), it's not ignored regardless of this line. This behavior needs further analysis (#16265).

Overall, this line currently does not affect anything, but it may when we fix #16265 so I've left it in for now.

That’s what I figured. 👍

This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [eslint](https://eslint.org) ([source](https://github.com/eslint/eslint)) | devDependencies | minor | [`8.24.0` -> `8.25.0`](https://renovatebot.com/diffs/npm/eslint/8.24.0/8.25.0) | --- ### Release Notes <details> <summary>eslint/eslint</summary> ### [`v8.25.0`](https://github.com/eslint/eslint/releases/tag/v8.25.0) [Compare Source](eslint/eslint@v8.24.0...v8.25.0) #### Features - [`173e820`](eslint/eslint@173e820) feat: Pass --max-warnings value to formatters ([#16348](eslint/eslint#16348)) (Brandon Mills) - [`6964cb1`](eslint/eslint@6964cb1) feat: remove support for ignore files in FlatESLint ([#16355](eslint/eslint#16355)) (Milos Djermanovic) - [`1cc4b3a`](eslint/eslint@1cc4b3a) feat: `id-length` counts graphemes instead of code units ([#16321](eslint/eslint#16321)) (Sosuke Suzuki) #### Documentation - [`90c6028`](eslint/eslint@90c6028) docs: Conflicting fixes ([#16366](eslint/eslint#16366)) (Ben Perlmutter) - [`5a3fe70`](eslint/eslint@5a3fe70) docs: Add VS to integrations page ([#16381](eslint/eslint#16381)) (Maria José Solano) - [`49bd1e5`](eslint/eslint@49bd1e5) docs: remove unused link definitions ([#16376](eslint/eslint#16376)) (Nick Schonning) - [`3bd380d`](eslint/eslint@3bd380d) docs: typo cleanups for docs ([#16374](eslint/eslint#16374)) (Nick Schonning) - [`b3a0837`](eslint/eslint@b3a0837) docs: remove duplicate words ([#16378](eslint/eslint#16378)) (Nick Schonning) - [`a682562`](eslint/eslint@a682562) docs: add `BigInt` to `new-cap` docs ([#16362](eslint/eslint#16362)) (Sosuke Suzuki) - [`f6d57fb`](eslint/eslint@f6d57fb) docs: Update docs README ([#16352](eslint/eslint#16352)) (Ben Perlmutter) - [`7214347`](eslint/eslint@7214347) docs: fix logical-assignment-operators option typo ([#16346](eslint/eslint#16346)) (Jonathan Wilsson) #### Chores - [`1f78594`](eslint/eslint@1f78594) chore: upgrade [@eslint/eslintrc](https://github.com/eslint/eslintrc)[@1](https://github.com/1).3.3 ([#16397](eslint/eslint#16397)) (Milos Djermanovic) - [`8476a9b`](eslint/eslint@8476a9b) chore: Remove CODEOWNERS ([#16375](eslint/eslint#16375)) (Nick Schonning) - [`720ff75`](eslint/eslint@720ff75) chore: use "ci" for Dependabot commit message ([#16377](eslint/eslint#16377)) (Nick Schonning) - [`42f5479`](eslint/eslint@42f5479) chore: bump actions/stale from 5 to 6 ([#16350](eslint/eslint#16350)) (dependabot\[bot]) - [`e5e9e27`](eslint/eslint@e5e9e27) chore: remove `jsdoc` dev dependency ([#16344](eslint/eslint#16344)) (Milos Djermanovic) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, click this checkbox. --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).  Co-authored-by: cabr2-bot <cabr2.help@gmail.com> Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1577 Reviewed-by: crapStone <crapstone@noreply.codeberg.org> Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org> Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>

jedwards1211 · 2022-10-17T15:54:02Z

This surprised me and needs to be explicitly documented, #16433

mdjermanovic added 7 commits September 25, 2022 23:37

do not use gitignoreToMinimatch on --ignore-pattern

f689141

remove .eslintignore lookup

89a13b1

remove flat-eslint tests that assume .eslintignore lookup

8504abc

remove --ignore-path CLI option

92ca045

remove ignorePath contructor option

1904f9a

update docs and jsdoc

2f9e14a

do not use gitIgnoreToMinimatch on ignorePatterns constructor option

ebaec07

mdjermanovic added core Relates to ESLint's core APIs and features accepted There is consensus among the team that this change meets the criteria for inclusion feature This change adds a new feature to ESLint labels Sep 27, 2022

mdjermanovic commented Sep 27, 2022

View reviewed changes

nzakas approved these changes Sep 29, 2022

View reviewed changes

mdjermanovic mentioned this pull request Sep 29, 2022

Bug: [new config system] .eslintignore doesn't work like .gitignore #16264

Closed

1 task

nzakas merged commit 6964cb1 into main Oct 3, 2022

nzakas deleted the issue16264 branch October 3, 2022 23:10

sync-by-unito bot mentioned this pull request Oct 10, 2022

build(deps-dev): bump eslint from 8.20.0 to 8.25.0 SmartTokenLabs/token-negotiator#420

Merged

moul-bot mentioned this pull request Oct 10, 2022

chore(deps-dev): bump eslint from 7.32.0 to 8.25.0 in /js berty/berty#4459

Closed

eslint-github-bot bot locked and limited conversation to collaborators Apr 2, 2023

eslint-github-bot bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Apr 2, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: remove support for ignore files in FlatESLint #16355

feat: remove support for ignore files in FlatESLint #16355

mdjermanovic commented Sep 27, 2022 •

edited

Loading

netlify bot commented Sep 27, 2022 •

edited

Loading

mdjermanovic Sep 27, 2022

mdjermanovic Sep 27, 2022

mdjermanovic Sep 27, 2022

nzakas left a comment

nzakas Sep 29, 2022

mdjermanovic Oct 3, 2022

nzakas Oct 3, 2022

jedwards1211 commented Oct 17, 2022

feat: remove support for ignore files in FlatESLint #16355

feat: remove support for ignore files in FlatESLint #16355

Conversation

mdjermanovic commented Sep 27, 2022 • edited Loading

Prerequisites checklist

What is the purpose of this pull request? (put an "X" next to an item)

What changes did you make? (Give an overview)

Is there anything you'd like reviewers to focus on?

netlify bot commented Sep 27, 2022 • edited Loading

✅ Deploy Preview for docs-eslint ready!

mdjermanovic Sep 27, 2022

Choose a reason for hiding this comment

mdjermanovic Sep 27, 2022

Choose a reason for hiding this comment

mdjermanovic Sep 27, 2022

Choose a reason for hiding this comment

nzakas left a comment

Choose a reason for hiding this comment

nzakas Sep 29, 2022

Choose a reason for hiding this comment

mdjermanovic Oct 3, 2022

Choose a reason for hiding this comment

nzakas Oct 3, 2022

Choose a reason for hiding this comment

jedwards1211 commented Oct 17, 2022

mdjermanovic commented Sep 27, 2022 •

edited

Loading

netlify bot commented Sep 27, 2022 •

edited

Loading