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

Include media 1.4.7 throws error #139

Closed
danez opened this issue Jul 29, 2016 · 18 comments
Closed

Include media 1.4.7 throws error #139

danez opened this issue Jul 29, 2016 · 18 comments
Labels

Comments

@danez
Copy link

danez commented Jul 29, 2016

This is the error we get with 1.4.7, it works fine with 1.4.6:

[exec] >> Error: Invalid unit ''.
     [exec] >>         on line 210 of node_modules/include-media/dist/_include-media.scss
     [exec] >> >>     @error $message;
     [exec] >>    ----^
     [exec] Warning:  Use --force to continue.
     [exec] 
     [exec] Aborted due to warnings.
     [exec] WARNING: Invalid unit ''.
     [exec] Backtrace:
     [exec]     node_modules/include-media/dist/_include-media.scss:212, in function `log`
     [exec]     node_modules/include-media/dist/_include-media.scss:486, in function `to-length`
     [exec]     node_modules/include-media/dist/_include-media.scss:457, in function `to-number`
     [exec]     node_modules/include-media/dist/_include-media.scss:350, in function `get-expression-value`
     [exec]     node_modules/include-media/dist/_include-media.scss:391, in function `parse-expression`
     [exec]     node_modules/include-media/dist/_include-media.scss:597, in mixin `media`
     [exec]     node_modules/include-media/dist/_include-media.scss:599, in mixin `media`

with the input:

@include media('screen', '<m')

and the variables being:

$breakpoints: (
  "s": 0px,
  "m": 768px,
  "l": 992px,
  "xl": 1200px,
  "xxl": 99999px
);
$media-expressions: (
  "screen": 'screen',
  "print": 'print',
  "handheld": 'handheld',
  "landscape": '(orientation: landscape)',
  "portrait": '(orientation: portrait)',
  "retina-2-x": '(-webkit-min-device-pixel-ratio: 2), (min-resolution: 192dpi)',
  "retina-3-x": '(-webkit-min-device-pixel-ratio: 3), (min-resolution: 350dpi)'
);
@weaintplastic
Copy link

This is due to the following commit: d2188bc from @IanMitchell

Error is in this line: https://github.com/eduardoboucas/include-media/blob/master/src/helpers/_parser.scss#L74

Calling get-expression-value($expression, $operator) with $expression: >=s and $operator: >= trim is returning an empty string.

@KittyGiraudel
Copy link
Collaborator

I cannot reproduce on SassMeister. :(

@danez
Copy link
Author

danez commented Jul 29, 2016

but sassmeister seems to have include-media 1.4.2

@KittyGiraudel
Copy link
Collaborator

KittyGiraudel commented Jul 29, 2016

I copied the 1.4.7 dist file.

@weaintplastic
Copy link

This is probably due to Libsass. Just try to execute the following in sassmeister and change from sass to Libsass.
For Sass this is working perfectly fine. Libsass is returning an empty string.

@charset 'UTF-8';


///
/// Find first char which is not a space
///
/// @param {String} $string - String to search
/// @param {String} $direction ['left'] - Either 'left' or 'right' to indicate search direction
///
/// @return {Number} - Index of first non-space character
///
@function first-index($string, $direction: 'left') {
  @for $i from 1 through str-length($string) {
    $index: if($direction == 'left', $i, -$i);

    @if str-slice($string, $index, $index) != ' ' {
      @return $index;
    }
  }

  @return 0;
}


@function trim($string) {
  @return str-slice(
    $string,
    first-index($string, 'left'),
    first-index($string, 'right')
  );
}


body {
  background: trim("s");
}

@danez
Copy link
Author

danez commented Jul 29, 2016

sass/libsass#2132

@KittyGiraudel
Copy link
Collaborator

God, again… Will this function ever work correctly? :D

@eduardoboucas
Copy link
Owner

Is there a temporary fix we can release?

@KittyGiraudel
Copy link
Collaborator

Maybe there is one of the Libsass repo.

@IanMitchell
Copy link
Contributor

Sorry guys, didn't think to check with libsass 😞

@KittyGiraudel
Copy link
Collaborator

We should revert @eduardoboucas.

@eduardoboucas
Copy link
Owner

Okay, I'll do that.

@eduardoboucas
Copy link
Owner

Reverted and published 1.4.8.

@IanMitchell
Copy link
Contributor

Working on a libsass fix here: sass/libsass#2133

@danez
Copy link
Author

danez commented Jul 29, 2016

thanks a lot

@weaintplastic
Copy link

thank you guys!

@eduardoboucas
Copy link
Owner

Am I right in thinking that it was fixed here?

@KittyGiraudel
Copy link
Collaborator

Yep.

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

No branches or pull requests

5 participants