Replace node-sass with sass (naïve attempt) #29
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
SimonAlling
changed the title
|
Superseded by #45. |
SimonAlling
added a commit
that referenced
this pull request
Apr 21, 2021
[Node Sass has been deprecated in favor of Dart Sass.][libsass] For this project in particular, Node Sass is the cause of lengthy compilations when running `npm ci`, annoying GYP errors and [multiple security vulnerabilities][snyk]. [libsass]: https://sass-lang.com/blog/libsass-is-deprecated [snyk]: https://snyk.io/test/npm/userscripter/1.4.0 When we first tried to migrate in #29, it turned out that ["just [replacing] `node-sass` in [our] `package.json` file with `sass`"][migrate] didn't work, because doing so broke our mechanism for accessing JavaScript values from within SCSS (i.e. `getGlobal`). For example, tests would fail like so: can expose data to SASS expect(received).toMatchInlineSnapshot(snapshot) Snapshot name: `can expose data to SASS 1` Snapshot: SassNumber {} Received: undefined [migrate]: https://sass-lang.com/blog/libsass-is-deprecated#how-do-i-migrate We tried to fix this by upgrading packages (e.g. `node-sass-utils` to version 1.1.3 and `sass-loader` to 11.0.1), but that would cause build errors in dependent userscripts like [Better SweClockers][bsc] (e.g. `SassError: Only 0 arguments allowed, but 1 was passed.` at `getGlobal` call sites). As far as we could tell, we wouldn't be able to fix that by upgrading dependencies, at least not without forcing dependent projects to upgrade to webpack 5. [bsc]: https://github.com/simonalling/better-sweclockers The solution we finally arrived at was to adapt `getGlobalFrom` to work with `sass` instead of `node-sass` and, in the webpack config we generate for dependent projects, make sure the parameters of the SASS variable getter are properly Dart Sass-encoded – conceptually: ```diff sassOptions: { functions: { - ["getGlobal"]: getGlobal, + ["getGlobal($x0)"]: getGlobal, }, }, ``` We decided that this is _not_ a breaking change, because although dependent projects using the SASS variable getter in JavaScript (i.e. not in SCSS) may break, we don't consider that a supported use case (and we don't know of any such projects). Dependent projects using `getGlobal` as intended (i.e. in SCSS) shouldn't be affected. In order to confirm this, we have built, tested and used Better SweClockers (as of [`f01d834`][bsc-commit]) based on this PR without any issues. [bsc-commit]: SimonAlling/better-sweclockers@f01d834 Co-authored-by: Simon Alling <alling.simon@gmail.com>
SimonAlling
added a commit
that referenced
this pull request
May 3, 2021
[Node Sass has been deprecated in favor of Dart Sass.][libsass] For this project in particular, Node Sass is the cause of lengthy compilations when running `npm ci`, annoying GYP errors and [multiple security vulnerabilities][snyk]. [libsass]: https://sass-lang.com/blog/libsass-is-deprecated [snyk]: https://snyk.io/test/npm/userscripter/1.4.0 When we first tried to migrate in #29, it turned out that ["just [replacing] `node-sass` in [our] `package.json` file with `sass`"][migrate] didn't work, because doing so broke our mechanism for accessing JavaScript values from within SCSS (i.e. `getGlobal`). For example, tests would fail like so: can expose data to SASS expect(received).toMatchInlineSnapshot(snapshot) Snapshot name: `can expose data to SASS 1` Snapshot: SassNumber {} Received: undefined [migrate]: https://sass-lang.com/blog/libsass-is-deprecated#how-do-i-migrate We tried to fix this by upgrading packages (e.g. `node-sass-utils` to version 1.1.3 and `sass-loader` to 11.0.1), but that would cause build errors in dependent userscripts like [Better SweClockers][bsc] (e.g. `SassError: Only 0 arguments allowed, but 1 was passed.` at `getGlobal` call sites). As far as we could tell, we wouldn't be able to fix that by upgrading dependencies, at least not without forcing dependent projects to upgrade to webpack 5. [bsc]: https://github.com/simonalling/better-sweclockers The solution we finally arrived at was to adapt `getGlobalFrom` to work with `sass` instead of `node-sass` and, in the webpack config we generate for dependent projects, make sure the parameters of the SASS variable getter are properly Dart Sass-encoded – conceptually: ```diff sassOptions: { functions: { - ["getGlobal"]: getGlobal, + ["getGlobal($x0)"]: getGlobal, }, }, ``` We decided that this is _not_ a breaking change, because although dependent projects using the SASS variable getter in JavaScript (i.e. not in SCSS) may break, we don't consider that a supported use case (and we don't know of any such projects). Dependent projects using `getGlobal` as intended (i.e. in SCSS) shouldn't be affected. In order to confirm this, we have built, tested and used Better SweClockers (as of [`f01d834`][bsc-commit]) based on this PR without any issues. [bsc-commit]: SimonAlling/better-sweclockers@f01d834 Co-authored-by: Simon Alling <alling.simon@gmail.com>
SimonAlling
added a commit
that referenced
this pull request
Jun 4, 2021
[Node Sass has been deprecated in favor of Dart Sass.][libsass] For this project in particular, Node Sass is the cause of lengthy compilations when running `npm ci`, annoying GYP errors and [multiple security vulnerabilities][snyk]. [libsass]: https://sass-lang.com/blog/libsass-is-deprecated [snyk]: https://snyk.io/test/npm/userscripter/1.4.0 When we first tried to migrate in #29, it turned out that ["just [replacing] `node-sass` in [our] `package.json` file with `sass`"][migrate] didn't work, because doing so broke our mechanism for accessing JavaScript values from within SCSS (i.e. `getGlobal`). For example, tests would fail like so: can expose data to SASS expect(received).toMatchInlineSnapshot(snapshot) Snapshot name: `can expose data to SASS 1` Snapshot: SassNumber {} Received: undefined [migrate]: https://sass-lang.com/blog/libsass-is-deprecated#how-do-i-migrate We tried to fix this by upgrading packages (e.g. `node-sass-utils` to version 1.1.3 and `sass-loader` to 11.0.1), but that would cause build errors in dependent userscripts like [Better SweClockers][bsc] (e.g. `SassError: Only 0 arguments allowed, but 1 was passed.` at `getGlobal` call sites). As far as we could tell, we wouldn't be able to fix that by upgrading dependencies, at least not without forcing dependent projects to upgrade to webpack 5. [bsc]: https://github.com/simonalling/better-sweclockers The solution we finally arrived at was to adapt `getGlobalFrom` to work with `sass` instead of `node-sass` and, in the webpack config we generate for dependent projects, make sure the parameters of the SASS variable getter are properly Dart Sass-encoded – conceptually: ```diff sassOptions: { functions: { - ["getGlobal"]: getGlobal, + ["getGlobal($x0)"]: getGlobal, }, }, ``` We decided that this is _not_ a breaking change, because although dependent projects using the SASS variable getter in JavaScript (i.e. not in SCSS) may break, we don't consider that a supported use case (and we don't know of any such projects). Dependent projects using `getGlobal` as intended (i.e. in SCSS) shouldn't be affected. In order to confirm this, we have built, tested and used Better SweClockers (as of [`f01d834`][bsc-commit]) based on this PR without any issues. [bsc-commit]: SimonAlling/better-sweclockers@f01d834 Co-authored-by: Simon Alling <alling.simon@gmail.com> Co-authored-by: Andrew Valleteau <avallete@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
https://sass-lang.com/blog/libsass-is-deprecated