USWDS-Compile - Sass: Create quiet deprecations setting by mahoneycm · Pull Request #131 · uswds/uswds-compile

mahoneycm · 2024-11-25T19:12:03Z

Summary

Silenced USWDS Sass deprecation warnings. Sass deprecation warnings related to USWDS code will now be turned off by default. If you would like to see deprecation warnings related to USWDS Sass, set settings.compile.sassDeprecationWarnings to true in your gulpfile.js . Deprecation warnings for non-USWDS Sass will output even when this value is set to false.

USWDS Sass deprecations are being tracked in uswds/uswds#6104

Based on work contributed by @mdmower-csnw. Thank you for your contribution!

Breaking change

This is not a breaking change.

Related issue

Closes #132

Related pull requests

Follow up to #126 and additional Sass deprecations popping up.

Documentation update: uswds/uswds-site#3006

Preview link

Test repo →

Problem statement

In its next major release, Sass is changing how it handles mixed declaration style definitions to match new CSS patterns. Because of this, users are getting alarming warnings during compilation.

The pattern is not currently deprecated and we are on track to update the related USWDS code.

Sass continues to push out new deprecation warnings in minor releases.

Solution

In gulpfile.js, create sassDeprecationWarnings setting and set the default to false.

This approach will mute USWDS warnings, including any future warnings that come up, while allowing warnings from custom sass to still be triggered in the terminal.

This will allow users to be made aware of impending deprecations without worrying about our own.

Users can decide to turn on the deprecation warning if they would like additional context on the upcoming deprecations.

Major changes

New setting: uswds.settings.compile.sassDeprecationWarnings set to false by default.
Users will no longer be notified of the upcoming Sass deprecations we are on track to fix.
Users will still be notified of deprecations related to custom Sass that they will be responsible for updating.

Testing and review

1. Confirm USWDS deprecations are silenced

On Sandbox main branch, install this version of compile

npm install "https://github.com/uswds/uswds-compile/tree/cm-quietDeps-setting" --save-dev

Run npm run uswds:buildSass
Confirm there are no deprecation warnings

2. Confirm quietSassDeps setting is respected

After completing the steps above, visit gulpfile.js

Under line 15 add the following:

uswds.settings.compile.sassDeprecationWarnings = true;

Run npm run uswds:buildSass
Confirm deprecation now warnings appear in terminal

3. Confirm custom sass triggers deprecation warnings

Checkout Sandbox testing repo locally
Run npm install
Run npm run uswds:buildSass
Confirm the deprecation warnings appear
Confirm they all point to _uswds-custom-styles.scss
Confirm there are no USWDS deprecation messages

mandylloyd

This works great! Love that it won't need further updates for future deps and it honors custom warnings.

LGTM! 👍

mejiaj

@mahoneycm I tested all three use cases you outlined. Confirming that this work fixes those use cases.

There was a little confusion in the first test case.

mahoneycm · 2024-12-06T20:53:40Z

@mejiaj to for clarirty, are you seeing deprecation warnings on the main branch immediately after installing this working branch?

In my instructions, I laid it out in three linear sections and marked where you would and would not see deprecation warnings. During my testing, I saw no deprecation warnings in the "1. Confirm USWDS deprecations are silenced" section.

They should only begin to appear at the end of the next section 🤔

amyleadem

LGTM! One small suggestion on the text in the README, but other than that, looking good!

Confirm that this silences USWDS Sass deprecation warnings
Confirmed project Sass still throws deprecation warnings
Confirmed Sass still compiles as expected
Confirmed that the uswds.settings.compile.quietSassDeps setting works as expected
Confirmed the README is updated and accurate
- Suggested a small tweak for clarity

Note

I needed to update the uswds branch in the Sandbox testing repo from step 3 to develop since the POAM branch was already merged in.

amyleadem

Adding one more suggestion for consideration. After reading through the doc updates, I'm feeling like some added clarity to the setting name might be helpful. Happy to talk about it you have any questions.

amyleadem · 2024-12-10T00:01:05Z

    },
    browserslist: ["> 2%", "last 2 versions", "IE 11", "not dead"],
    sassSourcemaps: true,
+    quietSassDeps: true


