ubyssey.ca icon indicating copy to clipboard operation
ubyssey.ca copied to clipboard

Remove deprecated `node-sass` package, replace with `sass`

Open psiemens opened this issue 1 year ago • 7 comments

What problem does this PR solve?

This PR updates gulp-sass and node-sass to work with newer versions of Node. Closes #1144

How did you fix the problem?

node-sass was deprecated in 2020 so I replaced it with Dart Sass (as recommended in this announcement), which is distributed as sass on NPM. I also had to update gulp-sass to a version that is compatible with sass.

psiemens avatar Jul 17 '23 22:07 psiemens

I confirmed locally that this upgrade works with Node v14, the version used in the Ubyssey production build.

psiemens avatar Jul 17 '23 22:07 psiemens

Note: This may also effect the #1145 branch as the deprecation of node-sass and its update to 9.0.0 combined with the node version caused breaking changes to our codebase. I did a similar commit to #1145 as well to test it.

@psiemens

FireBoyAJ24 avatar Jul 18 '23 04:07 FireBoyAJ24

the deprecation of node-sass and its update to 9.0.0 combined with the node version caused breaking changes to our codebase

@FireBoyAJ24 Do you mean that [email protected] broke with the current Node version (v14)? I can confirm that the same thing happened to me, but sass should actually work with Node v14 unless I'm mistaken.

Either way, we probably only need to merge one of the two PRs (this one or #1145).

psiemens avatar Jul 18 '23 22:07 psiemens

Do you mean that [email protected] broke with the current Node version (v14)?

Yes. I looked at node-sass on npm website and found that it would be outside of the supported node versions.

sass should actually work with Node v14 unless I'm mistaken.

Yes, it should work.

I believe it is best that we merge with #1153 and then allow the dependent bot pull from origin and update the packages to be updated. Then we can merge with #1145.

FireBoyAJ24 avatar Jul 19 '23 03:07 FireBoyAJ24

Steps:

  1. test
  2. merger
  3. deploy Next steps in new release: update node to 18

brittkhat avatar Aug 01 '23 17:08 brittkhat

When we update node-sass we get breaking changes. Gist: / is used for division and as a CSS separator. To reduce the confusion the slash is not used for division in Sass now.

I have added calc function to prevent breaking changes as recommended by node sass. You can see all the changes in the above commit.

FireBoyAJ24 avatar Aug 02 '23 06:08 FireBoyAJ24

Thanks @FireBoyAJ24! I brought this branch up to date with develop, so it should be ready to merge now.

psiemens avatar Aug 10 '23 00:08 psiemens