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

build(sass): migrate from node-sass to dart sass #455

Merged
merged 2 commits into from
Jul 9, 2021

Conversation

Hopkinsa
Copy link
Contributor

@Hopkinsa Hopkinsa commented Jul 8, 2021

Description

LibSass and Node Sass are deprecated. While they will continue to receive maintenance releases indefinitely, there are no plans to add additional features or compatibility with any new CSS or Sass features. Projects that still use it should move onto Dart Sass.
This will result is a compile warning - DEPRECATION WARNING: Using / for division is deprecated and will be removed in Dart Sass 2.0.0.
Therefore, as part of update to Dart also update Sass to use math.div

During the migration, additional warnings were also observed i.e.

"export 'SpacingVariant' was not found in '../spacing.interface'
 @ ./projects/canopy/src/lib/spacing/margin/margin.module.ts
 @ ./projects/canopy/src/lib/card/card.stories.ts
 @ ./projects/canopy sync \.stories\.ts$
 @ ./.storybook/config.js
 @ multi ./node_modules/@storybook/core/dist/server/common/polyfills.js ./node_modules/@storybook/core/dist/server/preview/globals.js ./.storybook/config.js (webpack)-hot-middleware/client.js?reload=true&quiet=true

From stackoverflow (https://stackoverflow.com/questions/40841641/cannot-import-exported-interface-export-not-found)
The root cause is in Typescript and transpilation. Once TypeScript code is transpiled, interfaces/types are gone. They don't exist anymore in the emitted files.

While the types are erased, the imports/exports aren't necessarily. The reason is that there are ambiguities and that the compiler doesn't always know for sure whether something that is exported is a type or a value.

To correct these errors, 'type' was added to some interface imports

Fixes #452

@Hopkinsa Hopkinsa requested a review from a team as a code owner July 8, 2021 10:28
owensgit
owensgit previously approved these changes Jul 8, 2021
Copy link
Contributor

@owensgit owensgit left a comment

Choose a reason for hiding this comment

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

Nice work @Hopkinsa! 👍

Copy link
Contributor

@andy-polhill andy-polhill left a comment

Choose a reason for hiding this comment

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

This is great @Hopkinsa cheers!

My only minor comment is that I wondered if thetype changes should be in a separate commit? I think that they are unrelated to the dart sass upgrade, but they are definitely valuable.

Having them in a separate commit would just give us the opportunity to document the reason in the commit message, and maybe link out to that SO post. Apologies for the fussiness!

@Hopkinsa
Copy link
Contributor Author

Hopkinsa commented Jul 9, 2021

Agreed - got caught up in the moment removing warnings

@owensgit
Copy link
Contributor

owensgit commented Jul 9, 2021

Having them in a separate commit would just give us the opportunity to document the reason in the commit message, and maybe link out to that SO pose.

Great idea, I think having the SO link in the long description part of the message will be great 👍

Hopkinsa added 2 commits July 9, 2021 10:52
The root cause is in Typescript and transpilation. Once TypeScript code is transpiled,
interfaces/types are gone. They don't exist anymore in the emitted files.
https://stackoverflow.com/questions/40841641/cannot-import-exported-interface-export-not-found
LibSass and Node Sass are deprecated. While they will continue to receive maintenance releases
indefinitely, there are no plans to add additional features or compatibility with any new CSS or
Sass features. Projects that still use it should move onto Dart Sass

https://sass-lang.com/blog/libsass-is-deprecated

fix Legal-and-General#452
Copy link
Contributor

@andy-polhill andy-polhill left a comment

Choose a reason for hiding this comment

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

Nice one @Hopkinsa, thanks for breaking those commits up.

Copy link
Contributor

@owensgit owensgit left a comment

Choose a reason for hiding this comment

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

Nice work @Hopkinsa!

@owensgit owensgit merged commit ba48ffa into Legal-and-General:master Jul 9, 2021
@github-actions
Copy link
Contributor

🎉 This PR is included in version 6.4.4 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Node-Sass is being deprecated, update to Dart Sass
3 participants