Bug in parsing nested maps

[joelschou]

Related to my previous issue (#12), but specifically to this line in the documentation:

Nested maps will compare #tags internally, before looking at the parent keys

There's a bug in the parsing, such that rearranging the keys in the nested map can give different results, even if those keys have no relation to each other whatsoever. You can actually see this in action using the "Prefers nested keys in nested maps" test:

This passes:

  $haystack: (
    'one': 'original value',
    'two': '#one',
    'three': '#two',
    'four': 'and a new value',
    'map': (
      'one': 'inner value',
      'second': '#one',
      'third': '#two',
    ),
  );
...
    $expect: (
      'one': 'inner value',
      'second': 'inner value',
      'third': 'original value',
    );

Move third up above second and it fails:

  $haystack: (
    'one': 'original value',
    'two': '#one',
    'three': '#two',
    'four': 'and a new value',
    'map': (
      'one': 'inner value',
      'third': '#two',
      'second': '#one',
    ),
  );
...
    $expect: (
      'one': 'inner value',
      'third': 'original value',
      'second': 'inner value',
    );
...
+ expected - actual
-[map] ("one": "inner value", "third": "inner value", "second": "original value")
+[map] ("one": "inner value", "third": "original value", "second": "inner value")

OR, insert a seemingly innocuous key: value pair into the nested list and it fails:

  $haystack: (
    'one': 'original value',
    'two': '#one',
    'three': '#two',
    'four': 'and a new value',
    'map': (
      'one': 'inner value',
      'foo': 'bar',
      'second': '#one',
      'third': '#three',
    ),
  );
...
    $expect: (
      'one': 'inner value',
      'foo': 'bar',
      'second': 'inner value',
      'third': 'original value',
    );
...
+ expected - actual
-[map] ("one": "inner value", "foo": "bar", "second": "original value", "third": "original value")
+[map] ("one": "inner value", "foo": "bar", "second": "inner value", "third": "original value")

I can't see any [non-buggy] reason why second or third would change their values depending on which key comes before it.

[mirisuzanne]

Oh, interesting. Are you willing to dig into this more – start by adding a test, and see if you can find a fix? I'm not likely to have much time in the next month, but would be happy to review/merge a PR, and publish a new release.

[joelschou]

Happy to keep digging 😄

And it turns out there are two distinct bugs; unfortunately the second arises out of a potential fix for the first.

1. This bug: In the @each which starts at line 141, it appears to have to do with the way $needle gets overridden, such that the next key being tested only has the potential overrides from the previous key in $layers. You can see this most clearly in my second example: 'second': '#one' fails because only ('foo': 'bar') is being merged on top of $haystack in line 143. I think I have a fix for this: instead of $needle: ($key: $value); on 148, we do $needle: map-merge($needle, ($key: $value));. Unfortunately...
2. New bug: ...because we're now maintaining 'one': 'inner value' in the merge on 143, 'third': '#three' maps to 'two': '#one' which maps to, yup, 'one': 'inner value' even though it should be looking in the outer context after being pointed to 'two'. I'll keep digging!

[joelschou]

PR: #15

So I think I fixed this for real. The code gets a little messy, but I wanted to make sure we weren't letting map manipulation from previous values leak into later ones.

Aside: while all the Sass list functions work on maps, it's too bad they turn maps into lists. I tried to use set-nth(...) on $needle itself, but I got a list in return and couldn't turn it back into a map without doing an @each similar to the one I did on $keys. ¯\_(ツ)_/¯