-
Notifications
You must be signed in to change notification settings - Fork 319
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
Replace node-sass with Dart Sass #3197
Conversation
LibSass has been deprecated. Their maintainers [recommend](https://sass-lang.com/blog/libsass-is-deprecated) switching to Dart Sass instead. Test plan: ran `yarn start` and `yarn build`; no visual changes.
This pull request is being automatically deployed with Vercel (learn more). nusmods-website – ./website🔍 Inspect: https://vercel.com/nusmodifications/nusmods-website/fylg2kfrn nusmods-export – ./export🔍 Inspect: https://vercel.com/nusmodifications/nusmods-export/quzpracd6 |
Codecov Report
@@ Coverage Diff @@
## master #3197 +/- ##
=======================================
Coverage 55.44% 55.44%
=======================================
Files 254 254
Lines 5317 5317
Branches 1227 1227
=======================================
Hits 2948 2948
Misses 2369 2369 Continue to review full report at Codecov.
|
Deployment preview for |
Merging without approval to unblock #3155 |
Is there a diff in the CSS output? I think that's the easiest way to check for regressions |
I did try to diff the generated CSS, but it was minified so it wasn't useful 😆 Anyway I figured it was unlikely that anything would break so I didn't pursue it further. This is what I did: # Remove `contenthash` from `filename` and `chunkFilename` in `MiniCssExtractPlugin` config object in webpack.parts.js.
git checkout master
yarn && yarn build
mv dist dist-nodesass
git checkout eliang/rip-node-sass
yarn && yarn build
diff dist dist-nodesass |
LibSass has been deprecated. Their maintainers recommend switching to Dart Sass instead.
Test plan: ran yarn start and yarn build; no apparent changes.