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

fix(references): value object references now work #623

Merged
merged 1 commit into from
May 12, 2021
Merged

Conversation

dbanksdesign
Copy link
Member

Issue #, if available: #615

Description of changes: Token values that are objects as opposed to simple strings were not working with outputReferences and throwing an error. These changes fix some behavior and make sure certain scenarios don't fail. This makes explicit what outputReferences can and can't do:

  • The value must be a string at time of formatting. This should be the case anyways, if not you will get [object Object] in output files. With outputReferences this was failing because an object does not have the .replace function.
  • The value must have the referenced values in it for outputReferences to replace the resolved values with the reference's name. For example if you define a color using an HSL object and transform it to a hex value, we cannot undo that transform to understand which parts of the value to use the reference's name.

Here are the changes:

  • In createPropertyFormatter, rather than looking at the original value and replacing references with the referenced token's name, we are now looking at the transformed value and replacing any referenced values with the reference's name. This allows us to still use outputReferences with a value object.
    • If the transform that converts the value object to a string mutates the reference so that it is not in the transformed value, it will not work with outputReferences. It will build fine, but just won't output the reference and instead output the transformed and resolved value. An example of this is converting an HSL object to a Hex string.
    • You can reference a non-token as seen in the transitive-transforms example. References to non-token's will not work with outputReferences because the references will not have a name attribute. Just like the previous point, it will build fine, but just won't output the reference and instead output the transformed and resolved value.
  • Built-in formats expect a token's value to be a string at the time of formatting. This means if the token's value is an object it needs to be transformed to a string
  • Updated getReferences to mirror usesReference with respect to object values. usesReference checks the object values, whereas getReferences did not.

Added some unit tests to getReferences to verify behavior.
Added integration tests to verify different behaviors:

  • Using an object value with references, transformed to a string where reference is present (css border short-hand). With and without outputReferences
  • Using an object value with references to non-tokens
  • Using an object value with references, transformed to a string where reference is not present (hsl -> hex)

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

Copy link
Collaborator

@chazzmoney chazzmoney left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work. For being some of the most complex code in all of SD, this is almost readable!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants