Update stylelint v16#7
Conversation
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThis PR upgrades stylelint from version 13 to version 16 and migrates style-related rules from standard stylelint to the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–25 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
| } else { | ||
| assert.deepEqual(warnings.map(x => x.text), test.warnings || [], 'warnings') | ||
| // Stylelint 16 appends rule name to warnings, e.g. "(kaliber/rule-name)" - strip it for comparison | ||
| const warningTexts = warnings.map(x => x.text.replace(/\s*\(kaliber\/[^)]+\)$/, '')) |
There was a problem hiding this comment.
geld dit dan alleen voor kaliber? of ook voor anderen?
There was a problem hiding this comment.
@AlbertSmit, the Stylelint behavior of appending rule names to warning text is not specific to kaliber — Stylelint automatically appends the rule name in parentheses at the end of the message for any rule, improving traceability of reported problems.
However, the regex here is kaliber-scoped intentionally, because this test harness (stylelint-plugins/machinery/test.js) is only ever used to test kaliber/* rules. Looking at the runTest function, the stylelint config it builds is always:
config: {
plugins: [require.resolve('../kaliber')],
rules: {
[`kaliber/${ruleName}`]: [true]
}
}Since only kaliber/ rules are ever linted through this harness, the narrower /\s*\(kaliber\/[^)]+\)$/ regex is sufficient. If this test utility were ever extended to cover non-kaliber rules (e.g., @stylistic/*), the regex would need to be broadened to something like /\s*\([^)]+\)$/ to handle any rule namespace.
| const results = resultString.split('\n') | ||
| .filter(x => x.toLowerCase().includes('⚠')) | ||
| .filter(x => x.includes('⚠') || x.includes('✖')) | ||
| // Exclude summary lines like "⚠ 110 problems (0 errors, 110 warnings)" |
There was a problem hiding this comment.
kun je dit niet meegeven aan de stylelint cli call?
| const results = resultString.split('\n') | ||
| .filter(x => x.toLowerCase().includes('⚠')) | ||
| .filter(x => x.includes('⚠') || x.includes('✖')) | ||
| // Exclude summary lines like "⚠ 110 problems (0 errors, 110 warnings)" |
There was a problem hiding this comment.
@coderabbitai can we not catch this in line 140 when we spawn 140? this seems a smell.
There was a problem hiding this comment.
@AlbertSmit, agreed — hardcoding result.stderr is brittle and tied to a specific Stylelint version's behaviour. Since spawnSync doesn't have a built-in option to merge streams, the cleanest fix at the spawnSync call site is to simply concatenate both outputs, making it resilient to whichever stream future versions write to:
const result = spawnSync(`node ./node_modules/.bin/stylelint --no-color ${files.join(' ')}`, { env, shell: true })
const resultString = result.stdout.toString() + result.stderr.toString()Alternatively, since shell: true is already set, a shell redirect would also work:
const result = spawnSync(
`node ./node_modules/.bin/stylelint --no-color ${files.join(' ')} 2>&1`,
{ env, shell: true }
)
const resultString = result.stdout.toString()The first option (combining both buffers) is generally preferable — it's portable, doesn't rely on shell-specific syntax, and future-proofs against Stylelint switching streams again.
There was a problem hiding this comment.
@coderabbitai i mean specifically // Exclude summary lines like "⚠ 110 problems (0 errors, 110 warnings)" and .filter(x => !/^\s*[⚠✖]\s+\d+\s+problem/.test(x)) and .filter(x => x.includes('⚠') || x.includes('✖')).
There was a problem hiding this comment.
@AlbertSmit, yes — the real fix is upstream at the spawnSync call. All three of those filters exist because the code is parsing Stylelint's human-readable text output, which is fragile by nature. Stylelint ships a built-in json formatter that gives you structured data instead:
function styleLint(files) {
const { FORCE_COLOR, ...env } = process.env
const result = spawnSync(
`node ./node_modules/.bin/stylelint --formatter json ${files.join(' ')}`,
{ env, shell: true }
)
// Stylelint exits non-zero when there are warnings/errors, so output goes to stdout
const output = JSON.parse(result.stdout.toString() || '[]')
const reportedProblems = new Set(
output.flatMap(file => file.warnings.map(w => w.rule))
)
return reportedProblems
}This removes the need for:
--no-color(irrelevant for JSON)- The
⚠/✖symbol filter - The summary-line exclusion regex
- The segment-splitting
.map()to extract the rule name
The warnings[].rule field in the JSON output is exactly what's being extracted by the current text-parsing dance, but without any of the fragility. It also handles errors and warnings uniformly since both appear in the warnings array (distinguished by severity).
There was a problem hiding this comment.
Misschien is dit dan nog een idee @tsharms, filteren op leestekens voelt een beetje sketchy, ook al zat het er al in?
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
2 participants
Ik heb
@stylistic/stylelint-plugintoegevoegd zodat we de rules die stylelint sinsds versie 16 verwijderd heeft nog kunnen gebruiken. Dit is een tussen oplossing want eigenlijk zegt Stylelint dat we deze rules moeten afvangen met Prettier.Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.