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

Support for Sass >= 1.33 #576

Closed
Ei-aaie opened this issue Jun 24, 2022 · 11 comments
Closed

Support for Sass >= 1.33 #576

Ei-aaie opened this issue Jun 24, 2022 · 11 comments
Labels

Comments

@Ei-aaie
Copy link

Ei-aaie commented Jun 24, 2022

Hello, since sass 1.33.0 (dart-sass), they introduced deprecation warning for division operation with slash.

See : https://sass-lang.com/documentation/breaking-changes/slash-div

I use ContentTools in one of my project, and got a lot of warnings that I can't fix :)

2 options : using css calc() or @use 'sass:math';

Thanks !

@anthonyjb anthonyjb added the bug label Jun 24, 2022
@anthonyjb
Copy link
Member

@Ei-aaie thanks for the heads up on this - there's not a lot of CSS in the project and SASS provide a nice migration tool by the looks of it so I'll push a fix up this weekend.

@Ei-aaie
Copy link
Author

Ei-aaie commented Jun 26, 2022

I forked the project, it looks like it runs a Ruby sass version, not Dart-sass

The result is that the migration does the job, but the build fails at :
sass:build

Error: Invalid CSS after "...  $number: math": expected "}", was ".div($number, $..."
        on line 45 of external/styles/bourbon/helpers/_str-to-num.scss
        from line 25 of external/styles/bourbon/_bourbon.scss
        from line 5 of src/styles/content-tools.scss

40:       }
41: 
42:       @else {
43:         // Move the decimal dot to the left
44:         $divider: $divider * 10;
45:         $number: math.div($number, $divider);
46:       }
47: 
48:       $result: $result + $number;
49:     }
50:   }

sass:math lib is dart-sass specific :)
There is a dart-sass grunt package, but I'm not very familiar with grunt / coffee synthax
https://www.npmjs.com/package/grunt-dart-sass

@anthonyjb
Copy link
Member

So I think locally now we use libsass for comping the project, but there's probably still the old ruby sass requirements in the last release of CT. Will look into what's possible, it's likely some of if not all the divisions can be removed to be honest.

@Ei-aaie
Copy link
Author

Ei-aaie commented Jun 26, 2022

After compiling locally, ~50% can be replaced by multiplication without changing any deps, for example :

-        margin-top: -($dialogHeight / 2);
+        margin-left: -($dialogWidth * 0.5);

sass-migrator uses this kind of smart fix if t's possible, and if not, applies math.div

@anthonyjb
Copy link
Member

@Ei-aaie thank you for the report and your work looking into the issue - I've pushed up a new release 1.6.15 which removes the reliance on the division operator I believe.

Multiplication would have been a good way to go on thinking about it but in the end I just when we declaring half sizes where convenient.

Let me know if you are still get any warnings in dart-scss now.

@Ei-aaie
Copy link
Author

Ei-aaie commented Jun 27, 2022

@anthonyjb It's almost it ! There are only 2 warnings left :
/external/styles/bourbon/functions/_strip-units.scss 4:12

and
/external/styles/bourbon/functions/_px-to-em.scss 12:12

@anthonyjb
Copy link
Member

Ahh that's an external library not part of ct - I'll see if I can remove ideally or upgrade if not.

@anthonyjb
Copy link
Member

@Ei-aaie if you look at the latest release of bourbon they don't support Dart SASS yet and had to revert the fix for div (thoughtbot/bourbon#1106 (comment)). So for now I'm just going to patch the version I include in CT if I can as they form part of the repo within the external directory. If that doesn't work I'll remove the dependency (but looking at it that's a bigger job so would be another weekend away).

@anthonyjb
Copy link
Member

@Ei-aaie ok can we try again, new release 1.6.16 - let me know.

@Ei-aaie
Copy link
Author

Ei-aaie commented Jun 27, 2022

Working like a charm ! Thank you !

@anthonyjb
Copy link
Member

@Ei-aaie great thank you for the help with issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants