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(exposeConfig): named exports in mjs for tree-shaking #582

Merged
merged 7 commits into from Dec 19, 2022

Conversation

ineshbose
Copy link
Collaborator

Resolve #481

@danielroe
Copy link
Contributor

I don't believe this would resolve that issue. The issue that people have raised is that the tailwind config is exposed as a single default export: #481 (comment).

@ineshbose ineshbose changed the title fix(exposeConfig): mjs with write as true Draft: fix(exposeConfig): mjs with write as true Dec 13, 2022
@ineshbose ineshbose marked this pull request as draft December 13, 2022 15:01
@ineshbose ineshbose changed the title Draft: fix(exposeConfig): mjs with write as true fix(exposeConfig): mjs with write as true Dec 13, 2022
@ineshbose
Copy link
Collaborator Author

Thanks for your feedback @danielroe. Just so I can confirm, the module still wishes to have tree-shaking (as also documented on https://tailwindcss.nuxt.dev/tailwind/config#referencing-in-the-application as an example)?

The write: true argument would still be required regardless if I'm not wrong. However, you're right about the object still in default export not resolving it - do you think each property would need to be exported individually instead?

@danielroe
Copy link
Contributor

What makes you think that this needs to be written? I don't believe that is required or desirable (it has a negative performance impact).

You are right that the docs need to be updated (or the virtual module needs to also have named exports).

@ineshbose
Copy link
Collaborator Author

The tailwind.config.mjs was not being created without write: true even when I had exposeConfig: true.

I can try to look into having named exports into the content (reiterate - TRY 😄) since it would be desirable to have tree shaking (in my use at least).

Copy link
Contributor

danielroe commented Dec 13, 2022

It doesn't need to be written to disk; it's a virtual file that will still be included in the build, if you import from it.

@ineshbose
Copy link
Collaborator Author

My apologies - I wasn't aware of that functionality!

Copy link
Contributor

No problem at all! Nuxt 3 makes extensive use of virtual modules for most source files it provides (as distinct from type declarations, which we write to disk so IDEs are aware of them).

@ineshbose ineshbose changed the title fix(exposeConfig): mjs with write as true fix(exposeConfig): named exports in mjs for tree-shaking Dec 13, 2022
@ineshbose ineshbose marked this pull request as ready for review December 13, 2022 15:46
src/module.ts Outdated Show resolved Hide resolved
src/module.ts Outdated Show resolved Hide resolved
@danielroe
Copy link
Contributor

perfect 👌

annoyingly, now we also have to update the declaration file below it so users have intellisense.

@ineshbose
Copy link
Collaborator Author

Completely forgot about that. So sorry for the back-and-forths, I'll do that.

@ineshbose
Copy link
Collaborator Author

ineshbose commented Dec 13, 2022

Note - not every export has a type definition, and I'm not sure if tailwindcssConfig is an appropriate name!

Could also just put the configuration values in type definition for it to be const.

Copy link
Contributor

@danielroe danielroe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

perfect - thanks! ❤️

@Atinux Atinux merged commit 6c3168d into nuxt-modules:main Dec 19, 2022
4 checks passed
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.

exposeConfig with specific part is undefined
3 participants