suggestion: Wondering if we can improve clarity with the setting name here to highlight that this is about Sass "warnings", not just "deps" (This also clarifies that we are talking about deprecations, not dependencies).

Also wondering if it should be written in the negative, so that we can set "false" as a default. This part is very much a quibble and could just be personal preference, but it felt a bit odd to add a settings.compile.quietSassDeps = false to the gulpfile.

Curious what you think!

Suggested change

quietSassDeps: true

sassDeprecationWarnings: false

Hmm I'm divided here. While I do think that the naming convention your suggesting makes sense, part of me thinks we should match what Sass is providing. Since quietDeps is the name of setting I see potential value in keeping ours close to it.

Do you see any value in changing it to sassQuietDeps so we're sort of "prefixing" the Sass setting name with "Sass"?

Alternatively, we use this approach which would probably be more readable for users unfamiliar with this Sass deprecation 🤔

sassDeprecationWarnings: false ... sass({ ... quietDeps: !settings.compile.sassDeprecationWarnings, })

Renamed in 7a4c434!

Co-authored-by: Amy Leadem <93996430+amyleadem@users.noreply.github.com>

mdmower-csnw · 2024-12-10T17:00:13Z

I don't mean to rock the boat as this is an appreciated feature, but I think it's worth pointing out that there are other open issues that could be solved at the same time by simply allowing users to configure sass options. The approach I took in the fork we're using not only silences deprecations by default (i.e. good out of box experience), but it also:

let's users fine-tune their deprecation notice preferences
could satisfy USWDS-Compile - Feature: Allow users to define SASS output style #80 if a restriction I put in place is removed
(I think) could satisfy USWDS-Compile - Feature: Ability to specify external packages #59 with a bit of effort in handling includes

… human readable

mahoneycm · 2024-12-10T21:15:54Z

@mdmower-csnw Thank you for sharing! We've decided to move forward with this approach for the time being but I've opened #133 for further discussion and to track this potential new feature.

We appreciate your input and feedback as we improve Compile!

cathybaptista

@mahoneycm just want to confirm... on the 1st test step, I am getting a deprication, although it is different from those I get in step 3. Here is a vid so you can see exactly what I'm talking about. :)

Screen.Recording.2024-12-11.at.4.14.52.PM.mov

mahoneycm · 2024-12-11T22:53:13Z

@mahoneycm just want to confirm... on the 1st test step, I am getting a deprication, although it is different from those I get in step 3. Here is a vid so you can see exactly what I'm talking about. :)

Good call out @cathybaptista! That deprecation is separate from style definition deprecations and is being tracked by uswds/uswds#6103! We'll resolve that in upcoming work. It's good to know that users may still see that warning despite this PR.

cathybaptista

@mahoneycm LGTM then. :)

amyleadem

LGTM! Approving because the code looks good, but the PR description needs some updates (listed below).

Thanks for setting up these testing branch and instructions. Made checking everything very straightforward!

Confirm that this silences USWDS Sass deprecation warnings by default
Confirmed that the uswds.settings.compile.sassDeprecationWarnings = true turns USWDS deprecation warnings back on
Confirmed non-USWDS Sass triggers deprecation warnings
Confirmed Sass still compiles as expected
Confirmed the README is updated and accurate
Confirmed the PR description is up-to-date and accurate
- There are outdated references to settings.compile.quietSassDeps. These should be updated to settings.compile.sassDeprecationWarnings
- I needed to update the uswds branch in the Sandbox testing repo from step 3 to develop since the POAM branch was already merged in.

mejiaj

LGTM. Tested on sandbox [cm-test-compile-131], but I had to install this feature branch because the POAM branch was being used.

mahoneycm · 2024-12-18T18:35:00Z

@mejiaj oop! Good catch. Updated to match Sandbox main to prevent any file changes.

…tDeps-setting

mahoneycm added 3 commits November 25, 2024 14:11

Create quietDeps setting

35b48cb

Update sass-embedded to test new deprecation warnings

6d0329b

Set default setting to true

ae7ffa0

mahoneycm mentioned this pull request Nov 26, 2024

USWDS-Sandbox - Testing repo for compile#131 [TEST] uswds/uswds-sandbox#169

Closed

