Skip to content

4.2 breaking change: custom $grid-breakpoints that are smaller than default ones give errors #27927

Closed
Closed
4.2 breaking change: custom $grid-breakpoints that are smaller than default ones give errors#27927
Labels
cssSass sources or compiled CSShas-prAn open pull request addresses itv4v4, the frozen docs on gh-pages

Description

@phazei
phazei
opened on Dec 27, 2018

I'm unable to compile sass with the new $grid-breakpoint changes.
This worked just fine with 4.1. It now doesn't seem possible with 4.2

In my custom variable file that's included before everything BS4 I have the following:

$grid-breakpoints: (
        xxs: 0,
        xs: 480px,
        sm: 576px,
        md: 768px,
        lg: 992px,
        xl: 1200px
);

As you can see, an additional custom breakpoint exists at 480px.

Previously 4.1 _variable.sass contained:

$grid-breakpoints: (
  xs: 0,
  sm: 576px,
  md: 768px,
  lg: 992px,
  xl: 1200px
) !default;

@include _assert-ascending($grid-breakpoints, "$grid-breakpoints");
@include _assert-starts-at-zero($grid-breakpoints);

And my code overrode all the breakpoints and worked fine.

Now in 4.2 _variable.sass:

$grid-breakpoints: () !default;
// stylelint-disable-next-line scss/dollar-variable-default
$grid-breakpoints: map-merge(
                (
                        xs: 0,
                        sm: 576px,
                        md: 768px,
                        lg: 992px,
                        xl: 1200px
                ),
                $grid-breakpoints
);

@include _assert-ascending($grid-breakpoints, "$grid-breakpoints");
@include _assert-starts-at-zero($grid-breakpoints);

And it gives the following errors:

WARNING: Invalid value for $grid-breakpoints: This map must be in ascending order, but key 'xxs' has value 0 which isn't greater than 1200px, the value of the previous key 'xl' !

WARNING: First breakpoint in `$grid-breakpoints` must start at 0, but starts at 480px.

WARNING: Invalid value for $container-max-widths: This map must be in ascending order, but key 'xs' has value 500px which isn't greater than 1140px, the value of the previous key 'xl' !

Activity

  1. phazei
    changed the title [-]4.2 broke building custom $grid-breakpoints that are smaller than default ones[/-] [+]4.2 breaking change: custom $grid-breakpoints that are smaller than default ones give errors[/+] on Dec 27, 2018

  2. phazei commented on Dec 27, 2018

    @phazei
    phazei
    on Dec 27, 2018
    Author

    The def of map-merge() specifies that the order of $map1 will have precedence while $map2 will override values.

    /**
 * Merges two maps together into a new map. Keys in `$map2` will take
 * precedence over keys in `$map1`.
 * 
 * This is the best way to add new values to a map.
 * 
 * All keys in the returned map that also appear in `$map1` will have the
 * same order as in `$map1`. New keys from `$map2` will be placed at the end
 * of the map.
 * 
 * Like all map functions, `map-merge()` returns a new map rather than
 * modifying its arguments in place.
 * 
 * @example
 * map-merge(("foo": 1), ("bar": 2)) => ("foo": 1, "bar": 2)
 * map-merge(("foo": 1, "bar": 2), ("bar": 3)) => ("foo": 1, "bar": 3)
 * @overload map_merge($map1, $map2)
 * @param $map1 [Sass::Script::Value::Map]
 * @param $map2 [Sass::Script::Value::Map]
 * @return [Sass::Script::Value::Map]
 * @raise [ArgumentError] if either parameter is not a map
 */
@function map_merge($map1, $map2) { /* stub */ }

    Since, unlike the other maps that are merged, the order is important, $map2 should have precedence.

    To achieve this I think the double application of map-merge() should take care of it:

    $grid-breakpoints: () !default;
