Skip to content

Conversation

@afischer
Copy link
Member

@afischer afischer commented Jul 16, 2025

Description of Change

Updates our dependencies to resolve errors and warnings when building with current versions of node. Swaps out node-sass to use the non-deprecated sass module, and makes necessary associated changes as outlined here, most of which were done using the sass-migrator tool.

Related Issue

Closes #362

Motivation and Context

We aim to continue to support the latest releases of node whenever possible!

Checklist

  • Ran npm run lint and updated code style accordingly
  • npm run test passes
  • PR has a description and all contributors/stakeholder are noted/cc'ed
  • tests are updated and/or added to cover new code
  • relevant documentation is changed and/or added

@afischer afischer self-assigned this Jul 16, 2025
@afischer afischer marked this pull request as ready for review July 16, 2025 14:18
Copy link
Contributor

@achavez achavez left a comment

Choose a reason for hiding this comment

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

This all makes sense to me. Just flagged one potential change to reduce the risk to the deployed app a tiny bit, but defer to you on the final decision.

@@ -1,4 +1,4 @@
FROM node:18
FROM node:24
Copy link
Contributor

Choose a reason for hiding this comment

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

Only question I have is whether it would be less risky to upgrade this to the latest LTS while still running the matrix tests to make sure we're ready to go all the way to the latest/current/active ... but defer 100% to you on that.

Copy link
Member Author

Choose a reason for hiding this comment

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

Generally, I think we've unofficially tried to support the current version as our recommended version of node to use in deployments, but I do like the idea of using the matrix for smoke testing in the future!

@afischer afischer merged commit 58c9ce9 into main Jul 16, 2025
4 checks passed
@afischer afischer deleted the node-lts-2025 branch July 16, 2025 14:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Build fails on node 20 (node-sass version in package-lock.json stops working after node 18)

2 participants