Igosuki / compass-mixins

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

SASS: replace deprecated use of slash for division #118

Closed noahott closed 2 years ago

noahott commented 2 years ago

Replace deprecated usage of slash for division using slash-migrator https://sass-lang.com/documentation/breaking-changes/slash-div

Igosuki commented 2 years ago

At which version does sass have the math module ?

noahott commented 2 years ago

I think it's version 1.23.0 https://sass-lang.com/documentation/modules/math

Igosuki commented 2 years ago

@noahott The @use directives are only available in Dart Sass. c.f. Only Dart Sass currently supports loading built-in modules with @use. Users of other implementations must call functions using their global names instead.

noahott commented 2 years ago

I was able to replace the usage of the Math module in _tag-cloud.scss with multiplication. I don't see any other way around this for _vertical_rythm.scss and _grid_background.scss.

Bootstrap and FontAwesome both implement a custom divide function so as to not need the Math module: https://github.com/twbs/bootstrap/blob/ed13d0155905533e3f0f7c97b99a7803994e0953/scss/_functions.scss#L223

I've updated my PR to use this method, but with the function named cm-divide to protect against namespace collisions.

Igosuki commented 2 years ago

I'm getting the following errors, they don't happen on master.

Sass error: Error: 8px*em and 16px have incompatible units.
   ╷
16 │     @while ($remainder >= $divisor) {
   │             ^^^^^^^^^^^^^^^^^^^^^^
   ╵
  lib/compass/utilities/_math.scss 16:13              cm-divide()
  lib/compass/typography/_vertical_rhythm.scss 38:15  @import
  lib/compass/_typography.scss 4:9                    @import
  lib/_compass.scss 3:9                               @import
  lib/_animate.scss 1:9                               @import
  test/imports_animation.scss 1:9                     root stylesheet
    at Object.wrapException (../compass-mixins/node_modules/sass/sass.dart.js:1254:17)
    at ComplexSassNumber0._number1$_coerceOrConvertValue$6$coerceUnitless$name$other$otherName (../compass-mixins/node_modules/sass/sass.dart.js:87482:17)
    at ComplexSassNumber0.coerceValueToMatch$3 (../compass-mixins/node_modules/sass/sass.dart.js:87454:19)
    at ComplexSassNumber0.coerceValueToMatch$1 (../compass-mixins/node_modules/sass/sass.dart.js:87457:19)
    at ComplexSassNumber0._number1$_coerceUnits$1$2 (../compass-mixins/node_modules/sass/sass.dart.js:87581:16)
    at ComplexSassNumber0._number1$_coerceUnits$2 (../compass-mixins/node_modules/sass/sass.dart.js:87588:19)
    at ComplexSassNumber0.greaterThanOrEquals$1 (../compass-mixins/node_modules/sass/sass.dart.js:87509:21)
    at ../compass-mixins/node_modules/sass/sass.dart.js:74385:49
    at _wrapJsFunctionForAsync_closure.$protected (../compass-mixins/node_modules/sass/sass.dart.js:3737:15)
    at _wrapJsFunctionForAsync_closure.call$2 (../compass-mixins/node_modules/sass/sass.dart.js:27919:12) {
  formatted: 'Error: 8px*em and 16px have incompatible units.\n' +
    '   ╷\n' +
    '16 │     @while ($remainder >= $divisor) {\n' +
    '   │             ^^^^^^^^^^^^^^^^^^^^^^\n' +
    '   ╵\n' +
    '  lib/compass/utilities/_math.scss 16:13              cm-divide()\n' +
    '  lib/compass/typography/_vertical_rhythm.scss 38:15  @import\n' +
    '  lib/compass/_typography.scss 4:9                    @import\n' +
    '  lib/_compass.scss 3:9                               @import\n' +
    '  lib/_animate.scss 1:9                               @import\n' +
    '  test/imports_animation.scss 1:9                     root stylesheet',
  line: 16,
  column: 13,
  file: '../compass-mixins/lib/compass/utilities/_math.scss',
  status: 1
}
noahott commented 2 years ago

I see, this function doesn't handle the additional units properly. For example, in "8px*em / 16px" the pixel units should cancel out and leave 0.5em. Sass:math handles this fine, but I guess this isn't a suitable solution for this problem