Skip to content

refactor(multiple): fix warnings related to division operator in latest version of Sass #22871

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

Merged
merged 1 commit into from
Jun 8, 2021

Conversation

crisbeto
Copy link
Member

@crisbeto crisbeto commented Jun 3, 2021

The latest version of Sass prints a warning when the division operator is used. These changes migrate us to the recommended math.div function.

Note: there a few warnings left from MDC. I've submitted material-components/material-components-web#7158 to address them.

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

Fixes #22866.

@crisbeto crisbeto added P2 The issue is important to a large percentage of users, with a workaround target: patch This PR is targeted for the next patch release labels Jun 3, 2021
@crisbeto crisbeto requested a review from jelbourn June 3, 2021 15:08
@crisbeto crisbeto requested review from andrewseguin, devversion, mmalerba and a team as code owners June 3, 2021 15:08
@google-cla google-cla bot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Jun 3, 2021
@josephperrott josephperrott removed the request for review from a team June 3, 2021 15:10
@crisbeto crisbeto changed the title refactor(multiple): fix warnings related to divsion operator in latest version of Sass refactor(multiple): fix warnings related to division operator in latest version of Sass Jun 3, 2021
@crisbeto crisbeto force-pushed the 22866/sass-division branch from 0e8d0e7 to 7aa3bd9 Compare June 3, 2021 16:02
Copy link
Member

@devversion devversion left a comment

Choose a reason for hiding this comment

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

LGTM, one minor nit from my side.

@crisbeto crisbeto force-pushed the 22866/sass-division branch from 7aa3bd9 to 15c6e77 Compare June 3, 2021 18:50
@crisbeto
Copy link
Member Author

crisbeto commented Jun 4, 2021

@jelbourn one thing we may want to consider here: the math.div function is available from Sass version 1.33.0 onwards whereas the CLI currently installs 1.32.12. Should we ask the CLI to update or delay this to a future version? Users that have updated Sass on their own will get a ton of warnings like in #22866 though. Alternatively, I can add our own helper for division that can be used to polyfill math.div.

@crisbeto crisbeto force-pushed the 22866/sass-division branch from 15c6e77 to 20bcfd8 Compare June 7, 2021 19:51
@crisbeto
Copy link
Member Author

crisbeto commented Jun 7, 2021

I've reworked the changes to make them backwards-compatible with Sass less than 1.34.0.

@crisbeto crisbeto added the action: merge The PR is ready for merge by the caretaker label Jun 7, 2021
@crisbeto crisbeto force-pushed the 22866/sass-division branch from 20bcfd8 to c7d76cf Compare June 7, 2021 19:56
Copy link
Member

@jelbourn jelbourn left a comment

Choose a reason for hiding this comment

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

LGTM

…st version of Sass

The latest version of Sass prints a warning when the division operator is used. These changes migrate us to the recommended `math.div` function.

Fixes angular#22866.
@crisbeto crisbeto force-pushed the 22866/sass-division branch from c7d76cf to 25126cd Compare June 8, 2021 04:38
@andrewseguin andrewseguin added target: minor This PR is targeted for the next minor release and removed target: patch This PR is targeted for the next patch release labels Jun 8, 2021
@andrewseguin andrewseguin merged commit a4043f4 into angular:master Jun 8, 2021
@fredericojesus
Copy link

How do I know when this is going to be released ? Thanks in advance.

@crisbeto
Copy link
Member Author

It'll be released in version 12.1.

@crhistianramirez
Copy link

I am seeing it in 12.0.6 but not in any of the newer versions like 12.1.1 for example. Is this expected?

@crisbeto
Copy link
Member Author

crisbeto commented Jul 7, 2021

That sounds weird since we only merged it into the 12.1.0 branch.

@crhistianramirez
Copy link

Never mind! My mistake 😅

@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Aug 7, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker cla: yes PR author has agreed to Google's Contributor License Agreement P2 The issue is important to a large percentage of users, with a workaround target: minor This PR is targeted for the next minor release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug(Material SCSS): [DEPRECATION WARNING] Using / for division is deprecated and will be removed in Dart Sass 2.0.0.
6 participants