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

feat: add a module option to disable tailwind global style injection in all pages #850

Conversation

RodrigoProjects
Copy link

Currently when we install this module we cannot opt-out of injecting the base tailwind file in all pages of our nuxt application, this PR makes manual handling and injection of the tailwind CSS at the will of the user if it wants to control the flow!

Initially I was gonna provide a composable that helps the injection with useHead but I think this could another PR for now

Copy link

netlify bot commented May 22, 2024

Deploy Preview for nuxt-tailwindcss ready!

Name Link
🔨 Latest commit 2928fe0
🔍 Latest deploy log https://app.netlify.com/sites/nuxt-tailwindcss/deploys/664f43f2b5f5090008c621fc
😎 Deploy Preview https://deploy-preview-850--nuxt-tailwindcss.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

what-the-diff bot commented May 22, 2024

PR Summary

This PR introduces several enhancements and modifications to various sections of the codebase. Here's a more simplified, non-technical summary:

  • Update to code configuration

    • in app.config.ts and tailwind.config.ts, we have updated some configurations. This adjusts the appearance of certain elements and the overall theme of the software to make it look consistent and more eye-catching.
  • UI modifications

    • In AppFooter.vue, we've added a link that leads to the license information. This helps users understand the terms under which the software is provided.
    • In AppHeader.vue, we've added a conditional logic to the header section. This essentially means the search box in the header will only display if certain conditions are met, enhancing usability for certain user scenarios.
  • Code formatting and readability improvement

    • In several files (AppHeader.vue, TailwindText.vue, TheLogo.vue, context.ts, module.ts), new lines were added to make the code easier to read and comprehend by separating different sections more clearly.
  • Enhancement of software modularity

    • The module.ts and types.ts files have a new configurable option globalInjection added. By default, it's turned on(true). This gives users more flexibility by letting them decide whether to use certain features globally or not.
  • Code Cleanup

    • In sink.test.ts, we deleted an unneeded comment, promoting better clarity and cleanliness in the codebase.

@ineshbose
Copy link
Collaborator

ineshbose commented May 22, 2024

Hey, thanks for the PR (most of the diff is linting), but for the globalInjection option, can't cssPath be set to false and then you don't have Tailwind injected globally?

@RodrigoProjects
Copy link
Author

Hello @ineshbose! That really makes sense, thanks for the comment and sorry for the duplicate feature!

What you think of providing a composable that injects the styles for tailwindcss manually tho?

@ineshbose
Copy link
Collaborator

What you think of providing a composable that injects the styles for tailwindcss manually tho?

Yes - I'm all for scoping Tailwind CSS only where required/used, but I believe that to be against the principles of Tailwind to be a global styling utility, so there's lot of discussion in the community as far as I know. We definitely want to provide options to developers, but lot of it depends on what technical implementation we can do (trying to make an unplugin may help us in many ways, but still will need lot of R&D afaik). Moreover, Tailwind 4 also changes Tailwind from a PostCSS plugin to a Vite plugin that will change behaviour of the CSS file greatly.. it's a bit in the air at the moment, but the guide would tell you to keep your Tailwind styles global and declarative; more than that, you can set up configurations to your project, and we may add it to the module level based on need/demand. 🙂

@RodrigoProjects
Copy link
Author

That is the ultimate end goal for sure and I really have the hopes up for tailwind V4 as well!

I am trying to suggest a middle ground for now in a more macro manner, I have a big CMS that has some pages that don't really use tailwind and are affected by some tailwind plugins I have like the forms reset one. Switching the import of the base tailwind styles to a runtime process would really benefit me at some google metrics and not breaking pages that are not designed to use tailwind, makes sense?

@ineshbose
Copy link
Collaborator

Yes, I hear you. It makes sense, and as you pointed, we could probably make use of useHead in that case. Are you able to configure something with cssPath: false and then a custom composable around useHead that adds in the Tailwind stylesheet?

@RodrigoProjects
Copy link
Author

Yap! Will come with something for you to review tomorrow if you have time 🙂

@RodrigoProjects
Copy link
Author

Okey, I added the new composable using useHead and it is injecting the resolveCss path into de DOM and works in SSR too (by consequence of using useHead) but I am trying to find a way to make the tailwind.css file a public asset so that I can have a style tag that can import the tailwind file.

Currently from my research nitro only supports adding new public assets dirs (nuxt/nuxt#13079) which we could make an hack and add the tailwind.css to a folder adding it to nitro public assets.

When using nuxt.options.css we pass it to Vite and handles global styles very well but for the new lazy use case we need a way to convert/move the tailwind.css file into an asset we can fetch.

I am open to more ideas and maybe this could an use case for @pi0 to make that closed PR open again!

@pi0
Copy link
Contributor

pi0 commented May 23, 2024

Sorry i haven't went through all discussions so quickly answering here:

  • If a public (non bundled) asset is being emitted, it should be hashed otherwise cache mismatches can happen (or at least a fetch function fetches it as /tailwind.config?{hashOrTimestamp}.
  • Nitro only supports additional dirs for custom static assets, you probably need to generate a dir in a temp file (using nitro hook is also possible to manually write file to build public folder)

@RodrigoProjects
Copy link
Author

Sorry i haven't went through all discussions so quickly answering here:

  • If a public (non bundled) asset is being emitted, it should be hashed otherwise cache mismatches can happen (or at least a fetch function fetches it as /tailwind.config?{hashOrTimestamp}.
  • Nitro only supports additional dirs for custom static assets, you probably need to generate a dir in a temp file (using nitro hook is also possible to manually write file to build public folder)

No need to say sorry here thanks for your work and making my life 10x easier!

Are you against me opening a PR in Nitro supporting single file asset additions like publicFiles option in the Nitro config that could also support hashing based on the content? Basically the same logic you do for dirs but giving an options of just providing files without the dir

@ineshbose
Copy link
Collaborator

When using nuxt.options.css we pass it to Vite and handles global styles very well but for the new lazy use case we need a way to convert/move the tailwind.css file into an asset we can fetch.

I think an unplugin or a Nitro plugin hooking into render:html may do here.. or maybe not.

I am trying to find a way to make the tailwind.css file a public asset so that I can have a style tag that can import the tailwind file

Could you elaborate on this a little for me please? With the useHead based composable, were you using link and that isn't sufficient?

@ineshbose
Copy link
Collaborator

I am closing this PR for now, but feel free to continue discussion. 🙂

(And if you're looking to discuss creating a PR on Nitro, please do create an issue on https://github.com/unjs/nitro, Pooya may miss this thread as he doesn't triage this module!)

@ineshbose ineshbose closed this May 30, 2024
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.

3 participants