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

@each with maps #492

Closed
toastal opened this issue Oct 2, 2014 · 19 comments · Fixed by #568 or #571
Closed

@each with maps #492

toastal opened this issue Oct 2, 2014 · 19 comments · Fixed by #568 or #571

Comments

@toastal
Copy link

toastal commented Oct 2, 2014

It looks like maps functionality was finally pulled in :). I was excited to finally use it, but

: error: expected 'in' keyword in @each directive

I'm assuming that @each hasn't been updated to support $key, $value like in the 3.3 release blog example.

@chriseppstein
Copy link
Contributor

Right. In Sass 3.3 we added generic support for destructuring assignment in @each and plan to add it to the language at a broader level soon. So you can do

@each $key, $value in $map because each iteration of $map is a pair.

In general Sass supports @each $arg1, $arg2, ..., $argN in $list-of-lists where each element in list of lists would bind the arguments in order and assign the rest as null. Since any single element is a list of 1, if one of the list elements is just a single value, then only $arg1 would be set.

@xzyfer
Copy link
Contributor

xzyfer commented Oct 6, 2014

In general Sass supports @each $arg1, $arg2, ..., $argN in $list-of-lists where each element in list of lists would bind the arguments in order and assign the rest as null.

Is it worth adding a separate bug for this case? These two use cases would likely be two separate patches to libsass.

Also the current test sass/sass-spec#54 only cover the maps use case.

@mirisuzanne
Copy link

@xzyfer I added those tests to the spec. Well, I made a pull request — if that looks good I (or someone else) can merge it in.

@xzyfer
Copy link
Contributor

xzyfer commented Oct 6, 2014

Awesome @ericam thanks!

@HamptonMakes
Copy link
Member

Done.

@laurentvd
Copy link

So, is this feature now implemented in the master branch?

@toastal
Copy link
Author

toastal commented Oct 9, 2014

It appears that the only things that were merged in were the test cases for the bug. The milestone is set for 3.1 and 3.0 is only at release candidate at this time. I'd assume all 3.0 bugs are to be wrapped up before anything else is really looked at. The reality seems to be SOON™ though.

@mirisuzanne
Copy link

If maps are supposed to be a key feature of 3.0, I'd love to see this make the cut. Looping isn't an edge case or a bonus feature — it's central to most map uses I've seen (and certainly any library use). There's a lot of excitement around 3.0, and I'd like to jump on that bandwagon, but none of my libraries can do much without looping through maps. Pretty please. :)

@xzyfer
Copy link
Contributor

xzyfer commented Oct 15, 2014

I agrees @each with multiple assigns is important. FWIW it can be easily worked around though. It's not ideal, but it's doable.

$map: (foo: bar, bar: baz);

// before
@each $k, $v in $map {
  // ...
}

// after
@each $k in map-keys($map) {
  $v: map-get($map, $k);
  // ...
}

FWIW IMHO the more critical issue with maps is their performance (#536). I've dropped my work on feature queries (#261) and reference combinator (#452) to focus on map performance.

@andreasisaak
Copy link

@xzyfer Without sass-list-maps, i have an error with your solution:

Warning: xxx/scss/components/nav/top-links:66: argument $mapofmap-keys($map) must be a map

@xzyfer
Copy link
Contributor

xzyfer commented Oct 17, 2014

Can you please post the code you used to get that error @andreasisaak ?

@andreasisaak
Copy link

@xzyfer I tested exactly your code :)

@xzyfer
Copy link
Contributor

xzyfer commented Oct 20, 2014

@andreasisaak hmm I'm not sure why you'd get that error, can you please create a sassmeister gist for me to debug?

Here is my PoC https://gist.github.com/xzyfer/adff8699236bd470afbd

@joelworsham
Copy link

Any array map I try to create gives me an error like:

error: error reading values after...

Like:
$map: (foo: bar, bar: baz);
gives me:
error: error reading values after foo

@xzyfer
Copy link
Contributor

xzyfer commented Oct 21, 2014

Sounds like you might be running libsass 2.0. Map support was added in 3.0.

@joelworsham
Copy link

Hmmm.

I'm using grunt, and grunt-sass, and I did an "npm update" in my project. How can I check my libsass version?

@xzyfer
Copy link
Contributor

xzyfer commented Oct 21, 2014

grunt-sass added libsass 3.0 in version 0.16.0. Check the version in node_modules/grunt-sass/package.json and node_modules/node-sass/package.json

@andreasisaak
Copy link

@xzyfer It was my fault! The code works perfectly

@xzyfer
Copy link
Contributor

xzyfer commented Oct 27, 2014

No worries @andreasisaak. Glad you got it sorted. I'm currently working on bring this to libsass.

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