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: considered content as function #592

Merged
merged 8 commits into from
Jan 25, 2023
Merged

fix: considered content as function #592

merged 8 commits into from
Jan 25, 2023

Conversation

ineshbose
Copy link
Collaborator

@ineshbose ineshbose commented Jan 14, 2023

#591

I'm not sure I had seen this implemented before, so I'm wondering how it was on the docs 😅

I've written tests for this, and updated the docs.

However, there would still be some work to do (looking at type definitions), so I'd like feedback and guidance!

PS. If I have your attention on this, could I please ask you to review #583 too? 😉

@nuxt-studio-dev
Copy link

nuxt-studio-dev bot commented Jan 14, 2023

Deploy Preview for tailwindcss ready!

Name Link
🔨 Latest commit 763a71e
😎 Deploy Preview https://tailwindcss.nuxt.dev?preview=1b94109fafa30aa94ce78d766ba5b7eb

Copy link
Collaborator

atinux commented Jan 16, 2023

Actually I think it comes from this line:

tailwindConfig = defu(_tailwindConfig || {}, tailwindConfig)

That does not leverage https://github.com/unjs/defu#array-function-merger 🤔

@ineshbose
Copy link
Collaborator Author

I missed the usage of defuArrayFn here:

tailwindConfig = defuArrayFn(tailwindConfig, moduleOptions.config)

Let me investigate with the written tests!

@ineshbose
Copy link
Collaborator Author

The issue also affects this too unfortunately:

tailwindConfig.content = [...(tailwindConfig.content || []), ...contentPaths]

because a function is not an iterable. In this case typeof tailwindConfig.content === 'function' may be necessary. All in all, these three lines are affected if I'm not wrong:

tailwindcss/src/module.ts

Lines 133 to 139 in c32cadb

tailwindConfig = defu(_tailwindConfig || {}, tailwindConfig)
}
tailwindConfig.content = [...(tailwindConfig.content || []), ...contentPaths]
// Merge with our default purgecss default
tailwindConfig = defuArrayFn(tailwindConfig, moduleOptions.config)

@ineshbose
Copy link
Collaborator Author

ineshbose commented Jan 16, 2023

I think that should help. Please let me know any feedback - I am still wondering about the config type definition.

Edit: ..or we can not deal with it (which I've gone for in the latest commit) 😉

@ineshbose ineshbose marked this pull request as ready for review January 16, 2023 19:06
@atinux atinux merged commit cf67d89 into nuxt-modules:main Jan 25, 2023
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