oddbird / accoutrement

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

bug: `tokens.get()` memorization #112

Closed enten closed 2 years ago

enten commented 2 years ago

Description

It seems that tokens.get() memorization isn't working:

  1. Firstly, the !global flag is missing when update the $_memo map ;
  2. Secondly, even with !global flag when update the $_memo map, the memorization won't work as expected when on-the-fly adjustments are given with $do parameter, because the resolved value from $_memo is returned before adjustments are applied.

So, with !global flag added when update the $_memo map, two tokens.get() calls with $do adjustments don't return the same value:

Test below fails with !global flag added when update the $_memo map:

$haystack: (
  'slow': 3s,
  'delay': 0.5s,
  'key': '#slow',
);

@include it('Memorize resolved value') {
  @include assert-equal(tokens.get($haystack, 'key', ('minus': '#delay')), 2.5s); // OK
  @include assert-equal(tokens.get($haystack, 'key', ('minus': '#delay')), 2.5s); // KO
}
AssertionError [ERR_ASSERTION]: Memorize resolved value [type: assert-equal]

- Expected
+ Received

- [number] 2.5s
+ [number] 3s

Source from v4.0.1: v4.0.1/sass/tokens/_api.scss

@function get($map, $key, $do: null) {
  // check the memo
  $resolved: map.get($_memo, $map, $key);
  @if $resolved { /// <-- #2: adjustments aren't applied when value is resolved from `$_memo`
    @return $resolved;
  }

  // lookup the key in the map
  $lookup: parse.lookup-alias($map, $key);

  @if $lookup {
    $resolved: parse.compile-token($map, map.get($lookup, 'token'), $key);
  }

  // update the memo
  $_memo: map.set($_memo, $map, $key, $resolved); /// <-- #1: `!global` flag is missing

  // handle any final adjustments
  @if $do { /// <-- #2: adjustments aren't applied when value is resolved from `$_memo`
    @return parse.compile-token($map, map.merge(('%value': $resolved), $do), $key);
  }

  // return the result
  @return $resolved;
}

Proposal to fix tokens.get() memorization: 68fc10e4/sass/tokens/_api.scss

@function get($map, $key, $do: null) {
  // check the memo
  $resolved: map.get($_memo, $map, $key);

  // when resolved value isn't in the memo
  @if not map.has-key($_memo, $map, $key) {
    // lookup the key in the map
    $lookup: parse.lookup-alias($map, $key);

    @if $lookup {
      $resolved: parse.compile-token($map, map.get($lookup, 'token'), $key);
    }

    // update the memo
    $_memo: map.set($_memo, $map, $key, $resolved) !global;
  }

  // handle any final adjustments
  @if $do {
    @return parse.compile-token($map, map.merge(('%value': $resolved), $do), $key);
  }

  // return the result
  @return $resolved;
}
jgerigmeyer commented 2 years ago

Fixed by #113