vets-website
vets-website copied to clipboard
replace node-sass with sass
Summary
vets-website
has been using node-sass
for quite some time, and it is time to replace node-sass
with sass
. Making this step will help us move forward and out of Node 14 and into a more recent version.
team: vsp-identity
- [ ] update sass-loader to its latest api
- [ ] update sass-loader options
- [ ] use sass-migrator to migrate division and strict-unary
- [ ] comment out @forward rule
- [ ] remove rebuilding node-sass in scripts
Related issue(s)
- Link to ticket created in va.gov-team repo department-of-veterans-affairs/va.gov-team#0000
- Link to previous change of the code/bug (if applicable) department-of-veterans-affairs/vets-website#0000
- Link to epic if not included in ticket department-of-veterans-affairs/va.gov-team#0000
Testing done
- Describe what the old behavior was prior to the change
- Describe the steps required to verify your changes are working as expected
- Describe the tests completed and the results
- _Exclusively stating 'Specs and automated tests passing' is NOT acceptable as appropriate testing
- Optionally, provide a link to your test plan and test execution records
Screenshots
Note: This field is mandatory for UI changes (non-component work should NOT have screenshots).
Before | After | |
---|---|---|
Mobile | ||
Desktop |
What areas of the site does it impact?
(Describe what parts of the site are impacted if code touched other areas)
Acceptance criteria
Quality Assurance & Testing
- [ ] I fixed|updated|added unit tests and integration tests for each feature (if applicable).
- [ ] No sensitive information (i.e. PII/credentials/internal URLs/etc.) is captured in logging, hardcoded, or specs
- [ ] Linting warnings have been addressed
- [ ] Documentation has been updated (link to documentation *if necessary)
- [ ] Screenshot of the developed feature is added
- [ ] Accessibility testing has been performed
Error Handling
- [ ] Browser console contains no warnings or errors.
- [ ] Events are being sent to the appropriate logging solution
- [ ] Feature/bug has a monitor built into Datadog or Grafana (if applicable)
Authentication
- [ ] Did you login to a local build and verify all authenticated routes work as expected with a test user
:warning: Team Sites (only applies to modifications made to the VA.gov header) :warning:
- [ ] The vets-website header does not contain any web-components
- [ ] I used the proxy-rewrite steps to test the injected header scenario
- [ ] I reached out in the
#sitewide-public-websites
Slack channel for questions
Requested Feedback
(OPTIONAL) What should the reviewers know in addition to the above. Is there anything specific you wish the reviewer to assist with. Do you have any concerns with this PR, why?
@acald-creator I was able to build successfully with node 14 & node 16.18.1. I did run into errors building higher than 16 (I used 18.18.0 and 19.6.0) so that could possible spawn another ticket for investigation and/or a fix
ERROR in ./src/platform/site-wide/wc-loader.js <- there's 109 errors total across vets-website,
Module build failed (from ./node_modules/babel-loader/lib/index.js):
Error: error:0308010C:digital envelope routines::unsupported
@acald-creator I was able to build successfully with node 14 & node 16.18.1. I did run into errors building higher than 16 (I used 18.18.0 and 19.6.0) so that could possible spawn another ticket for investigation and/or a fix
ERROR in ./src/platform/site-wide/wc-loader.js <- there's 109 errors total across vets-website, Module build failed (from ./node_modules/babel-loader/lib/index.js): Error: error:0308010C:digital envelope routines::unsupported
Already have an open ticket on this to track the error. Open ticket here has been updated to reflect this: https://app.zenhub.com/workspaces/identity-5f5bab705a94c9001ba33734/issues/gh/department-of-veterans-affairs/va.gov-team/68616
Due to the magnitude of what's being updated here, we're going to definitely want to review this one closely in a review instance. Additionally, I'd like to request that in addition to the standard FE COP review, that we have someone from the design system team review this as well to ensure that no components were unintentionally impacted.
:wave: Hey there! I'm one of the engineers on the Design System Team. I think it's great y'all are working on this. I'm not sure if you've seen the prior discovery conducted on this particular issue, and in case you haven't there are two pieces:
- https://github.com/department-of-veterans-affairs/va.gov-team/issues/37351.
- Analysis on vets-website adjacent repos
Just want to make sure your team has as much historical context as possible for this. Let me know if there's anything the design system team can do to assist.
👋 Hey there! I'm one of the engineers on the Design System Team. I think it's great y'all are working on this. I'm not sure if you've seen the prior discovery conducted on this particular issue, and in case you haven't there are two pieces:
Just want to make sure your team has as much historical context as possible for this. Let me know if there's anything the design system team can do to assist.
I'm aware of the first open issue.
The only change that I believe could impact is the @forward rule
, and while this is commented out in the meantime, because building with this on, will not work. It is necessary to upgrade USWDS to v3 in order to even use this rule. As I can see that there seems to be workarounds, this line is commented out for the time being, as there could be plans in the future to migrate to USWDS 3 internally in this repo, as well as formation.
There is no change to Foundation Sites and USWDS at this time. I've attempted to do so in a separate test repo, but found it was too cumbersome, and saw the latest work from the component library which made me rethink the approach.
I also intend to upgrade the node engine as well to at least v16, but not v18, as there are other issues with babel and other packages that needs to be taken care of first.
Please take a look and provide guidance if you will, I would like to see and make this happen, and will also like to help out in the meantime.
@micahchiang it looks like you shared a confluence link. That is not open to everyone (just open to Platform Crew). So if you'd like to share that documentation, you'll need to do it in a different way.
@andreahewitt-odd - ah thanks for the reminder. Sorry about that!
@acald-creator - the initiatives the design system team are working on currently are to sunset Formation in favor of css-library, which will use USWDS v3 as one of its dependencies.
@acald-creator - the initiatives the design system team are working on currently are to sunset Formation in favor of css-library, which will use USWDS v3 as one of its dependencies.
@micahchiang Thanks for bringing that up. I did see and acknowledge the css-library
efforts.
Since I am not able to access the Confluence page, do you have a timetable on sunsetting Formation for css-library
?
This PR is stale because it has been open 120 days with no activity. It will recieve a stale label every day for 14 days before being closed unless it recieves a comment or the stale label is removed.
This PR is stale because it has been open 120 days with no activity. It will recieve a stale label every day for 14 days before being closed unless it recieves a comment or the stale label is removed.
awaiting PR to upgrade to node engine 20.9.0
- https://github.com/department-of-veterans-affairs/vets-website/pull/31931