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

USWDS-Compile - POAM: December '24 #130

Merged
merged 5 commits into from
Dec 18, 2024
Merged

USWDS-Compile - POAM: December '24 #130

merged 5 commits into from
Dec 18, 2024

Conversation

mahoneycm
Copy link
Contributor

@mahoneycm mahoneycm commented Nov 18, 2024

Summary

POAM dependency updates for November + December 2024.

Related issue

USWDS-Team - POAM: December 2024

Vulnerabilities

No changes in vulnerabilities

found 0 vulnerabilities

Major changes

  • Updated to latest USWDS version
  • New sass-emedded version brings on new sass deprecation warnings

New sass deprecation warnings

We have a handful of new sass warnings that are repeated throughout build process:

Deprecation Warning: Sass @import rules are deprecated and will be removed in Dart Sass 3.0.0.

Deprecation Warning: Global built-in functions are deprecated and will be removed in Dart Sass 3.0.0.
Use math.unit instead.

Deprecation Warning: Global built-in functions are deprecated and will be removed in Dart Sass 3.0.0.
Use map.merge instead.

Deprecation Warning: Global built-in functions are deprecated and will be removed in Dart Sass 3.0.0.
Use string.unquote instead.

Deprecation Warning: color.lightness() is deprecated. Suggestion: color.channel($color, "lightness", $space: hsl)

Deprecations were added to uswds/uswds#6104 to resolve. We can also chose to silence these deprecations for downstream users if we would like, or alternatively, we can choose to not update sass until deprecations are handled.

Dependency updates

Dependency Name Old Version New Version
@uswds/uswds ^3.9.0 ^3.10.0
postcss 8.4.47 8.4.49
sass-embedded 1.79.4 1.83.0

Testing instructions

Tip

To reduce review time, I have updated the November PR which had not been merged yet. Because of this, you'll see cm-poam-nov-24 as the branch name. I have updated the branch with the latest changes from December.

  1. Install this branch on site
    • On Site's main branch, run:
      npm install "https://github.com/uswds/uswds-compile/tree/cm-poam-nov-24" --save
  2. Run gulp build scripts and ensure compile runs as expected without error.
    • You'll see new deprecation warnings. These deprecated functions will be supported until the next major version of Sass is supported.
  3. Ensure no regressions when package is used to compile site

@mahoneycm mahoneycm marked this pull request as ready for review November 20, 2024 16:43
@mahoneycm mahoneycm requested a review from a team as a code owner November 20, 2024 16:43
@mdmower-csnw
Copy link
Contributor

mdmower-csnw commented Nov 20, 2024

Deprecations were added to uswds/uswds#6104 to resolve. We can also chose to silence these deprecations for downstream users if we would like, or alternatively, we can choose to not update sass until deprecations are handled.

Hi @mahoneycm, hope you don't mind some outside input. I see that a "light touch" approach was taken in #126 . I would normally agree, but it seems like the sass folks have been on a deprecation spree since v1.77. (My personal projects using bootstrap have similarly been affected.) Is there a chance the USWDS team could consider using option quietDeps: true instead of adding new deprecations as they arise to that array (requiring new uswds-compile releases)?

Or if you'd like to let end users make an informed decision for themselves about whether to silence or show deprecation warnings, introducing settings.compile.sassOptions could be a way to shift responsibility off of this team.

Thanks!

@mahoneycm
Copy link
Contributor Author

Hey there @mdmower-csn! I appreciate the recommendation!

We actually discussed all the new deprecation messages and how we would like to handle them yesterday and both of your suggestions were considered! I'm going to experiment with them both a bit today and share my findings with the team.

As a user, do you have a preference? I think I lean towards the option for users to silence the warnings themselves. While testing, I found silencing the deprecations in compile would also silence any warnings found in custom CSS that the users would need to resolve themselves and I don't want users to be blindsided when Sass 2.0.0 releases.

Let me know what you think!

@mdmower-csnw
Copy link
Contributor

@mahoneycm

We actually discussed all the new deprecation messages and how we would like to handle them yesterday and both of your suggestions were considered! I'm going to experiment with them both a bit today and share my findings with the team.

Excellent, glad to hear it!

As a user, do you have a preference? I think I lean towards the option for users to silence the warnings themselves. While testing, I found silencing the deprecations in compile would also silence any warnings found in custom CSS that the users would need to resolve themselves and I don't want users to be blindsided when Sass 2.0.0 releases.

Thanks for pointing out the issue where deprecations are silenced for user SCSS as well. That sways me the most towards letting end users define sass options. I played around with this for a while today and this seemed like the right balance of "good out of box experience" and "let the user choose the right level of warnings for themselves": mdmower-csnw@1a02189

cathybaptista
cathybaptista previously approved these changes Nov 22, 2024
Copy link

@cathybaptista cathybaptista left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mahoneycm, LGTM! Ran fine and not seeing any regressions on site.

@mahoneycm mahoneycm changed the title USWDS-Compile - POAM: November '24 USWDS-Compile - POAM: December '24 Dec 9, 2024
amyleadem
amyleadem previously approved these changes Dec 9, 2024
Copy link
Contributor

@amyleadem amyleadem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

  • Confirmed I can run npm install without error.
  • Confirmed I can run gulp commands after installing on uswds-sandbox

Copy link
Contributor

@amyleadem amyleadem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@thisisdano thisisdano merged commit ab13b8b into develop Dec 18, 2024
3 checks passed
@thisisdano thisisdano deleted the cm-poam-nov-24 branch December 18, 2024 23:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Fed Final Review
Development

Successfully merging this pull request may close these issues.

5 participants