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

it throws a deprecation warning : Slash as Division #121

Closed
hiratsuka-r opened this issue May 17, 2022 · 8 comments
Closed

it throws a deprecation warning : Slash as Division #121

hiratsuka-r opened this issue May 17, 2022 · 8 comments

Comments

@hiratsuka-r
Copy link

It will be my first time participating.
it throws a deprecation warning... it works for now.

see: https://sass-lang.com/d/slash-div

Deprecation Warning: Using / for division outside of calc() is deprecated and will be removed in Dart Sass 2.0.0.
Recommendation: math.div($base-line-height, $base-font-size) or calc($base-line-height / $base-font-size)
More info and automated migrator: https://sass-lang.com/d/slash-div

   ╷
32 │ $base-rhythm-unit: $base-line-height / $base-font-size * $font-unit;
   │                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   ╵
    node_modules/compass-mixins/lib/compass/typography/_vertical_rhythm.scss 32:20  @import
    node_modules/compass-mixins/lib/compass/_typography.scss 4:9                    @import
    node_modules/compass-mixins/lib/_compass.scss 3:9                               @import

You can avoid it by doing this for the time being.

$base-rhythm-unit: calc($base-line-height / $base-font-size) * $font-unit

node version : v16.14.2

@blaiz
Copy link

blaiz commented May 19, 2022

We're having the exact same issue here. We were able to fix all the instances in our own code by wrapping divisions with calc() but we can't fix the warnings related to compass-mixins since it's in node_modules. Could the fix be as simple as wrapping those in calc()? I could open a PR if that's the case.

@hiratsuka-r
Copy link
Author

hiratsuka-r commented May 20, 2022

@blaiz
Thank you for your comment.

Could the fix be as simple as wrapping those in calc ()?

Yes, that's right. The official page also says:

This deprecation does not affect uses of / inside calc () expressions.

Alternatively, users can wrap division operations inside a calc() expression, which Sass will simplify to a single number.
// WRONG, will not work in future Sass versions.
@debug (12px/4px); // 3

// RIGHT, will work in future Sass versions.
@debug calc(12px / 4px); // 3

https://sass-lang.com/d/slash-div
We apologize for the inconvenience, but we would be grateful if you could fix it.

@hiratsuka-r
Copy link
Author

hiratsuka-r commented May 20, 2022

I got the following warning:

Deprecation Warning: Using / for division outside of calc() is deprecated and will be removed in Dart Sass 2.0.0.

32 │ $base-rhythm-unit: $base-line-height / $base-font-size * $font-unit;
   │                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    node_modules/compass-mixins/lib/compass/typography/_vertical_rhythm.scss 32:20  @import


36 │ $base-leader: ($base-line-height - $base-font-size) * $font-unit / $base-font-size;
   │               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    node_modules/compass-mixins/lib/compass/typography/_vertical_rhythm.scss 36:15  @import


40 │ $base-half-leader: $base-leader / 2;
   │                    ^^^^^^^^^^^^^^^^
    node_modules/compass-mixins/lib/compass/typography/_vertical_rhythm.scss 40:20  @import


120 │   $rhythm: $font-unit * ($lines * $base-line-height - $offset) / $font-size;
    │            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    node_modules/compass-mixins/lib/compass/typography/_vertical_rhythm.scss 120:12  rhythm()

Maybe there are other things I missed.
I'm not confident ..., but I also get the source and are looking for parts to repair.

@Igosuki
Copy link
Owner

Igosuki commented May 22, 2022

Could you please submit a PR with the changes ? Thanks

@blaiz
Copy link

blaiz commented May 22, 2022 via email

@hiratsuka-r
Copy link
Author

hiratsuka-r commented May 23, 2022

I don't get any warnings at hand.

Pushing to github.com:Igosuki/compass-mixins.git
ERROR: Permission to Igosuki/compass-mixins.git denied to hiratsuka-r.
fatal: Could not read from remote repository.

Please add to the team with write access to the repository.
↑ It doesn't matter if it's temporary.

@blaiz
Copy link

blaiz commented May 23, 2022

Here's a PR written by my coworker that fixes the current issue

@hiratsuka-r
Copy link
Author

@blaiz @SiriVadlapudi
I'm sorry it took a long time
Thank you for the fix! I checked the contents of the pull request.

There was only one difference from what I fixed. I made a comment.
Please check them when you have time.

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

No branches or pull requests

3 participants