fix: update libsass to v0.21.0 #27683
Conversation
|
Hi @mraarif - Thanks for the link! I think the approach taken in this PR is a bit more 'surgical' and minimal, so I'm inclined to go forward with it. We manually walked through every error we encountered and fixed them individually, so it doesn't add any Are you aware of any issues in the code here? Happy to learn from your wisdom/effort! |
Updating libsass to latest. Libsass is now stricter about undefined placeholder mixins, and so we had to mark a bunch of broken code as !optional. We had upgraded libsass until it broke with this version: https://github.com/sass/libsass/releases/tag/3.3.3 That release of libsass included a fix for a missing error on undefined placeholders. Our code was always wrong, now it just complains. This PR fixes it.
davidjoy
force-pushed
the
djoy/update_libsass_to_0.20.1
branch
from
412cf73 to
935e173
Compare
|
seems like you know what you're up against, I just mentioned it if you could use the context and discoveries we made in that PR |
abutterworth
changed the title
No longer rely on themes to import bootstrap functions
|
Your PR has finished running tests. There were no failures. |
|
📣 💥 Heads-up: You must either rebase onto master or merge master into your branch to avoid breaking the build. We recently removed diff-quality and introduced lint-amnesty. This means that the automated quality check that has run on your branch doesn't work the same way it will on master. If you have introduced any quality failures, they might pass on the PR but then break the build on master. This branch has been detected to not have commit 2e33565 as an ancestor. Here's how to see for yourself: If you have any questions, please reach out to the Architecture team (either #edx-shared-architecture on Open edX Slack or #architecture on edX internal). |
|
This work has stalled. See #31616. |
Updates libsass to latest. Requires an update to the edx.org-next theme https://github.com/edx/edx-themes/pull/737
Merge https://github.com/edx/edx-themes/pull/737 before this PR
Libsass is now stricter about undefined placeholder mixins, and so we had to mark a bunch of broken code as
!optional. We had upgraded libsass until it broke with this version: https://github.com/sass/libsass/releases/tag/3.3.3 That release of libsass included a fix for a missing error on undefined placeholders. Our code was has existing missing placeholders, now it just complains. This PR fixes those.Libsass
@extendseems to have changed behavior in such a way that our usage of it our theme bloats the css output to a point where it's either 10s of megabytes or simply fails to compile. https://github.com/edx/edx-themes/pull/737 addresses this.Key file sizes
Sandbox: https://libsass-upgrade.sandbox.edx.org/
(edx-app: djoy/update_libsass_to_0.20.1)
(edx-themes: abutterworth/libsass-upgrade)
Create Sandbox Job: https://tools-edx-jenkins.edx.org/job/sandboxes/job/CreateSandbox/39080/
Testing