// stylelint-disable-next-line scss/dollar-variable-default
$grid-breakpoints-ordered: map-merge(
                $grid-breakpoints,
                (
                        xs: 0,
                        sm: 576px,
                        md: 768px,
                        lg: 992px,
                        xl: 1200px
                )
);
$grid-breakpoints: map-merge($grid-breakpoints-ordered, $grid-breakpoints)

    Unless there's some other sass function that could handle this, I'm not sure of any better way.

  3. phazei commented on Dec 27, 2018

    @phazei
    phazei
    on Dec 27, 2018
    Author

    Additionally the docs give a specific use example:

    If you wanted just four grid tiers, you’d update the $grid-breakpoints ...

    which is no longer true since by just defining fewer breakpoints, the default ones will be added back in on merge.

  4. nicolomonili commented on Dec 28, 2018

    @nicolomonili
    nicolomonili
    on Dec 28, 2018

    I have the same problem, i'm trying to compile with https://github.com/leafo/scssphp
    Before the version 4.2 everything was ok, now i have the same mistakes as you.

  5. nicolomonili commented on Dec 28, 2018

    @nicolomonili
    nicolomonili
    on Dec 28, 2018

    The def of map-merge() specifies that the order of $map1 will have precedence while $map2 will override values.

    /**
 * Merges two maps together into a new map. Keys in `$map2` will take
 * precedence over keys in `$map1`.
 * 
 * This is the best way to add new values to a map.
 * 
 * All keys in the returned map that also appear in `$map1` will have the
 * same order as in `$map1`. New keys from `$map2` will be placed at the end
 * of the map.
 * 
 * Like all map functions, `map-merge()` returns a new map rather than
 * modifying its arguments in place.
 * 
 * @example
 * map-merge(("foo": 1), ("bar": 2)) => ("foo": 1, "bar": 2)
 * map-merge(("foo": 1, "bar": 2), ("bar": 3)) => ("foo": 1, "bar": 3)
 * @overload map_merge($map1, $map2)
 * @param $map1 [Sass::Script::Value::Map]
 * @param $map2 [Sass::Script::Value::Map]
 * @return [Sass::Script::Value::Map]
 * @raise [ArgumentError] if either parameter is not a map
 */
@function map_merge($map1, $map2) { /* stub */ }

    Since, unlike the other maps that are merged, the order is important, $map2 should have precedence.

    To achieve this I think the double application of map-merge() should take care of it:

    $grid-breakpoints: () !default;
// stylelint-disable-next-line scss/dollar-variable-default
$grid-breakpoints-ordered: map-merge(
                $grid-breakpoints,
                (
                        xs: 0,
                        sm: 576px,
                        md: 768px,
                        lg: 992px,
                        xl: 1200px
                )
);
$grid-breakpoints: map-merge($grid-breakpoints-ordered, $grid-breakpoints)

    Unless there's some other sass function that could handle this, I'm not sure of any better way.

    Have you solved in this way for now?

  6. Johann-S
    added
    cssSass sources or compiled CSS
    v4v4, the frozen docs on gh-pages
    on Dec 28, 2018
  7. luisfpg
    added a commit that references this issue on Dec 28, 2018

    Upgraded NPM dependencies

    f135ae5

  8. phazei commented on Dec 29, 2018

    @phazei
    phazei
    on Dec 29, 2018
    Author

    Have you solved in this way for now?

    Nah, I solved it by changing the dep from ^4.1.0 to ~4.1.0 so it works in the build pipeline.
    Easier than forking it for now.
    Although, that code does fix it locally. It also needs to be done for the $container-max-widths as well:

    $container-max-widths: () !default;
