Feat: support tailwind config files #2831
Conversation
🦋 Changeset detectedLatest commit: 19f29cb The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
bholmesdev
force-pushed
the
feat/support-tailwind-config-files
branch
from
866e434
to
9f860b9
Compare
bholmesdev
force-pushed
the
feat/support-tailwind-config-files
branch
from
af6517b
to
2c7e80f
Compare
| | undefined; | ||
|
|
||
| export default function (integrationConfig: IntegrationConfig): AstroIntegration { | ||
| const applyAstroConfigPreset = integrationConfig?.config?.applyAstroPreset ?? true; |
There was a problem hiding this comment.
something I'll keep in mind: setting default variables options seems to be a common use-case for integrations. Nuxt ships with a helper for integrations to define default options easily, maybe could do the same in the future.
| const userConfig = await getUserConfig(config.projectRoot, integrationConfig?.config?.path); | ||
|
|
||
| let tailwindConfig: TailwindConfig; | ||
| if (typeof userConfig?.value === 'object' && userConfig.value !== null) { |
There was a problem hiding this comment.
If userConfig?.value returned something but that something wasn't an object, I almost would want this to be an error that the user sees, instead of silently failing this check and using the default config below.
Can we do the simpler if (userConfig && userConfig.value) { here?
There was a problem hiding this comment.
Actually, taking a step further back: what happens if options.path is given to the integration, but then fails to load? I'd expect that to be an error for the user to investigate & fix.
Another good reason to do this is that it should really let you simplify the code below where you're trying to handle this tricky case. For example, I think it would mean that you could just do this, below:
/* throw if options.path was given, but userConfig or userConfig.value failed to load */
const tailwindConfig = userConfing.value ?? getDefaultTailwindConfig(config.src);
if (applyAstroConfigPreset) {
tailwindConfig.presets.push(getDefaultTailwindConfig(config.src));
}
There was a problem hiding this comment.
Ah, great thinking! One issue with this refactor though: it'll throw an error when a tailwind.config is missing no matter what. Ideally, it would log a human-readable error specifically when you pass a bad config.path. Made a refactor here.
I swear I'll remember this by like... week 8 😆 |
lol when you figure it out please let me know your secret :) |
|
|
||
| const tailwindConfig: TailwindConfig = (userConfig?.value as TailwindConfig) ?? getDefaultTailwindConfig(config.src); | ||
| if (applyAstroConfigPreset) { | ||
| tailwindConfig.presets = [...(tailwindConfig.presets || []), getDefaultTailwindConfig(config.src)]; |
There was a problem hiding this comment.
So if I'm reading this correctly, tailwindConfig becomes:
{
...tailwindConfig,
presets: [...tailwindConfig.presets, getDefaultTailwindConfig(config.src)]
}
does this make sense? It feels odd to put the entire default tailwind config here as a preset. If this is expected, a comment would help future readers not get confused.
There was a problem hiding this comment.
Yep, just realized this was confusing! Added a clarifying comment here, and only applied the preset when a user config is present (otherwise we're applying the default config twice).
There was a problem hiding this comment.
Update: discovered that adding a preset removed Tailwind's default config. Thought they would merge 😕 Used the resolveConfig function to merge those defaults back in! 19f29cb
Changes
tailwind.config.jsin your project. This PR changes that!Testing
Docs