amzn / style-dictionary

A build system for creating cross-platform styles.
https://styledictionary.com
Apache License 2.0
3.87k stars 543 forks source link

feat: allow deferring transitive transforms from the transformer #1069

Closed jorenbroekema closed 8 months ago

jorenbroekema commented 9 months ago

Issue #, if available: fixes https://github.com/amzn/style-dictionary/issues/1073 Related: https://github.com/tokens-studio/sd-transforms/issues/211

See test for use case, but in summary: support transitive transforms that rely on sibling props for metadata in order to do the transformation, and allowing references in them. For this, it's necessary to allow letting the transformer decide that a prop's transformation should be deferred until the next cycle of reference resolutions, since only the transformer has the context about why deferring is necessary. returning undefined seemed to make the most sense to me, it says "the transformation could not be defined, so try again later"

I mostly created this draft PR to prove that it can work without breaking other behaviors or types. TODO before merging:

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

dbanksdesign commented 8 months ago

I think this makes sense to me. I'm trying to think if there is another way to do it without having the transformer function return undefined. It makes sense, but it also seems kind of magical, like secret behavior. I can't really think of anything better at the moment.

jorenbroekema commented 8 months ago

I think this makes sense to me. I'm trying to think if there is another way to do it without having the transformer function return undefined. It makes sense, but it also seems kind of magical, like secret behavior. I can't really think of anything better at the moment.

Yeah exactly, I agree, it's the best thing I could come up with even though it's a bit magical. I was also thinking about Rollup's hooks https://rollupjs.org/plugin-development/#resolveid for example their resolveId hook that can return false to treat a module as external or return null to defer the resolution to the next plugin. So I also considered returning false or null, but null and false might be somewhat valid token values, whereas undefined never is as far as I can tell.

Anyway, I'll start wrapping up this PR and ping you when it's ready