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.

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

Switch to using the dart version of sass instead of node-sass #11019

Merged
merged 2 commits into from Nov 17, 2020

Conversation

niik
Copy link
Member

@niik niik commented Nov 12, 2020

Superseedes #10949

Description

In #10949 @dennisameling graciously updated our version of node-sass to v5 which is required to move us forward with the Windows/arm64 work. This PR takes a slightly different approach by switching the engine out entirely in favor of the "pure JS" version of the sass compiler. In theory this could affect build performance negatively but if anything it appears to be faster from the few tests I've done though that could be related to the upgraded sass-loader as well.

See #10949 (comment)

Hey @dennisameling, thanks for opening this! I'd be happy to merge this as-is but I'm wondering if we should just switch out node-sass in favor of sass and avoid the native compilation step altogether. It seems that's what webpack is recommending these days

ℹ️ We recommend using Dart Sass.

I believe we'd need to swap node-sass for sass in addition to bumping sass-loader to the latest version.

This will rid us of one more native module which will be helpful with both the Windows ARM64 and the upcoming macOS ARM64 builds.

Copy link
Contributor

@dennisameling dennisameling left a comment

Choose a reason for hiding this comment

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

Thanks! Can confirm this works on arm64 Windows 🚀

@niik niik merged commit 159727a into development Nov 17, 2020
@niik niik deleted the dart-sass branch November 17, 2020 09:33
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.

None yet

2 participants