jamonserrano / plumber-sass

Easy baseline grid with SASS
https://jamonserrano.github.io/plumber-sass
MIT License
258 stars 15 forks source link

plumber breaks with node-sass 4.9.4 on macOS Mojave #9

Closed rhedman closed 6 years ago

rhedman commented 6 years ago

Hello jamonserrano!

I've been using plumber for a while now using both grunt (node-sass, older version) and gulp (gulp-sass). In my current project I'm using strictly npm scripts to build my front-end layer and I'm having trouble with plumber causing node-sass to crash when compiling.

The first to issues was regarding variables. I had to move line 97 and 98 to be above line 96 eding up with this order

$line-height: $line-height $grid-height; $baseline-from-bottom: ($line-height - $font-size) / 2 + ($font-size $baseline); $corrected-baseline: round($baseline-from-bottom); $baseline-difference: $corrected-baseline - $baseline-from-bottom; `

But I also get a more serious error on line 115 and 117 saying

"message": "0.943remrem isn't a valid CSS value.", "formatted": "Error: 0.23575remrem isn't a valid CSS value.\n on line 106 of src/vendor/baseline/plumber.scss\n from line 13 of src/app.scss\n>> \t\t$padding-top: - $baseline-difference * $grid-height;\n\n ----------------^\n"

That's referring to these two lines of code

$padding-top: (1 - $baseline-difference) $grid-height; $padding-bottom: $baseline-difference $grid-height;

It seems like $grid-height is only getting the unit "rem" but without a value.

Have you seen this error before? Not sure how to proceed to fix it.

Thanks in advance!

jamonserrano commented 6 years ago

Hi @rhedman, thanks for the feedback! Can you share your plumber settings, the relevant parts in your code and the first error that made you reorder the rows? The second error is likely caused by the row reorder as now both $line-height and $baseline-difference are units (e.g. 2rem) and their product will have remrem or rem*rem unit.

I've rebuilt the plumber documentation page with node-sass 4.9.4 and didn't see any errors so it's possible that you have a use case that is not covered yet.

rhedman commented 6 years ago

Thanks for your reply Here's my plumber defaults

@include plumber-set-defaults( $grid-height: rem($base-unit), $baseline: $body-baseline, $font-size: 2, $line-height: 3, $leading-top: 0, $leading-bottom: 4 )

Which uses these variables:

$body-baseline: .198;

// Size base $size-base: 20px !default;

// Base units $base-unit: $size-base / 2 !default;

And this function

@function rem($size) { @return ($size / $size-base) * 1rem; }

They are included in the project like this

// Vendor @import './vendor/baseline/plumber';

// Settings @import './base/functions'; @import './base/variables';

This is the same setup I've used in all my other projects (I have also tried to change $grid-height in the plumber defaults to have a static value of 1rem without any luck)

And finally this is the error message I get (after changing everything back in my plumber sass.)

""message": "Undefined variable: \"$corrected-baseline\".", "formatted": "Error: Undefined variable: \"$corrected-baseline\".\n on line 96 of src/vendor/baseline/_plumber.scss, in mixin plumber\n from line 4 of src/components/typography/_typography.scss\n from line 16 of src/app.scss\n>> \t$baseline-difference: $corrected-baseline - $baseline-from-bottom;"

Line 4 of the typography sass file is line 1 of the plumber mixin: @include plumber(

h1 { @include plumber( $font-size: 3.5, $line-height: 4, $leading-bottom: 1, $baseline: $headings-baseline ); font-family: $font-family-headings; }

And line 16 in app.scss is the import of the typography sass @import './components/typography/typography';

rhedman commented 6 years ago

OK, I think I just figured this one out after hours of scratching my head. It's my SASS-lint config (with autofix) that keeps breaking plumber. It changes the order of some code blocks thus causing them to end up in the wrong order.

Thanks for your time (and sorry for wasting it) and thanks for Plumber. It's a great tool I use daily :)

jamonserrano commented 6 years ago

@rhedman Glad to hear you resolved it! I can probably add // sass-lint:disable-all to the top of _plumber.scss so it never gets linted together with the application code.

rhedman commented 6 years ago

I tried that and my linter accepts that fine but the auto-fixer doesn't and changes the file anyway. It also ignores my config where I specify that all files in the vendor-folder should be ignored. I will stop using the auto-fixer for now.