Rename 'include' to 'references' and filter out by default

[jorenbroekema]

"source" versus "include" property has been the source of a lot of confusion for users, including myself as well.

Include acts as a base layer of tokens that is generally only used as references for other tokens, and it does 2 things:

• When include tokens collide with source tokens, the source tokens always takes precedence, and I believe no token collision warnings are thrown in this case
• An isSource property with value true is added for any tokens that originate from files in the "source" globs array

The main problems here is that "include" is poorly named and that the majority use case for such tokens is to not include them in the output, which means it requires from users to always add a filter that filters tokens by isSource. It is somewhat tedious to repeat this filter for every platform and each file when this is the majority use case for design systems, as usually the guidelines of a design system do not permit usage of primitives tokens, only semantics and components tokens.

Suggestion (both are breaking changes):

• Rename include to references, so it's clear that you should use this for token sets that are merely used as reference/alias for other tokens.
• Filter out references tokens in formats by default, unless global config property outputReferenceTokens (new property suggestion, we don't have this yet) is set to true, OR if outputReferences on the format level options is set to true, because in this case we cannot safely filter out refs if we plan on outputting for example CSS custom properties for tokens referencing other tokens.

Impact:

• Doing the rename in your config + setting outputReferencesTokens to true means you get the exact same behavior as before
• Doing the rename only means you can delete all of your filters that specifically filter out tokens that do not have isSource set to true, since tokens from references array will be filtered out by default now
• You can still use the isSource property, I don't see a strict need to remove that, although it will be less useful IMO

[mikekamminga]

I think we should also align this with the DTCG resolver proposal by @SorsOps to align terminology. I agree it can be more clear. More thoughts following soon!

[dbanksdesign]

A bit of history on include and source. The original product that I built Style Dictionary for in Amazon had a set up where we had a general style dictionary package that was the base. Then there was a mobile specific style dictionary that imported the base style dictionary package in the include and added its own tokens and overrode some tokens. It needed to output the entire set so that the mobile platforms didn't need to depend on 2 packages and have both packages set up with the same transforms/formats to be used on the mobile platforms. Also the original impetus was that because it was expected that include tokens that were overridden by source tokens should not warn the user because the intention was that source should override include. But if you have 2 tokens in source at the same path we do want to warn you because that was probably unintended.

The one difficulty with calling the 'base' set of tokens 'references' is they might not all be references.

All that being said, I am 100% open to changing the name of anything if it is more understandable and v4 is the time to do it. I'm not married to include and source, but whatever we go with we should be clear what the 2 things are doing and what use-cases they support.

Just my initial thoughts, I'll keep noodling on it

[luupanu]

One option would be to keep include but rename it to defaults or base which I think is more in line what that property is about, and keep references as something that truly are only references and are never built. This has the benefit of supporting both a base / default / whitelabel layer and an alias / reference layer without any additional filtering.

I guess the problem with this is that the name references would be confusing with outputReferences in format level. Maybe in this could consider something like aliases as a name instead of references and outputReferences could even by default resolve these to the next base / token layer (aka source / include reference or a straight up value).

There's one problem to solve with this approach that I see: what overrides what for three properties, I guess the order is defaults < references/aliases < source, but a lot of different overrides might be confusing for users.

Anyway both approaches also have one other problem to solve: if a glob pattern has a file in both references/aliases and e.g. source, what is the expected outcome? As I see it they seem mutually exclusive, should the build check for this and just produce an error?

[jorenbroekema]

I think there is a fundamental issue with creating properties ('include') that do the same thing as other properties ('source') where the only difference is the layering. What if I need a third layer, then I need to come up with another property name, etc.

There's already a way to do extending/layering by using sdInstance.extend() which I improved significantly in v4 by doing a deepmerge action on the configs when extending, meaning if both the original instance and the extension have a source array in config, the values will be accumulated and deduplicated. This wouldn't get rid of the token collision warnings however, so perhaps we can think of an API for "conscious token collisions" by expanding on the API of the "source" property and support any level of layering.

One suggested API that we could use is the following extension on "source":

{
  "source": [
    "foo.json",
    ["base-1.json", "base-2.json"], // layer 1, collisions with layer 2 are not warned about
    ["extend-1.json", "extend-2.json"], // layer 2, collisions with layer 1 are not warned about
    "bar.json"
  ]
}

Note: when doing sd.extend(), the source property would be deepmerged where Arrays merging behavior is that the items from the extension are added at the end of the array of the base, and deduped in case there are duplicate values. Note 2: this is already the case in v3, the order matters, when a collision occurs the token from the file from the glob that's ordered later in "source" takes precedence, more docs/tests for this behavior would be good

• token collisions with foo.json throw warnings
• token collisions with bar.json throw warnings
• base-*.json is considered a layer, and collisions across layers are not warned about
• extend-*.json is considered another layer, and collisions across layers are not warned about
• layered sets, when colliding with sets that are not a layer (foo.json and bar.json), still throw warnings

This extends the source API without a breaking change, the old API still works as is, and it allows any number of layers.

That said, you can also just ignore the collision warnings, they're a LOT less verbose by default in v4, and you can turn off warnings altogether as well. In addition, I changed the warning message a bit so it's more clear that you can ignore the warning if the collision was intended. So I am wondering how important the use case still is that @dbanksdesign described and whether this API redesign is worth it. My suggested API shouldn't be too much work I think, so I'm a bit on the edge about this.

Finally, the proposal of "references" tackles the other use case which is creating a split between tokens you intend on outputting and tokens merely used as a reference layer, but I wanted to clarify that I think that the global property outputReferenceTokens becomes redundant when the "source" prop supports layering, because then you can just use the layering if preventing token collision messages is primarily what you're after. outputReferences on the format level is still relevant, and custom formats in general will need to use the token's isReference (and also outputReferences option if they have that) prop to determine whether to output it or not

[luupanu]

Personally I see collision warnings important in telling me there probably is a mistake somewhere since output may be completely unexpected. In fact we have our CI setup in a way that the build does not pass if there are collisions detected. It wouldn't be great at least to have to deduce somehow if some collision warning is important or not, so I support the API redesign in that way.

Would collisions between foo.json and the base layer in your example still log a warning? Basically from what I gather from your example everything inside an array would drop the warnings.

[jorenbroekema]

Would collisions between foo.json and the base layer in your example still log a warning? Basically from what I gather from your example everything inside an array would drop the warnings.

Files originating from globs that are inside the same nested array (layer) will throw collision warnings if there are any. Files from layers colliding with files from outside the layer will also throw collision warnings if there are any. (so "yes" to answer your question shortly) The only reason it wouldn't throw a warning is if the collision is happening in files that are both inside layers but not the same layer, so across layers