// stylelint-disable-next-line scss/dollar-variable-default
$container-max-widths-order: map-merge(
  $container-max-widths,
  (
    sm: 540px,
    md: 720px,
    lg: 960px,
    xl: 1140px
  )

);
$container-max-widths: map-merge($container-max-widths-order, $container-max-widths);

  9. XhmikosR commented on Dec 29, 2018

    @XhmikosR
    XhmikosR
    on Dec 29, 2018
    Member

    /CC @MartijnCuppens

  10. mdo commented on Dec 30, 2018

    @mdo
    mdo
    on Dec 30, 2018
    Member

    I think we need to ditch these built-in map-merges for regular Sass maps. It now feels over-engineered, and perhaps poorly documented for how folks can customize things.

    With the usage of map-merge everywhere, we have the ability to easily add key-value pairs, but not to remove them. There's no option in Sass to do this, but you can replace a value or add new ones (with map-merge). This is an issue because if you want fewer key-value pairs in a Sass map, you'll see errors like those of #27927 (comment).

    So, what if we remove all map-merge in our variables?

    Let's consider an example where we look at our $grid-breakpoints.

    $grid-breakpoints: (
  xs: 0,
  sm: 576px,
  md: 768px,
  lg: 992px,
  xl: 1200px
) !default;

    To replace all of the values, we redefine the keys' values:

    $grid-breakpoints: (
  xs: 0,
  sm: 400px,
  md: 800px,
  lg: 1200px,
  xl: 1600px
);

    To replace individual key-value pairs, we use map-merge:

    $grid-breakpoints: (
  xs: 0,
  sm: 576px,
  md: 768px,
  lg: 992px,
  xl: 1200px
) !default;

$grid-breakpoints: map-merge($grid-breakpoints, (xl: 1600px));

    To add additional values, we also use map-merge:

    $grid-breakpoints: map-merge(
  $grid-breakpoints,
  (
    xxl: 1600px,
    xxxl: 1800px
  )
);

    To do a bit of everything, we can use replace the default values and use multiple map-merges:

    $grid-breakpoints: (
  xs: 300px,
  sm: 500px,
  md: 800px,
  lg: 1100px,
  xl: 1400px
);
$grid-breakpoints: map-merge(
  (
    xxs: 0
  ),
  $grid-breakpoints
);
$grid-breakpoints: map-merge(
  $grid-breakpoints,
  (
    xxl: 1600px,
    xxxl: 1800px
  )
);

    You can see all of this in this Sassmeister gist.

  11. Bek81 commented on Jan 2, 2019

    @Bek81
    Bek81
    on Jan 2, 2019

    Hi @mdo
    following your instructions, we are trying this way:

    $grid-breakpoints: (
    xs: 360px,
    sm: 576px,
    md: 768px,
    lg: 992px,
    xl: 1200px
);

$grid-breakpoints: map-merge((xxs: 0), $grid-breakpoints);
$grid-breakpoints: map-merge($grid-breakpoints, (xxl: 1440px, xxxl: 1680px, xxxxl: 1920px));

$container-max-widths: (
    xs: 330px,
    sm: 540px,
    md: 720px,
    lg: 960px,
    xl: 1140px
);
$container-max-widths: map-merge((xxs: 0), $container-max-widths);
$container-max-widths: map-merge($container-max-widths, (xxl: 1380px, xxxl: 1620px, xxxxl: 1860px));

