Wire up stylelint for CSS/SCSS - #1857
Conversation
Adds a .stylelintrc.json (extending @wordpress/stylelint-config), lint:css/lint:css:fix npm scripts, a lint-staged entry so husky's pre-commit hook enforces it on staged *.scss files, and a lint-css.yml CI workflow mirroring the existing lint-js.yml pattern. stylelint was already an indirect dependency via @wordpress/scripts but was never actually wired into the lint pipeline.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds Stylelint configuration and package commands for CSS/SCSS linting, defines ignored paths, extends staged-file checks, and introduces a GitHub Actions workflow that lints changed stylesheets in pull requests and all stylesheets otherwise. ChangesCSS linting
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant PullRequest
participant GitHubActions
participant GitHubAPI
participant Stylelint
PullRequest->>GitHubActions: Trigger CSS lint workflow
GitHubActions->>GitHubAPI: Retrieve changed files
GitHubAPI-->>GitHubActions: Return non-removed CSS/SCSS paths
GitHubActions->>Stylelint: Lint selected stylesheet paths
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
.stylelintrc.json (1)
2-2: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDeclare
@wordpress/stylelint-configdirectly indevDependencies.
.stylelintrc.jsonextends@wordpress/stylelint-config/scss-stylistic, butpackage.jsononly lists@wordpress/scripts. Add the Stylelint config explicitly so resolution doesn’t rely on hoisting or transitive installs.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.stylelintrc.json at line 2, Add `@wordpress/stylelint-config` directly to package.json’s devDependencies, matching the existing .stylelintrc.json extends entry, while leaving the current `@wordpress/scripts` dependency unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/lint-css.yml:
- Around line 63-67: Update the changed-file handling around cssFiles and the
line-74 lint invocation so PR-controlled filenames are not interpolated into
shell text. Serialize the filtered CSS/SCSS filenames (for example, as JSON),
pass the serialized value through the workflow environment, and use an
argv-based Node or Python wrapper to parse it and invoke npx with -- before the
filenames, preserving spaces and preventing shell interpretation.
- Around line 3-13: Update the lint-style workflow step to run only when the
pull request changed-file list contains stylesheet files, so CSS-free PRs skip
repository-wide scanning. Make full-repository linting available only through an
explicit, separately controlled push or workflow_dispatch path rather than
running by default on every trigger. Preserve the existing stylesheet lint
command for runs that are intentionally enabled.
In `@package.json`:
- Line 15: Update the aggregate lint script in package.json so npm run lint does
not invoke full CSS linting while the existing Stylelint baseline remains
failing; keep CSS linting available through its separate command, or clean the
violations before re-adding it to the aggregate.
---
Nitpick comments:
In @.stylelintrc.json:
- Line 2: Add `@wordpress/stylelint-config` directly to package.json’s
devDependencies, matching the existing .stylelintrc.json extends entry, while
leaving the current `@wordpress/scripts` dependency unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3357e393-ccfc-4ef0-bb4d-5ef712acb52d
📒 Files selected for processing (3)
.github/workflows/lint-css.yml.stylelintrc.jsonpackage.json
| on: | ||
| # Run on direct pushes to integration branches and on all pull requests. | ||
| push: | ||
| branches: | ||
| - develop | ||
| - main | ||
| - master | ||
| - trunk | ||
| pull_request: | ||
| # Allow manually triggering the workflow. | ||
| workflow_dispatch: |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Skip linting when a PR has no stylesheet changes.
When the changed-file list is empty, Line 74 still runs wp-scripts lint-style with no paths, which scans the entire repository. This makes unrelated PRs—and every push/manual run—hit the known baseline of approximately 27,000 violations. Skip the step for CSS-free PRs and make full-repository linting an explicit, separately controlled behavior.
Also applies to: 49-52, 72-74
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/lint-css.yml around lines 3 - 13, Update the lint-style
workflow step to run only when the pull request changed-file list contains
stylesheet files, so CSS-free PRs skip repository-wide scanning. Make
full-repository linting available only through an explicit, separately
controlled push or workflow_dispatch path rather than running by default on
every trigger. Preserve the existing stylesheet lint command for runs that are
intentionally enabled.
| const cssFiles = changedFiles | ||
| .filter(file => file.status !== 'removed') | ||
| .map(file => file.filename) | ||
| .filter(filename => filename.endsWith('.scss') || filename.endsWith('.css')) | ||
| .join(' '); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
Pass changed filenames as arguments, not shell text.
Line 74 interpolates PR-controlled filenames into an unquoted shell command. This both breaks filenames containing spaces and permits shell metacharacters such as ;, $(), or backticks to execute on the runner. Serialize the file list (for example, as JSON), pass it through env, and invoke npx through an argv-based Node/Python wrapper with -- before the filenames.
Also applies to: 72-74
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/lint-css.yml around lines 63 - 67, Update the changed-file
handling around cssFiles and the line-74 lint invocation so PR-controlled
filenames are not interpolated into shell text. Serialize the filtered CSS/SCSS
filenames (for example, as JSON), pass the serialized value through the workflow
environment, and use an argv-based Node or Python wrapper to parse it and invoke
npx with -- before the filenames, preserving spaces and preventing shell
interpretation.
| "dist:dotorg": "npm run dist:keep-build-folder", | ||
| "dist:keep-build-folder": "npx webpack --mode production && composer install --no-dev && composer dump-autoload --no-dev -o && ./scripts/dist.sh --keep-build-folder", | ||
| "lint": "./vendor/bin/phpcs && npm run lint:js", | ||
| "lint": "./vendor/bin/phpcs && npm run lint:js && npm run lint:css", |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Do not add full CSS linting to the aggregate command before the baseline is clean.
Line 15 makes npm run lint fail on every clean checkout that reaches the CSS stage because the repository currently contains approximately 27,000 existing Stylelint violations. Either clean the baseline first or keep full CSS linting separate until it is actionable.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@package.json` at line 15, Update the aggregate lint script in package.json so
npm run lint does not invoke full CSS linting while the existing Stylelint
baseline remains failing; keep CSS linting available through its separate
command, or clean the violations before re-adding it to the aggregate.
coverage/html/_css/*.css is third-party CSS bundled into the PHPUnit coverage HTML report, not plugin source. It wasn't covered by the default @wordpress/scripts ignore file (which only skips build/ and vendor/), so it was inflating lint output. build/, dist/, and vendor/ are added explicitly too so exclusion doesn't depend on the bundled default ignore file.
1 participant
Summary
stylelintand@wordpress/stylelint-configwere already pulled in transitively via@wordpress/scripts, but nothing actually wired them up — no config, no npm script, no lint-staged entry, no CI job..stylelintrc.jsonextending@wordpress/stylelint-config/scss-stylistic(same base wp-scripts uses by default).lint:css/lint:css:fixnpm scripts and foldslint:cssinto the aggregatelintscript.*.scsstolint-stagedso husky'spre-commithook enforces it on staged files, same as PHP/JS..github/workflows/lint-css.yml, mirroring the existinglint-js.ymlpattern (PR-scoped changed-file linting).Note
This does not fix any existing style violations — running
npm run lint:csscurrently reports ~27k pre-existing issues across the SCSS files. The full-repo lint (on push todevelop/main) will fail until those are addressed in a follow-up; PR runs only lint files changed in that PR.Test plan
npm run lint:cssruns and reports findings using the new configlint-stagedconfig picks up*.scsson pre-commitLint CSSworkflow runs on the PRSummary by CodeRabbit