department-of-veterans-affairs / va.gov-team

Public resources for building on and in support of VA.gov. Visit complete Knowledge Hub:
https://depo-platform-documentation.scrollhelp.site/index.html
281 stars 197 forks source link

Upgrade sass processing in vets-website #37351

Open timwright12 opened 2 years ago

timwright12 commented 2 years ago

vets-website is currently using node-sass v4.14.1. node-sass is currently a deprecated dependency and future projects that still use it should move onto Dart Sass.

vets-website needs to make this upgrade because it is blocking the upgrade of the node engine (currently set to 14.15.0) (and because we should).

Definition of Done

ndsprinkle commented 1 year ago

This looks like a decent guide to migrate from node-sass to dart-sass: https://dev.to/ccreusat/migrating-from-node-sass-to-sass-dart-sass-with-npm-3g3c

ndsprinkle commented 1 year ago

I will try to wrap up what I discovered from this ticket.

First, I grabbed full page screenshots for several pages in vets-website. I planned on using Pixelmatch (https://github.com/mapbox/pixelmatch) to do some visual regression testing.

Then, I followed a guide (https://dev.to/ccreusat/migrating-from-node-sass-to-sass-dart-sass-with-npm-3g3c) to upgrade from node sass to sass dart. Didn't have any issues with the actual upgrade process.

When trying to run vets-website after the upgrade, I had thousands of warnings, but the site ultimately compiled after about 30 minutes of the warning messages. The warnings had to do with slash div (https://sass-lang.com/documentation/breaking-changes/slash-div).

Because the site actually compiled, I was able to grab new full page screenshots. Using Pixelmatch, I confirmed all screenshots pre and post upgrade were identical, except one: find-forms

localhost_3001_find-forms_.png

I re-grabbed screenshots and re-ran the Pixelmatch test, but got the same result. It's entirely possible it's related to some logic involving a "last" element for the featured-content-list-item class, but I did not confirm this.

I also attempted the slash div migration tool (https://sass-lang.com/documentation/breaking-changes/slash-div#automatic-migration) in the design system, but it didn't address all of the warnings.

In my opinion, an upgrade of this size that affects the design system in this many ways should be handled largely by the design system team. However, it's possible I missed something in my migration tool steps or my upgrade process that someone else may figure out and be able to fix all the warning messages and Pixelmatch fail on the find-forms page.

micahchiang commented 1 year ago

Dropping in - This is something the Design System team has an interest in doing. We can put it on our roadmap.

micahchiang commented 1 year ago

I also attempted the slash div migration tool (https://sass-lang.com/documentation/breaking-changes/slash-div#automatic-migration) in the design system, but it didn't address all of the warnings.

@ndsprinkle - do you have a branch we can check out to see where things stand? Also, when you say you attempted the div migration in the design system, can you tell me which repositories you tried that in? Thanks!

it-harrison commented 10 months ago

draft of my discovery findings related to the tasks to complete this work

micahchiang commented 10 months ago

Thanks @it-harrison for the write up. It'll give us some clarification on how to approach this work moving forward.

micahchiang commented 10 months ago

Re-opening since this ticket is still needed to capture any necessary work moving forward.

humancompanion-usds commented 9 months ago

foundation-sites v6 (the latest version) has a few !global assignments. This means that unless a new version is released or we stop using this library we will not be able to update to dart-sass v2 when it releases.

I think we're just using Foundation for the grid, correct? Could we make switching to the USWDS grid a thing we front-load as well? I believe there are sass function drop in replacements for Foundation's grid that essentially turn those classes into a flex grid.