refactor!: files is include - #326
Open
alexander-akait wants to merge 2 commits into
Open
Conversation
The ESLint check read the module graph and nothing else, so a file webpack never built was invisible to it: a module nothing imports yet went unchecked until something imported it. Stylelint walked the file system and did not have the problem, which made `files` mean two things depending on which check read it. It means one thing now. Naming `files` says what to check, so every file they match is checked; leaving it unset keeps each check reading what it read before, which is the module graph for ESLint and a walk for Stylelint. The walk is the one the Stylelint check already used, so this chooses between two paths that both existed rather than adding a third. No option was added for it. A check that has been told which files to look at has been told enough. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GzZci4NQeiqwdrVfd7dGXy
Every equivalent plugin for the other bundlers spells this `include` — the name comes from rollup's own filter helper, which most of them build on — and we were the one of seven that did not. Renaming it also gives `files` back to the checks. Stylelint has an option of that name, and the plugin was taking the word for itself and then handing its value over; the value was replaced by each lint's own file list anyway, so it is dropped from what Stylelint is told and `files` now means Stylelint's `files` wherever it is written. The dedupe the module phase does is a set rather than a scan of an array while the names are moving: it is asked of every module of every build. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GzZci4NQeiqwdrVfd7dGXy
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Every equivalent plugin for the other bundlers spells this option
include— the name comes from rollup's own filter helper, which most of them build on — and we were the one of seven that did not. A rename has to happen before 1.0.0 ships or never, which is why it is worth doing now rather than later.Renaming it also gives
filesback to the checks. Stylelint has an option of that name, and the plugin was taking the word for itself and then passing its value along; each lint names its own file list and replaces that value, so it is now dropped from what Stylelint is told, andfileswritten in astylelintentry means Stylelint'sfilesuntouched.One thing came along with it: the dedupe the module phase does is a
Setrather thanArray#includesover a growing list. It is asked of every module of every build, so it was a scan per module.Stacked on #325, which is where
include's "checks a file webpack never built" behaviour comes from — its commit is in this branch and leaves the diff once it merges. Review the last commit only.What kind of change does this PR introduce?
refactor
Did you add tests for your changes?
The existing cases move to the new name, and
test/stylelint/stylelint-options.test.jsgains the assertion that matters here: given bothfilesandinclude, Stylelint is toldfilesand neverinclude.Does this PR introduce a breaking change?
Yes.
filesat the top level or in a check entry becomesinclude. Astylelintentry that wrotefilesmeaning "the files to check" keeps working by accident — Stylelint reads it — but should move toincludeto be checked the same way as every other check. Both migration tables inREADME.mdrecord the rename.If relevant, what needs to be documented once your changes are merged or what have you already documented?
README.mdhere: the option's own section, every reference to it, the third-party check example, and both migration tables. The webpack.js.org page wants the rename after release.Use of AI
AI was used. Claude Code checked the name against the published option sets of the Vite, Rollup and esbuild plugins, traced every read of
filesthrough the source before moving it, found that the value handed to Stylelint was always replaced, and ran the suite, lint, audit and both dist entry points. It also corrected itself mid-change: it had reported that the option's array doubled as the dedupe accumulator, which the runner's own spread rules out.🤖 Generated with Claude Code
https://claude.ai/code/session_01GzZci4NQeiqwdrVfd7dGXy
Generated by Claude Code