Skip to content

Dart-sass migration - #46235

Merged
tim-dot-org merged 6 commits into
stagingfrom
tim/dart-sass-migration
May 9, 2022
Merged

Dart-sass migration#46235
tim-dot-org merged 6 commits into
stagingfrom
tim/dart-sass-migration

Conversation

@tim-dot-org

@tim-dot-org tim-dot-org commented May 6, 2022

Copy link
Copy Markdown
Contributor

This PR splits out the dart-sass migration from the webpack upgrade PR.
Node-sass is officially deprecated and all users are encouraged to switch to dart-sass, which has the same api, but is better supported. During the course of the webpack upgrade, we ran into several issues with our scss loading and dart-sass also provides much better debug error messages.

  • Replaces node-sass with dart-sass.
  • outputStyle needed to be changed.
  • sass-loader needed to be upgraded.
  • Ran the sass-migration tool to remove division syntax deprecation warnings.
  • A couple scss files needed to explicitly include 'color' as 'color.scss', which is potentially due to our weird convertScssToJs script, but I'm unsure what the connection there is. Only those two files required changes, despite color being used extensively, and no other imports needed to be changed apart from color.

Links

dart-sass breaking changes
webpack upgrade doc

Testing story

Loaded main page and applab and everything looks styled.

@tim-dot-org
tim-dot-org requested a review from jamescodeorg May 6, 2022 22:42

@jamescodeorg jamescodeorg left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks great! Could you just add a note about why we decided to move to dart-sass in the PR comments? Thanks for taking care of this!

@tim-dot-org
tim-dot-org merged commit 356896c into staging May 9, 2022
@tim-dot-org
tim-dot-org deleted the tim/dart-sass-migration branch May 9, 2022 19:59
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.

2 participants