website
website copied to clipboard
ER: Fix needed for conflicts scss-lint.yml and .stylelint.json
Emergent Requirement - Problem
- The
lint-scss.ymlaction returned an error when PR #4945 (fixes #4864) was opened. Note that the error message does not refer to code that was changed, but rather that the linter does not recognize scss syntax. We need to implement a fix so that github/super-linter v5.0.0 recognizes and lints scss.
Additional Details
- This error has also occurred on PRs for other SCSS files since
github/super-linterwas updated to v5.0.0 last week, thus the error is likely related to the updated version. - The error logs refer to "CssSyntaxError" and "...you should use the 'customSyntax' option when linting something other than CSS"
- Below, the "Proposed Solutions (draft)" details the code for implementing 'customSyntax' in the
.stylelint.json
Log screenshot for Lint SCSS action
See lines 76-79:Issue you discovered this emergent requirement in
- #4945
Date discovered
7/19/23
Did you have to do something temporarily
- [ ] YES
- [x] NO
Who was involved
@t-will-gillis @ronaldpaek @angelenelm
What happens if this is not addressed
An error will be raised every time lint-scss.yml runs on an scss file. Furthermore, when the linter raises errors inappropriately it is not functioning as a check on the scss code.
Resources
Recommended Action Items
- [x] Make a new issue
- [x] Discuss with team
- [ ] Let a Team Lead know
Potential solutions [draft]
- Recommended starting point- Caution: the following is not a complete solution yet.:
- [ ] In .stylelintrc.json insert at line 2, before and at same level as "rules:"
"extends": ["stylelint-config-recommended-scss"], "customSyntax": "postcss-scss",- [ ] Possible that this line should be:
"extends": "stylelint-config-sass-guidelines",- [ ] Remove the line
"no-extra-semicolons": true. This prop is deprecated. - [ ] Remove the comma from the line immediately above.
- Doing the above and running the linter on _privacy-policy.scss will result in successfully implementing scss syntax in the linter, but will also cause an error on line 30 saying that "map-get" should be replaced with "map.get". The Sass Migrator can be used to fix the issue with "map" and will satisfy
lint.scss.ymlas can be seen in this log.
Log screenshot for Lint SCSS after the above edits (with "map-get")
- UPDATE 8/22/23: Conclusions after testing the above recommendations:
- The potential edits shown above for
.stylelintrc.jsondo successfully address the "CssSyntaxError" and the note that "...you should use the 'customSyntax' - However, these edits lead stylelint to raise other errors (such as with
map-get). - The new errors themselves can be addressed using the Sass Migrator:
sass-migrator --migrate-deps module main.scss. As shown includes options for tracing the modules/ dependencies throughout the entire codebase. The Migrator will trace and refactor all scss files on the website, using more recent syntax such as:
map-get ----> map.get
@includes ----> @use
@media #{xxx} ----> @media #{layout. xxx}
etc.
- It appears that over 50 files (!) are refactored by Migrator
- At this point, when Docker is run from here, it appears that none of the css/scss styles are used, that is additional refactoring is still required:
Post Sass Migrator
-
CONCLUSION: I think that all of the above could be done and this could be fixed, but we may want to discuss whether this is worth the time and energy to 'fix' a problem (i.e. with the linter telling us incorrectly that there are errors ) that is not currently effecting the website.
-
Updating the Sass/scss syntax might be a worthwhile endeavor but it appears that this will be a major project.
-
Recent log run after Sass Migrator
Additional comments below
-See comments below
8/22/23: A new update added to the description above.
Hi @roslynwythe, thank you for taking up this issue! Hfla appreciates you :)
Do let fellow developers know about your:- i. Availability: (When are you available to work on the issue/answer questions other programmers might have about your issue?) ii. ETA: (When do you expect this issue to be completed?)
You're awesome!
P.S. - You may not take up another issue until this issue gets merged (or closed). Thanks again :)
@t-will-gillis is there another linter that would do scss better for our codebase? And if so, could we turn off its linting of all other file types, so we only use it on our scss files, and turn off scss linting on our current linter. That way they could play nice with eachother?
Continuing from the notes above...
Bottom line up front:
- As an immediate fix, my thoughts are that we should revert the
github/super-linterversion to 4.8.1. This was the version we were using prior tolint-scss.ymlfailures and would stop most of the workflow failures, I believe. However, I also feel that this is a band-aid. - If we reverted to an earlier version of "super-linter", we would need to add an ignore statement to the
dependabot.ymlfile so it won't try to update.
Here is the longer story:
- Trying to keep this simple, my interpretation is that the Sass used by our website is old (but admittedly, I don't know Sass). I think the reason that the sass linter stopped working when we updated to a newer version of the super-linter most likely is because (at the time) it was 2023 and our Sass code is essentially unchanged from 2019. Simple evidence of this is that our .scss uses syntax such as an
@import, which according to a 2020 Medium article had already been replaced by@use. (There are many more examples) - Here is another contemporary article Introducing Sass Modules about the 2019-20 Sass changes.
- It should be safe for us to revert to version 4.8.1 because (from what I can tell through a search) the
lint-scss.ymlis the only file using "super-linter". - I believe that with the later versions, "super-linter" is pushing us into using current, up-to-date Sass, which has many changes.
- Side note: there are many default configurations for the
stylelint.ymlfile. - Possibly (?) one of these configurations could be written to match what we have currently
- Side note: there are many default configurations for the
- As mentioned above, there is a Sass Migrator that will convert all of the code to the current version of Sass, but even after doing so there will be a lot of clean up- see this log (expand line 221).
- All this can and probably should be done if we continue using Sass, but it will be pretty involved.
- (Side note: Some of these changes can be incrementally done such as adding blank lines and possibly adding code exceptions.)
- I do not have an informed opinion about this (I don't know Sass and CSS is not my strong point either), but for what it is worth I wonder if it makes any sense to look at switching to another program besides Sass or even use strict CSS?