use dart sass, and update related modules#10208
Merged
brad-decker merged 1 commit intodevelopfrom Jan 19, 2021
Merged
Conversation
2f5f85c to
72d6875
Compare
72d6875 to
6826629
Compare
6826629 to
d14ec0c
Compare
Collaborator
Builds ready [d14ec0c]
Page Load Metrics (481 ± 64 ms)
|
Member
|
Do any of these differences affect us? |
Member
|
The easiest way to tell might be to compare the .css output before and after this PR. |
Contributor
Author
|
@Gudahtt i'll do that, and report back -- i did do a number of things on the build presented by the bot in this PR to make sure there weren't glaring issues and nothing stuck out to me but I'll examine css output. |
Contributor
Author
|
the differences between the two files are plentiful, but they appear to be entirely changes in white space/formatting. Examples: before this change: .tab-bar__tab__caret {
display: none; }
@media screen and (max-width: 575px) {
.tab-bar__tab__caret {
display: block;
background-image: url("/images/caret-right.svg");
width: 36px;
height: 36px;
opacity: 0.5;
background-size: contain;
background-repeat: no-repeat;
background-position: center; }
[dir='rtl'] .tab-bar__tab__caret {
-webkit-transform: rotate(180deg);
transform: rotate(180deg); } }
.transaction-activity-log__activity::after {
content: '';
position: absolute;
left: 0;
top: 0;
height: 100%;
width: 7px;
border-right: 1px solid #909090; }after this change: .tab-bar__tab__caret {
display: none;
}
@media screen and (max-width: 575px) {
.tab-bar__tab__caret {
display: block;
background-image: url("/images/caret-right.svg");
width: 36px;
height: 36px;
opacity: 0.5;
background-size: contain;
background-repeat: no-repeat;
background-position: center;
}
[dir=rtl] .tab-bar__tab__caret {
-webkit-transform: rotate(180deg);
transform: rotate(180deg);
}
}
.transaction-activity-log__activity::after {
content: "";
position: absolute;
left: 0;
top: 0;
height: 100%;
width: 7px;
border-right: 1px solid #909090;
}The only differences that concerned me were these: .fa-stopwatch:before {
content: "\f2f2"; }.fa-stopwatch:before {
content: "";
}which after researching I found sass/dart-sass#568 -- I also double checked and the font-awesome fonts are still working on storybook and the extension. |
This file contains hidden or 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
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
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.
Rationale
At some point in the future, I would like to refactor the way we use our scss files to make use of the
@userule. switching fromnode-sasstosassis a requirement of doing that, becausenode-sassbinds tolibsasswhich hasn't received new language features since November 2018. The official implementation of sass isdart-sasswhich has js bindings through thesassdependency.More short-termed: I have need of the
math.powfunction that was added officially to themathmodule but never landed inlibsassThis PR
node-sassdependency (gulp-sass still depends on it and it must be compiled in prep-deps on circleci).sass(js bindings todart-sass)adds fibers, which is recommended by bothdespite this recommendation, thegulp-sassandsass-loaderfor improving performance during compilation.fiberspackage is marked as deprecated and heavily discouraged from use. Furthermore, it didn't appear to do anything in terms of performance on the build. I removed this complexity from the PR.sass-loadersassas implementation tosass-loaderdesign-system/index.scssfor@forwardits contents,which should have zero impact on the final result(1). However, ifnode-sasswas still being used it would fail to compile.@forwarddoesn't forward maps? I'm not sure what happened there but in a future PR where I used$color-mapfromdesing-system/colorsit didn't work until I@use'd colors directly. This isn't blocking this PR because$color-mapis not used yet.