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

web: replace node-sass with sass #5846

Merged
merged 1 commit into from
Jun 3, 2022
Merged

web: replace node-sass with sass #5846

merged 1 commit into from
Jun 3, 2022

Conversation

milas
Copy link
Contributor

@milas milas commented Jun 3, 2022

The node-sass package has been deprecated and superseded by the
sass package.

The node-sass package used a C library via node-gyp, which itself
requires Python to wrangle the compilation. The version of node-sass
we were using was old and thus relied on an old version of node-gyp,
which only supported <= Python 3.8.

This caused problems with Homebrew builds with embedded assets, as
Python 3.9 is used there, so node-gyp would error out.

While it's theoretically possible to upgrade node-sass to a newer
version to transitively get a newer node-gyp that will not bail on
Python 3.9, that did NOT work out of the box, and the recommended
solution for the problem was...switch to sass, which does not have
any native code requirements and is actively supported.

As part of this, the sass compiler emitted warnings on deprecated
division outside of calc(), so those have all been updated to avoid
producing a bunch of noise in the build log.

I did a visual pass after this change and didn't see any differences
in styling in Chrome on macOS.

tl;dr: This will dramatically simplify producing Homebrew builds with
embedded assets by sidestepping the need for a Python/node-gyp
build chain just for bundling the UI assets.

@milas milas added dependencies Pull requests that update a dependency file ui labels Jun 3, 2022
@milas milas requested review from nicksieger and landism June 3, 2022 18:09
@milas milas self-assigned this Jun 3, 2022
The `node-sass` package has been deprecated and superseded by the
`sass` package.

The `node-sass` package used a C library via `node-gyp`, which itself
requires Python to wrangle the compilation. The version of `node-sass`
we were using was old and thus relied on an old version of `node-gyp`,
which only supported <= Python 3.8.

This caused problems with Homebrew builds with embedded assets, as
Python 3.9 is used there, so `node-gyp` would error out.

While it's theoretically possible to upgrade `node-sass` to a newer
version to transitively get a newer `node-gyp` that will not bail on
Python 3.9, that did NOT work out of the box, and the recommended
solution for the problem was...switch to `sass`, which does not have
any native code requirements and is actively supported.

As part of this, the `sass` compiler emitted warnings on deprecated
division outside of `calc()`, so those have all been updated to avoid
producing a bunch of noise in the build log.

I did a visual pass after this change and didn't see any differences
in styling in Chrome on macOS.

tl;dr: This will dramatically simplify producing Homebrew builds with
       embedded assets by sidestepping the need for a Python/`node-gyp`
       build chain just for bundling the UI assets.
Copy link
Member

@landism landism left a comment

Choose a reason for hiding this comment

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

nice SHA

@milas milas merged commit 88a5ed7 into master Jun 3, 2022
@milas milas deleted the milas/node-sass branch June 3, 2022 19:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file ui
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants