Skip to content

Conversation

adamwathan
Copy link
Member

This is an alternative to #639 which I think may be a better solution (or maybe not!)

Instead of each plugin internally knowing how to find the right values it should use from the theme, this PR pushes that logic to the existing configuration merging layer, so that there is always a "complete" config at the end of the day, instead of a config that is completely missing the backgroundColors, borderColors, and textColors keys.

Essentially this flattens/freezes any sort of inherited value at the merge layer, so that each plugin simply receives it's configuration directly.

There are pros and cons to both approaches annoyingly, but at least today I am convinced this is the better approach.

My main argument for it internally is that I foresee a future where one day I split out all of the "framework generation" code from Tailwind into another project, like maybe tailwindcss/engine, and that project is a PostCSS plugin that only deals with plugins and has no concept of a default theme or default styles.

If that project existed, I would want to be able to use the utility plugins from Tailwind as plugins to the engine without there ever being weird errors about things like "key 'theme' not found" because the plugins are reaching up to the config looking for their values. If every plugin is configured explicitly from the outside in, they would all be straightforward to use in the engine context.

The only real con to this approach is that this merge layer could continue to grow and get more complicated if we introduce other "magic" shared keys for things like spacing or sizing. There is something admittedly nice about keeping all of the fallback/inheritance logic for a specific plugin localized within that plugin. Hard call.

@hacknug
Copy link
Contributor

hacknug commented Feb 6, 2019

spacing and sizing 🎉

@adamwathan
Copy link
Member Author

Closing in favor of #645.

@adamwathan adamwathan closed this Feb 6, 2019
@adamwathan adamwathan deleted the merge-colors branch May 13, 2019 14:01
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