mahoneycm requested review from CTGM-Bixal, amyleadem, cathybaptista, mandylloyd and mejiaj November 26, 2024 16:28

mahoneycm marked this pull request as ready for review November 26, 2024 16:28

mahoneycm requested a review from a team as a code owner November 26, 2024 16:28

mahoneycm changed the title ~~USWDS-Compile - Sass: Create quietDeps setting [WIP]~~ USWDS-Compile - Sass: Create quietDeps setting Dec 2, 2024

mandylloyd previously approved these changes Dec 6, 2024

View reviewed changes

mejiaj previously approved these changes Dec 6, 2024

View reviewed changes

mejiaj changed the title ~~USWDS-Compile - Sass: Create quietDeps setting~~ USWDS-Compile - Sass: Create quiet deprecations setting Dec 6, 2024

mejiaj requested review from heymatthenry and thisisdano December 6, 2024 19:53

Add quietSassDeps setting to settings table

23901df

mahoneycm dismissed stale reviews from mejiaj and mandylloyd via 23901df December 9, 2024 22:47

amyleadem reviewed Dec 9, 2024

View reviewed changes

Comment thread README.md Outdated

amyleadem suggested changes Dec 9, 2024

View reviewed changes

amyleadem suggested changes Dec 10, 2024

View reviewed changes

Update README.md

1074780

Co-authored-by: Amy Leadem <93996430+amyleadem@users.noreply.github.com>

Rename setting to sassDeprecationWarnings and update logic to be more…

7a4c434

… human readable

mahoneycm requested a review from amyleadem December 10, 2024 21:02

mahoneycm mentioned this pull request Dec 10, 2024

USWDS-Compile - Feature: Allow users to configure Sass options #133

Open

2 tasks

cathybaptista reviewed Dec 11, 2024

View reviewed changes

mahoneycm requested a review from cathybaptista December 11, 2024 22:53

cathybaptista previously approved these changes Dec 12, 2024

View reviewed changes

amyleadem previously approved these changes Dec 17, 2024

View reviewed changes

mejiaj previously approved these changes Dec 18, 2024

View reviewed changes

Merge branch 'develop' of github.com:uswds/uswds-compile into cm-quie…

824a8e1

…tDeps-setting

mahoneycm dismissed stale reviews from mejiaj, amyleadem, and cathybaptista via 824a8e1 December 18, 2024 23:24

thisisdano approved these changes Dec 18, 2024

View reviewed changes

thisisdano merged commit 7b88d94 into develop Dec 18, 2024

thisisdano deleted the cm-quietDeps-setting branch December 18, 2024 23:29

Uh oh!

Conversation

mahoneycm commented Nov 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Breaking change

Related issue

Related pull requests

Preview link

Problem statement

Solution

Major changes

Testing and review

1. Confirm USWDS deprecations are silenced

2. Confirm quietSassDeps setting is respected

3. Confirm custom sass triggers deprecation warnings

Uh oh!

mandylloyd left a comment

Choose a reason for hiding this comment

Uh oh!

mejiaj left a comment

Choose a reason for hiding this comment

Uh oh!

mahoneycm commented Dec 6, 2024

Uh oh!

Uh oh!

amyleadem left a comment

Choose a reason for hiding this comment

Uh oh!

amyleadem left a comment

Choose a reason for hiding this comment

Uh oh!

amyleadem Dec 10, 2024

Choose a reason for hiding this comment

Uh oh!

mahoneycm Dec 10, 2024

Choose a reason for hiding this comment

Uh oh!

mahoneycm Dec 10, 2024

Choose a reason for hiding this comment

Uh oh!

mdmower-csnw commented Dec 10, 2024

Uh oh!

mahoneycm commented Dec 10, 2024

Uh oh!

cathybaptista left a comment

Choose a reason for hiding this comment

Uh oh!

mahoneycm commented Dec 11, 2024

Uh oh!

cathybaptista left a comment

Choose a reason for hiding this comment

Uh oh!

amyleadem left a comment

Choose a reason for hiding this comment

Uh oh!

mejiaj left a comment

Choose a reason for hiding this comment

Uh oh!

mahoneycm commented Dec 18, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

mahoneycm commented Nov 25, 2024 •

edited

Loading