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.

Sign up for GitHub

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

Jump to bottom

Migrate from node-sass to sass #27254

Merged
merged 4 commits into from
Jun 24, 2024
Merged

Migrate from node-sass to sass #27254

merged 4 commits into from
Jun 24, 2024

Conversation

domlander
Copy link
Contributor

@domlander domlander commented Jun 24, 2024 •
edited
Loading

What is the value of this and can you measure success?

The node-sass project is deprecated in favour of Dart SASS.

What does this change?

Uses the Dart SASS implementation of SASS instead of node-sass.

  • Tested locally
  • Tested on CODE

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jun 24, 2024 •
edited
Loading

Deploy build 2218 of dotcom:frontend-all to CODE

All deployment options

From guardian/actions-riff-raff.

@domlander domlander marked this pull request as ready for review June 24, 2024 14:07
@domlander domlander requested a review from a team as a code owner June 24, 2024 14:07
@domlander domlander changed the title Migrate From node-sass To sass Migrate from node-sass to sass Jun 24, 2024
mxdvl
mxdvl approved these changes Jun 24, 2024
View reviewed changes
Copy link
Contributor

@mxdvl mxdvl left a comment

Choose a reason for hiding this comment

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

lovely 🎯

package.json Outdated
@@ -121,6 +120,7 @@
"requireindex": "^1.1.0",
"rimraf": "^2.6.1",
"rxjs": "^6.6.7",
"sass": "^1.77.6",
Copy link
Contributor

Choose a reason for hiding this comment

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

our recommendation is to pin dependencies to a patch, rather than using ranges.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I shall pin the SASS relevant dependencies

@domlander domlander force-pushed the doml/migrate-node-sass-to-sass branch from 971f023 to ea4d547 Compare June 24, 2024 14:22
@domlander domlander merged commit b4417de into main Jun 24, 2024
6 checks passed
@domlander domlander deleted the doml/migrate-node-sass-to-sass branch June 24, 2024 14:43
@prout-bot
Copy link
Collaborator

Seen on ADMIN-PROD (merged by @domlander 12 minutes and 2 seconds ago)

@prout-bot
Copy link
Collaborator

Seen on FRONTS-PROD (merged by @domlander 13 minutes and 6 seconds ago)

@domlander domlander linked an issue Jun 24, 2024 that may be closed by this pull request
Direct dependency node-sass is incompatible with recent versions of Python #27227
Closed
@arelra
Copy link
Member

arelra commented Jun 26, 2024

@domlander I noticed that the make compile build log now contains a large number of sass deprecation warnings:

https://github.com/guardian/frontend/actions/runs/9681667479/job/26712785987#step:9:2452

If we don't intend to fix these is there a way to silence them?

@domlander
Copy link
Contributor Author

@domlander I noticed that the make compile build log now contains a large number of sass deprecation warnings:

https://github.com/guardian/frontend/actions/runs/9681667479/job/26712785987#step:9:2452

If we don't intend to fix these is there a way to silence them?

I have started looking into this. I'll create a ticket to track.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mxdvl mxdvl mxdvl approved these changes

Assignees
No one assigned
Labels
Seen-on-ADMIN-PROD Seen-on-FRONTS-PROD
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Direct dependency node-sass is incompatible with recent versions of Python
4 participants
@domlander @prout-bot @arelra @mxdvl