oddbird / accoutrement

Combined Sass Accoutrement tools, with option for individual module imports
MIT License
37 stars 6 forks source link

tokens.get() returns null for some present values (4.0.0-beta2) #99

Closed johncrim closed 2 years ago

johncrim commented 2 years ago

I'm in the process of updating our codebase to use the new accoutrement (version 4.0.0-beta2), and I found an odd error that occurs in some situations. In short, tokens.get() returns null sometimes, when there is a value in the map. I haven't been able to figure out the cause yet, but I've narrowed it down to 2 very similar cases, one of which works as expected, and one of which doesn't.

Here's a unit test showing the problematic behavior:


    @include it('Handles CSS values in maps') {
      $gradients: (
        'accent': linear-gradient(hsl(31deg 94% 52%) 10%, hsl(288deg 95% 62%) 55%, hsl(274deg 95% 57%) 80%),
        'accent2': linear-gradient(orange 10%, magenta 55%, violet 80%)
      );

      @include assert() {
        @include output() {
          background-image: tokens.get($gradients, 'accent');
        }
        @include expect() {
          background-image: linear-gradient(hsl(31deg 94% 52%) 10%, hsl(288deg 95% 62%) 55%, hsl(274deg 95% 57%) 80%);
        }
      }
    }

I'm running this test in test/tokens/_api.scss in the accoutrement tests, on the modules branch - so using all the current build dependencies.

In the above case, the tokens.get() call returns null. If I change the tokens.get() key from 'accent' to 'accent2' then tokens.get() returns the expected value.

I'll dig into this further, but wanted to report it.

johncrim commented 2 years ago

The bug is in _parse.scss : _resolve-value() function. If the map value contains a color string of format #rrggbb it tries to resolve it as an alias, and fails, so returns null for the whole value. I have a few tests proving this, and I'm working on a fix.

This is a regression - the issue wasn't present in v3. My opinion so far is that the previous missing key behavior (of just returning the key) is correct when replacing aliases in a token value. That way something like linear-gradient(#eee, #222) (which is an unquoted string) would result in linear-gradient(#eee, #222). Also, if eee were defined in the map, #eee would be replaced with that value.

I considered treating unquoted strings differently - because unquoted strings must be valid CSS - sass enforces this. So therefore we could never have alias references in unquoted strings. But I figure such behavior would be less obvious to a developer using accoutrement.

The slightly surprising (to me) reason the test case above doesn't work is that the hsl(..) values within the map value are converted to #rrggbb values in the map (by sass), and then null is returned from parse.compile-token() because #rrggbb isn't resolved as an alias.