Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ER: Fix needed for conflicts scss-lint.yml and .stylelint.json #5048

Open
3 of 9 tasks
t-will-gillis opened this issue Jul 22, 2023 · 4 comments
Open
3 of 9 tasks

ER: Fix needed for conflicts scss-lint.yml and .stylelint.json #5048

t-will-gillis opened this issue Jul 22, 2023 · 4 comments
Assignees
Labels
Added to dev/pm agenda Bug Something isn't working Complexity: Large Draft Issue is still in the process of being created ER Emergent Request Feature: Refactor CSS Page is working fine - CSS needs changes to become consistent with other pages role: back end/devOps Tasks for back-end developers size: 0.25pt Can be done in 0.5 to 1.5 hours size: 3pt Can be done in 13-18 hours

Comments

@t-will-gillis
Copy link
Member

t-will-gillis commented Jul 22, 2023

Emergent Requirement - Problem

Additional Details

  • This error has also occurred on PRs for other SCSS files since github/super-linter was 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: Screenshot 2023-07-21 200735

Issue you discovered this emergent requirement in

Date discovered

7/19/23

Did you have to do something temporarily

  • YES
  • 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

  • Make a new issue
  • Discuss with team
  • Let a Team Lead know

Potential solutions [draft]

  • Recommended starting point- Caution: the following is not a complete solution yet.:

     "extends": ["stylelint-config-recommended-scss"],
     "customSyntax": "postcss-scss",
    
    • Possible that this line should be:
     "extends": "stylelint-config-sass-guidelines",
    

    or

    "extends" : "stylelint-config-standard-scss",  
    
    • 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.yml as can be seen in this log.

Log screenshot for Lint SCSS after the above edits (with "map-get") Screenshot 2023-07-21 205809
  • UPDATE 8/22/23: Conclusions after testing the above recommendations:
  • The potential edits shown above for .stylelintrc.json do 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 Screenshot 2023-08-22 112322
  • 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

@t-will-gillis t-will-gillis added Bug Something isn't working role: back end/devOps Tasks for back-end developers Complexity: Large Feature: Refactor CSS Page is working fine - CSS needs changes to become consistent with other pages size: 3pt Can be done in 13-18 hours size: 0.25pt Can be done in 0.5 to 1.5 hours labels Jul 22, 2023
@t-will-gillis t-will-gillis added this to New Issue Approval in Project Board via automation Jul 22, 2023
@t-will-gillis
Copy link
Member Author

8/22/23: A new update added to the description above.

@kimberlytanyh kimberlytanyh added the ER Emergent Request label Sep 10, 2023
@JessicaLucindaCheng JessicaLucindaCheng moved this from New Issue Approval to Emergent Requests in Project Board Nov 7, 2023
@roslynwythe roslynwythe self-assigned this Nov 28, 2023

This comment was marked as outdated.

@roslynwythe roslynwythe added the Draft Issue is still in the process of being created label Nov 28, 2023
@ExperimentsInHonesty
Copy link
Member

@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?

@t-will-gillis
Copy link
Member Author

t-will-gillis commented May 11, 2024

Continuing from the notes above...

Bottom line up front:

  • As an immediate fix, my thoughts are that we should revert the github/super-linter version to 4.8.1. This was the version we were using prior to lint-scss.yml failures 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.yml file 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.yml is 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.yml file.
    • Possibly (?) one of these configurations could be written to match what we have currently
  • 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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Added to dev/pm agenda Bug Something isn't working Complexity: Large Draft Issue is still in the process of being created ER Emergent Request Feature: Refactor CSS Page is working fine - CSS needs changes to become consistent with other pages role: back end/devOps Tasks for back-end developers size: 0.25pt Can be done in 0.5 to 1.5 hours size: 3pt Can be done in 13-18 hours
Projects
Project Board
  
Emergent Requests
Development

No branches or pull requests

4 participants