Replace scss-lint with stylelint #1825
Conversation
- Remove scss_lint gem dependency and configuration files - Add stylelint and stylelint-config-standard-scss npm packages - Create .stylelintrc.json configuration matching previous scss-lint rules - Update rake tasks to use stylelint instead of scss-lint - Add stylelint to CI workflow (lint-js-and-ruby.yml) - Include SCSS linting in main rake lint task and autofix task - Add yarn script 'lint:scss' for running stylelint Fixes #1688 🤖 Generated with [Claude Code](https://claude.ai/code) Co-authored-by: Justin Gordon <justin808@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com>
Cannot modify workflow files without workflows permission. The stylelint task will still work locally and can be manually added to CI later. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-authored-by: Claude <noreply@anthropic.com>
WalkthroughReplaces scss-lint with stylelint across the repo: removes the Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant CI as CI
participant Rake as Rake (lint tasks)
participant ESL as ESLint
participant RUB as RuboCop
participant STY as Stylelint
Dev->>Rake: rake lint
Note over Rake: aggregated lint tasks
Rake->>ESL: run eslint
ESL-->>Rake: results
Rake->>RUB: run rubocop
RUB-->>Rake: results
Rake->>STY: yarn run lint:scss
STY-->>Rake: results
CI->>CI: run workflow
CI->>STY: yarn run lint:scss (new step)
STY-->>CI: results
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (1)📓 Common learnings⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
🔇 Additional comments (1)
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 |
Code Review for PR #1825: Replace scss-lint with stylelintSummaryThis PR successfully modernizes the SCSS linting stack by replacing the deprecated scss-lint with stylelint. The migration is well-executed with a few important issues to address. Critical Issues1. Missing CI Integration
|
Code Review - PR #1825: Replace scss-lint with stylelint✅ Positive Aspects
🐛 Issues FoundCRITICAL: Missing Trailing Newline
|
- Remove @stylistic/stylelint-plugin dependency (not needed) - Fix deprecated rule names (color-keyword → color-named) - Disable formatting rules to match original scss-lint philosophy - Configuration now works correctly with stylelint v16 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
package.json (1)
103-103: DRY the SCSS globs; call the Yarn script from RakeAvoid duplicating the long glob list in Rake. Use the package.json script from the Rake task, and add a fix variant for consistency.
Apply in rakelib/lint.rake:
- task :scss do - sh_in_dir(gem_root, "yarn run stylelint \"spec/dummy/app/assets/stylesheets/**/*.scss\" \"spec/dummy/client/**/*.scss\"") - end + task :scss do + sh_in_dir(gem_root, "yarn run stylelint --version", "yarn run lint:scss") + endOptionally add a fix script to package.json:
"scripts": { ... "lint:scss": "stylelint \"spec/dummy/app/assets/stylesheets/**/*.scss\" \"spec/dummy/client/**/*.scss\"", + "lint:scss:fix": "yarn run lint:scss -- --fix", ... }Then in rakelib/lint.rake autofix task:
- sh_in_dir(gem_root, "yarn run stylelint \"spec/dummy/app/assets/stylesheets/**/*.scss\" \"spec/dummy/client/**/*.scss\" --fix") + sh_in_dir(gem_root, "yarn run lint:scss:fix").stylelintrc.json (2)
9-11: Consider dropping ignoreAtRules; standard-scss already handles SCSS at-rulesstylelint-config-standard-scss enables scss/at-rule-no-unknown; the common SCSS at-rules are recognized. Keep only if you’ve hit false positives.
5-8: Disable conflicting Stylelint formatting rulesPrettier lowercases CSS hex colors by default, which conflicts with these stylistic rules. Disable them (or switch to stylelint-config-prettier) in .stylelintrc.json:
- "color-hex-length": "long", - "color-hex-case": "upper", - "number-leading-zero": "always", + "color-hex-length": null, + "color-hex-case": null, + "number-leading-zero": null,rakelib/lint.rake (2)
13-16: Use version output for consistency and call the Yarn scriptMatch eslint/rubocop behavior and avoid repeating globs.
- task :scss do - sh_in_dir(gem_root, "yarn run stylelint \"spec/dummy/app/assets/stylesheets/**/*.scss\" \"spec/dummy/client/**/*.scss\"") - end + task :scss do + sh_in_dir(gem_root, "yarn run stylelint --version", "yarn run lint:scss") + end
28-33: DRY the autofix command and update top-level descriptionReuse the Yarn fix script and update the “autofix” desc to mention Stylelint.
- sh_in_dir(gem_root, "yarn run stylelint \"spec/dummy/app/assets/stylesheets/**/*.scss\" \"spec/dummy/client/**/*.scss\" --fix") + sh_in_dir(gem_root, "yarn run lint:scss:fix")And update the desc near Lines 41-42 to include “stylelint --fix”.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.scss-lint.yml(0 hunks).stylelintrc.json(1 hunks)Gemfile.development_dependencies(0 hunks)package.json(2 hunks)rakelib/docker.rake(1 hunks)rakelib/lint.rake(1 hunks)spec/dummy/bin/scss-lint(0 hunks)
💤 Files with no reviewable changes (3)
- Gemfile.development_dependencies
- spec/dummy/bin/scss-lint
- .scss-lint.yml
🧰 Additional context used
📓 Path-based instructions (3)
{Gemfile,Rakefile,**/*.{rb,rake,gemspec,ru}}
📄 CodeRabbit inference engine (CLAUDE.md)
{Gemfile,Rakefile,**/*.{rb,rake,gemspec,ru}}: Before every commit/push, runbundle exec rubocopand fix all violations in Ruby code
Let RuboCop handle all Ruby code formatting; never manually format Ruby files
Files:
rakelib/docker.rakerakelib/lint.rake
{Gemfile,Rakefile,**/*.{rb,rake,gemspec,ru,js,jsx,ts,tsx,json,yml,yaml,md,css,scss}}
📄 CodeRabbit inference engine (CLAUDE.md)
Ensure all committed files end with a trailing newline character
Files:
rakelib/docker.rakerakelib/lint.rakepackage.json
**/*.{js,jsx,ts,tsx,json,yml,yaml,md,css,scss}
📄 CodeRabbit inference engine (CLAUDE.md)
Let Prettier handle all non-Ruby file formatting; never manually format these files
Files:
package.json
🧠 Learnings (5)
📚 Learning: 2025-09-22T07:21:12.910Z
Learnt from: CR
PR: shakacode/react_on_rails#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-22T07:21:12.910Z
Learning: Applies to **/*.{js,jsx,ts,tsx,json,yml,yaml,md,css,scss} : Let Prettier handle all non-Ruby file formatting; never manually format these files
Applied to files:
.stylelintrc.jsonrakelib/lint.rake
📚 Learning: 2025-09-22T07:21:12.910Z
Learnt from: CR
PR: shakacode/react_on_rails#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-22T07:21:12.910Z
Learning: Applies to {Gemfile,Rakefile,**/*.{rb,rake,gemspec,ru}} : Before every commit/push, run `bundle exec rubocop` and fix all violations in Ruby code
Applied to files:
rakelib/lint.rake
📚 Learning: 2025-09-22T07:21:12.910Z
Learnt from: CR
PR: shakacode/react_on_rails#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-22T07:21:12.910Z
Learning: Applies to {Gemfile,Rakefile,**/*.{rb,rake,gemspec,ru}} : Let RuboCop handle all Ruby code formatting; never manually format Ruby files
Applied to files:
rakelib/lint.rake
📚 Learning: 2025-09-22T07:21:12.910Z
Learnt from: CR
PR: shakacode/react_on_rails#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-22T07:21:12.910Z
Learning: Applies to {Gemfile,Rakefile,**/*.{rb,rake,gemspec,ru,js,jsx,ts,tsx,json,yml,yaml,md,css,scss}} : Ensure all committed files end with a trailing newline character
Applied to files:
rakelib/lint.rake
📚 Learning: 2025-01-23T18:20:45.824Z
Learnt from: alexeyr-ci
PR: shakacode/react_on_rails#1687
File: spec/dummy/package.json:0-0
Timestamp: 2025-01-23T18:20:45.824Z
Learning: When adding or updating dependencies in spec/dummy/package.json, maintain version consistency with other package.json files in the codebase to avoid potential version conflicts.
Applied to files:
package.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: rspec-package-tests (3.2, minimum)
- GitHub Check: claude-review
- GitHub Check: rspec-package-tests (3.4, minimum)
🔇 Additional comments (2)
rakelib/lint.rake (1)
23-24: LGTM: scss included in aggregate lint taskGood integration into the main lint flow.
rakelib/docker.rake (1)
9-12: LGTM: Docker task now references StylelintDescription and task align with the new linter.
|
Code Review: Replace scss-lint with stylelint Overall Assessment: This is a well-executed modernization PR that successfully migrates from the deprecated scss-lint to the actively maintained stylelint. The migration is clean, thorough, and follows best practices. STRENGTHS
CRITICAL ISSUE: Missing CI Integration The PR description claims "Add stylelint to CI workflow (lint-js-and-ruby.yml)" but NO changes were made to .github/workflows/lint-js-and-ruby.yml. Impact: SCSS linting will not run in CI, defeating the purpose of the migration. Fix Required - Add after line 82 in .github/workflows/lint-js-and-ruby.yml:
CODE QUALITY CONCERNS
SECURITY & PERFORMANCE No security concerns identified CHECKLIST BEFORE MERGE
CONCLUSION Solid work overall! The main blocker is the missing CI integration. Once the CI workflow is updated and minor suggestions are addressed, this will be ready to merge. |
|
Code Review - PR 1825 Overall Assessment: This is a well-executed modernization PR that successfully migrates from the deprecated scss-lint to stylelint. The changes are focused, well-structured, and maintain backward compatibility with the original linting philosophy. Strengths:
Issues and Recommendations: CRITICAL - Missing Trailing Newline: Glob Pattern Duplication: Security: No security concerns identified. Using well-maintained, official stylelint packages from npm. Performance: Should be comparable or better. Stylelint is actively maintained and optimized. Test Coverage: Adequate for this type of change. Action Items Before Merge:
Summary: This is a high-quality PR that successfully modernizes the SCSS linting infrastructure. The main blocker is the missing trailing newline which will cause CI failures per repository requirements. Recommendation: Approve after addressing the trailing newline issue Review generated with attention to CLAUDE.md requirements and React on Rails project conventions. |
- Remove @stylistic/stylelint-plugin dependency (not needed) - Fix deprecated rule names (color-keyword → color-named) - Disable formatting rules to match original scss-lint philosophy - Configuration now works correctly with stylelint v16 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
Remove scss_lint from lockfile after gem removal to fix CI frozen bundle mode 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
.stylelintrc.json (3)
5-5: Disable formatting rule to align with “minimal/disable formatting” goal."color-hex-length" is a formatting rule and may fight Prettier; consider disabling it.
Apply:
- "color-hex-length": "long", + "color-hex-length": null,
7-24: Simplify scss/at-rule-no-unknown (redundant ignore list).The standard-scss config already recognizes SCSS at-rules; the explicit ignore list is unnecessary.
Apply:
- "scss/at-rule-no-unknown": [ - true, - { - "ignoreAtRules": [ - "include", - "mixin", - "each", - "if", - "else", - "for", - "while", - "function", - "return", - "use", - "forward" - ] - } - ], + "scss/at-rule-no-unknown": true,
39-45: Redundant ignores for non-SCSS files.Since tasks only lint .scss via globs, these ignores are not needed; optionally drop to simplify. If keeping, consider also ignoring node_modules/** explicitly.
rakelib/lint.rake (3)
33-35: Reuse yarn script for --fix to avoid duplicated globs.Delegate to lint:scss with --fix for maintainability.
Apply:
- scss_paths = "\"spec/dummy/app/assets/stylesheets/**/*.scss\" \"spec/dummy/client/**/*.scss\"" - stylelint_cmd = "yarn run stylelint #{scss_paths} --fix" - sh_in_dir(gem_root, stylelint_cmd) + sh_in_dir(gem_root, "yarn run lint:scss --fix")Optional: consider running Prettier after stylelint so Prettier is the final formatter.
44-45: Update task description to include stylelint --fix.The description is stale.
Apply:
-desc "Auto-fix all linting violations (eslint --fix, prettier --write, rubocop -A)" +desc "Auto-fix all linting violations (eslint --fix, stylelint --fix, prettier --write, rubocop -A)"
13-17: Call thelint:scssscript in the Rake task
Replace the manualstylelintinvocation and glob duplication with:- task :scss do - sh_in_dir(gem_root, - "yarn run stylelint \"spec/dummy/app/assets/stylesheets/**/*.scss\" \"spec/dummy/client/**/*.scss\"") - end + task :scss do + sh_in_dir(gem_root, "yarn run lint:scss") + end
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (2)
.stylelintrc.json(1 hunks)rakelib/lint.rake(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
{Gemfile,Rakefile,**/*.{rb,rake,gemspec,ru}}
📄 CodeRabbit inference engine (CLAUDE.md)
{Gemfile,Rakefile,**/*.{rb,rake,gemspec,ru}}: Before every commit/push, runbundle exec rubocopand fix all violations in Ruby code
Let RuboCop handle all Ruby code formatting; never manually format Ruby files
Files:
rakelib/lint.rake
{Gemfile,Rakefile,**/*.{rb,rake,gemspec,ru,js,jsx,ts,tsx,json,yml,yaml,md,css,scss}}
📄 CodeRabbit inference engine (CLAUDE.md)
Ensure all committed files end with a trailing newline character
Files:
rakelib/lint.rake
🧠 Learnings (4)
📚 Learning: 2025-09-22T07:21:12.910Z
Learnt from: CR
PR: shakacode/react_on_rails#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-22T07:21:12.910Z
Learning: Applies to {Gemfile,Rakefile,**/*.{rb,rake,gemspec,ru}} : Before every commit/push, run `bundle exec rubocop` and fix all violations in Ruby code
Applied to files:
rakelib/lint.rake
📚 Learning: 2025-09-22T07:21:12.910Z
Learnt from: CR
PR: shakacode/react_on_rails#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-22T07:21:12.910Z
Learning: Applies to {Gemfile,Rakefile,**/*.{rb,rake,gemspec,ru}} : Let RuboCop handle all Ruby code formatting; never manually format Ruby files
Applied to files:
rakelib/lint.rake
📚 Learning: 2025-09-22T07:21:12.910Z
Learnt from: CR
PR: shakacode/react_on_rails#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-22T07:21:12.910Z
Learning: Applies to {Gemfile,Rakefile,**/*.{rb,rake,gemspec,ru,js,jsx,ts,tsx,json,yml,yaml,md,css,scss}} : Ensure all committed files end with a trailing newline character
Applied to files:
rakelib/lint.rake
📚 Learning: 2025-09-22T07:21:12.910Z
Learnt from: CR
PR: shakacode/react_on_rails#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-22T07:21:12.910Z
Learning: Applies to **/*.{js,jsx,ts,tsx,json,yml,yaml,md,css,scss} : Let Prettier handle all non-Ruby file formatting; never manually format these files
Applied to files:
rakelib/lint.rake
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: claude-review
- GitHub Check: build-dummy-app-webpack-test-bundles (3.2, 20)
- GitHub Check: rspec-package-tests (3.4, minimum)
- GitHub Check: rspec-package-tests (3.2, minimum)
🔇 Additional comments (2)
.stylelintrc.json (1)
4-4: Correct rule name fixed (color-named).Good catch replacing the invalid "color-keyword" with "color-named".
rakelib/lint.rake (1)
24-26: Added stylelint to the aggregate lint task.Looks good.
|
Code Review: Replace scss-lint with stylelint Overall Assessment: This is a well-executed migration from deprecated scss-lint to modern stylelint. Implementation is clean, follows project conventions, and maintains the original minimal linting philosophy. STRENGTHS:
CRITICAL ISSUE - Missing Trailing Newline:
CI Workflow Question:
SECURITY & PERFORMANCE:
TESTING RECOMMENDATIONS: Before merge:
Post-merge:
CODE STYLE COMPLIANCE:
FINAL VERDICT: Approve with Minor Fix Required Required before merge: Add trailing newline to .stylelintrc.json This migration properly modernizes the SCSS linting toolchain. Excellent work overall! |
Code Review: PR #1825 - Replace scss-lint with stylelintOverviewGood modernization effort replacing deprecated scss-lint. The PR follows a reasonable conservative approach, but there are some important gaps in rule mapping and configuration. High Priority Issues1. Missing Rule Mappings (.stylelintrc.json)The original .scss-lint.yml had these rules that are not properly mapped: Missing hex notation uppercase:
Missing leading zero requirement:
Missing ID selector enforcement:
2. Incomplete SCSS At-Rule CoverageThe ignoreAtRules list (lines 10-21) may miss some SCSS directives. Recommendation: Consider adding warn, error, debug, and extend to the ignoreAtRules array. Medium Priority Issues3. Task Description Inconsistency (rakelib/lint.rake:24)The description still says "Skip scss" but the task now includes scss. Fix: Update description to "Run all eslint, rubocop & stylelint linters" 4. Missing Version Check in Rake TaskOther linters (eslint, rubocop) include version checks. Consider adding version check for stylelint as well. Strengths
Recommendations
Approval StatusThe PR is functionally correct but would benefit from refinements before merge. The missing rule mappings could lead to style inconsistencies with the original scss-lint behavior. Suggested Action: Address the high-priority missing rules, update the task description, then this will be ready to merge. |
Complete the SCSS linting migration by adding stylelint to the CI pipeline. This step was originally attempted but reverted due to permission constraints. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
Code Review: Replace scss-lint with stylelintI've reviewed this PR and overall it looks excellent! The migration from the deprecated scss-lint to stylelint is well-executed. Here's my detailed feedback: ✅ Strengths
🔍 Suggestions & Considerations1. Minor: Duplicate Glob PatternsThe stylelint command is duplicated in three places:
Recommendation: This is acceptable for maintainability, but consider if the paths might diverge. The current approach is fine. 2. Missing Newline Check
3. Configuration: Consider AddingYou might want to explicitly add these rules for better SCSS validation: "scss/at-import-partial-extension": null,
"scss/load-no-partial-leading-underscore": nullThese are commonly needed for Rails/Sprockets SCSS workflows, but not blocking if current config works. 4. Performance: Glob Pattern OptimizationThe glob patterns look good, but you could potentially use 🧪 Testing ChecklistBefore merging, please verify:
🔒 Security✅ No security concerns - configuration changes only, no code execution risks. 📊 Test Coverage✅ N/A - This is a linting configuration change. Manual testing via the test plan is appropriate. 🎯 Overall AssessmentExcellent work! This PR:
Recommendation: ✅ Approve pending verification of:
📝 Minor NitThe only cosmetic suggestion: In Great migration! 🚀 |
Code Review - PR #1825: Replace scss-lint with stylelintOverall AssessmentThis is a well-executed migration from the deprecated scss-lint to modern stylelint. The PR successfully maintains the project's minimal linting philosophy while upgrading to actively maintained tooling. Strengths1. Clean Dependency Management
2. Configuration ParityThe .stylelintrc.json effectively translates the original scss-lint rules:
3. Integration Updates
4. Code Quality
Issues & Recommendations1. Missing CI Integration (High Priority)The stylelint check was removed from .github/workflows/lint-js-and-ruby.yml in commit 2/6. This means:
Recommendation: Re-add the CI step for stylelint 2. Configuration Issues (Medium Priority)a) Missing Trailing Newline b) Missing Hex Case Rule c) Missing Leading Zero Rule 3. Code Duplication (Low Priority)The SCSS file paths appear in multiple places:
Recommendation: Consider extracting to a shared constant or helper method. 4. Lint Task Organization (Low Priority)In rakelib/lint.rake:
Security Considerations
Test CoverageThe PR description mentions testing was done:
Recommendation: Ensure CI has stylelint step to prevent regressions. Performance Considerations
DocumentationPotentially Missing:
Action ItemsMust Fix Before Merge:
Should Fix:
Nice to Have:
Final Score: 8.5/10Excellent work! This is a solid migration that maintains backward compatibility while modernizing the tooling. The main issues are:
Once these are addressed, this PR will be merge-ready! File References:
|
Code Review for PR #1825: Replace scss-lint with stylelint✅ Overall AssessmentThis PR successfully migrates from the deprecated scss-lint to the modern stylelint, which is a necessary and well-executed modernization. The migration maintains consistency with the project's minimal linting approach. 👍 Strengths
🔍 Observations & Suggestions
|
Summary
Configuration Philosophy
Maintains the original minimal approach - only enables essential CSS validation rules while disabling formatting rules, matching the original scss-lint configuration philosophy.
Test Plan
Fixes #1688
🤖 Generated with Claude Code
This change is
Summary by CodeRabbit
Chores
CI