fix(outputReferences): sorting of tokens in dtcg format

timges commented 1 month ago

Fixes https://github.com/amzn/style-dictionary/issues/1305

Issue Summary: The current implementation of the sortByReference function doesn't properly handle the DTCG token format, causing references in the output to be declared before their target. This occurs because the function assumes that token values can be accessed using the value attribute, which is not the case for the DTCG format, where the attribute is $value.

For example, this issue results in incorrect output like:

// Do not edit directly, this file was auto-generated.

$colors-primary: $colors-red;
$colors-red: #ff0000;

caused by:

// -- sortByReference.js
function sortByReference(dictionary)
  function sorter(a, b) {
    // [...]
    // If token a uses a reference and token b doesn't, b might come before a
    // read on..
    if (a.original && dictionary.usesReference(a.original.value)) {
      // Both a and b have references, we need to see if the reference each other
      if (b.original && dictionary.usesReference(b.original.value)) {
        const aRefs = dictionary.getReferences(a.original.value);
        const bRefs = dictionary.getReferences(b.original.value);
    // [...]
  }
}

Description of changes: This PR addresses the issue by modifying the sortByReference function to account for the usesDtcg options. It ensures that the correct token value is accessed based on the format (default or DTCG), preventing incorrect reference sorting.

// -- sortByReference.js
// [...]
  const valueKey = usesDtcg ? '$value' : 'value';
// [...]
  const aUsesRefs = a.original && usesReferences(a.original[valueKey]);
  const bUsesRefs = b.original && usesReferences(b.original[valueKey]);

Additional Changes:

Refactored the module to improve readability and follow clean code conventions.
Inline comments where already used heavily, so I stuck to that practice and commented some lines.

Checklist

[x] fix issue
[x] add unit tests
[x] add integration tests
[x] refactor code

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

aws-amplify-us-west-1[bot] commented 1 month ago

This pull request is automatically being deployed by Amplify Hosting (learn more).

Access this pull request here: https://pr-1349.d16eby4ekpss5y.amplifyapp.com

timges commented 1 month ago

Done 👌 Thanks for the review, I appreciate that! 💯

amzn / style-dictionary

fix(outputReferences): sorting of tokens in dtcg format #1349