@import '../../scripts/vendor/bootstrap-4.2.1/scss/bootstrap.scss';

    but these are the results:

    WARNING: Invalid value for $grid-breakpoints: This map must be in ascending order, but key 'xxs' has value 0px which isn't greater than 1200px, the value of the previous key 'xl' !

    WARNING: First breakpoint in $grid-breakpoints must start at 0, but starts at 360px.

    WARNING: Invalid value for $container-max-widths: This map must be in ascending order, but key 'xxs' has value 0px which isn't greater than 1140px, the value of the previous key 'xl' !

    Error: Invalid null operation: "null sub 0.02".

    Can you help us, please? Thanks a lot.

    Benedetto

  12. phazei commented on Jan 2, 2019

    @phazei
    phazei
    on Jan 2, 2019
    Author

    @Bek81
    Those weren't instructions, he was giving an example of how sass maps could work and how they could potentially work in the core _variables.scss file. Simply revert to 4.1 until it's resolved as there's no fix without modifying _variables.scss.

    There's also no use in using the merge-map in your own definitions of the variables. The last example @bdo gave would have to have the extra xxs and xxl variables in like $breakpoints-before / $breakpoints-after which is way overly complicated.

    The example I provided makes overriding the breakpoints nearly the same as 4.1, but still has the issue of not being able to remove breakpoints. Reverting to the way it was done in 4.1 without merge-map seems like it might be the best solution. The update provides no benefit other than being able to override a single break point or add one larger, most other cases you'd need to override each breakpoint manually anyway.

  13. JulienCabanes commented on Jan 4, 2019

    @JulienCabanes
    JulienCabanes
    on Jan 4, 2019

    It's also possible to sort maps after merge, I tried with this gist and it worked without changing anything else : https://gist.github.com/Jakobud/a0ac11e80a1de453cd86f0d3fc0a1410#gistcomment-2327765

  14. ianks commented on Jan 9, 2019

    @ianks
    ianks
    on Jan 9, 2019

    This is preventing us from upgrading to 4.2

  15. carlosmauri commented on Jan 10, 2019

    @carlosmauri
    carlosmauri
    on Jan 10, 2019

    Hi there,
    what's the status of this "warning" issue?

    I would suggest to just revert the new $grid-breakpoints declaration to what it was before, on the 4.1 version:

    $grid-breakpoints: (
  xs: 0,
  sm: 576px,
  md: 768px,
  lg: 992px,
  xl: 1200px
) !default;

    the new approach uses:

    $grid-breakpoints: map-merge(
  (
    xs: 0,
    sm: 576px,
    md: 768px,
    lg: 992px,
    xl: 1200px
  ),
  $grid-breakpoints
);

    so when you try to update the variable using your custom.scss like this:

    $grid-breakpoints: (
    xs: 0,
    ss: 380px,
    sm: 576px,
    md: 768px,
    lg: 992px,
    xl: 1200px,
    xx: 1440px
);

    the _assert_ascending function will throw the following warning:

    Invalid value for $grid-breakpoints: This map must be in ascending order, but key 'ss' has value 320px which isn't greater than 1200px, the value of the previous key 'xl' !
    scss/_functions.scss 18:7    _assert-ascending()

    Adding @debug $key; @debug $num; you can easily see what's going on.

    scss/_functions.scss:11 Debug: xs
scss/_functions.scss:12 Debug: 0
scss/_functions.scss:11 Debug: sm
scss/_functions.scss:12 Debug: 576px
scss/_functions.scss:11 Debug: md
scss/_functions.scss:12 Debug: 768px
scss/_functions.scss:11 Debug: lg
scss/_functions.scss:12 Debug: 992px
scss/_functions.scss:11 Debug: xl
scss/_functions.scss:12 Debug: 1200px
scss/_functions.scss:11 Debug: ss
scss/_functions.scss:12 Debug: 320px

    The ss value I've added in my custom.scss goes to end of the map so this function.
    when compares $prev-num >= $num (1200px >= 320px) -> warning message

    Note that this is just a warning so the CSS gets compiled.
    Thanks!

    If you really want to avoid this warning message you can (temporary and hacky fix) comment this line:
    @include _assert-ascending($grid-breakpoints, "$grid-breakpoints");
    _variables.scss lines 201 and 221

  16. MartijnCuppens
    pinned this issue on Jan 10, 2019
  17. MartijnCuppens
    added
    has-prAn open pull request addresses it
    on Jan 13, 2019
  18. mdo
    closed this as completedon Jan 14, 2019
  19. MartijnCuppens
    unpinned this issue on Jan 14, 2019

  20. unrevised6419 commented on Jan 16, 2019

    @unrevised6419
    unrevised6419
    on Jan 16, 2019
    Contributor

    #25392

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    cssSass sources or compiled CSShas-prAn open pull request addresses itv4v4, the frozen docs on gh-pages

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions