Igosuki / compass-mixins

A collection of compass' stylesheet for bower dependencies and libsass
Other
592 stars 197 forks source link

it throws a deprecation warning : Slash as Division #121

Closed hiratsuka-r closed 2 years ago

hiratsuka-r commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

@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 commented 2 years ago

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 commented 2 years ago

Could you please submit a PR with the changes ? Thanks

blaiz commented 2 years ago

Will do.

On Sun, May 22, 2022, 00:22 Guillaume Balaine @.***> wrote:

Could you please submit a PR with the changes ? Thanks

— Reply to this email directly, view it on GitHub https://github.com/Igosuki/compass-mixins/issues/121#issuecomment-1133835468, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAMK6Z5WHLC7XPPPA27SSLTVLHODFANCNFSM5WDXRMKA . You are receiving this because you were mentioned.Message ID: @.***>

hiratsuka-r commented 2 years ago

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 commented 2 years ago

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

hiratsuka-r commented 2 years ago

@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.