HMR for config imports and latency improvement

TechAkayy commented 1 year ago

Version

@nuxtjs/tailwindcss: v6.7.0 nuxt: v3.5.0

Reproduction Link

Without nuxt tailwind module (no HMR issues)

With nuxt tailwind module (HMR issues)

Steps to reproduce

I have exactly the same repo: vanilla - the first one without nuxt tailwind module, following official tailwind docs for nuxt 3 (https://tailwindcss.com/docs/guides/nuxtjs#3) module - the second one using our nuxt tailwind module

I maintain my color palette (primary, secondary etc) in an external file called tokens.cjs and they extend my theme (via a tw plugin).

Firstly, when updating the token.cjs file, HMR doesn't work with the module version that uses our nuxt tailwind module.

Secondly, when updating the tailwind config file, unlike the vanilla version, it doesn't apply the changes instantly, instead app restart taking few seconds to load and apply.

Here is a reproduction demo on youtube - https://www.youtube.com/watch?v=WY0UpPQRXYQ

Clone both repos, install deps and start dev-servers.
In tokens.cjs file, swap the primary palette with the commented one beneath it.
- In vanilla version, the whole app instantly HMR applies the new primary color palette
- In module version, the changes are not applied and it requires a manual restart of the dev-server to apply every atomic change
In tailwind config file, there is a custom plugin that adds a "rainbow" custom class (used in the "WELCOME TO" title in hero section at the top of my homepage). Change from-purple-500 to from-green-500.
- In vanilla version, the gradient change is instantly applied
- In module version, the gradient change gets applied, but the app re-start seems to take a few seconds
The tailwind config files in both apps are available as tailwind.config.alt.ts and can be swapped with the existing tailwind.config.cjs and the above issues are the same. So, probably not related to using tailwind config file in cjs format.

What is Expected?

The HMR updates should instantly apply.

What is actually happening?

My app pretty much does what's nicely covered in the new @nuxthq/ui library. It maintains color pairs in tailwind.css entry file, and swaps them for light/dark mode (material design inspired), and apply them to my BaseSheet & BaseButton components.

These HMR issues makes it hard for me (from a DX perspective) to adopt the new nuxt-ui library as it has a hard dependency on our nuxt tailwind module. Wondering if it's possible to use the new nuxt-ui library without nuxt tailwind module, as tailwind v3.3 introduced esm/ts config files.

Thanks for looking into this.

ineshbose commented 1 year ago

Hi, thanks for opening an issue and reporting your feedback on the HMR responsiveness from the module.

This is definitely an area of improvement and would require some investigation - I'll look into resolving this as fast as possible (but PR also very welcome 🙂)

ineshbose commented 1 year ago

Couple of notes based on my investigation:

A big difference in configuration between vanilla Tailwind CSS and this module is that this module allows multiple configurations to be input (from configPath as array option or Nuxt layers). These are added to nuxt.options.watch and we use defu for the resulting merged configuration.

Vanilla Tailwind CSS (as a PostCSS plugin) looks for a single configuration file for which it then creates a tracking context (https://github.com/tailwindlabs/tailwindcss/blob/776d5bf614cbc5ee5c3979cac8b2fcc65a2b7b17/src/lib/setupTrackingContext.js) which works pretty well, but again, this is restricted to a single configuration.

This creates a hurdle - what can we do? Maybe we have separate postcss loaders but it can be logistically complex? Maybe we can just make use of presets in twconfig with require(configPaths) but the merging strategy isn't what we're looking for? Maybe we can use createTemplate that imports all configurations and exports default defu(importedConfigs) and pass the template dst to tailwindcss (pcss plugin) but this could be fragile?

TechAkayy commented 1 year ago

Thanks bunch @ineshbose. I'm not proficient enough to weigh the options. As a Nuxt end-user, would be good to not lose the experience with HMR, as most of the users won't be looking at merging configs, rather they just want to install the module and start using tailwind, and avoid creating a seperate config file.

May be some sort of escape hatch where if there was no multiple config merge required, then let the user get the default HMR experience? So, if not users of libraries like nuxtlabs/ui, at least most of the users using vanilla setup benefits from this. Sorry, I know I'm not very helpful!

Pezhvak commented 1 year ago

Any updates on fixing this?

ineshbose commented 1 year ago

Still in brainstorming process, and I can't think of implementing this without breaking changes at the moment, so I can't say it'll come out anytime soon. But very open to ideas and suggestions please as we're still brainstorming and I may have missed something.

ineshbose commented 6 months ago

Config HMR should be available to try out on nightly. :)

ineshbose commented 6 months ago

Hi @TechAkayy

Please do let me know how the experience goes with this change - I will make improvements/release accordingly.

TechAkayy commented 6 months ago

Good day @ineshbose,

Thank you for the amazing help. I tested out the nightly build on three apps.

App-1. (pnpm) The playground. Downloaded it to my local machine from https://stackblitz.com/github/nuxt-modules/tailwindcss/tree/first-class-hmr. The repo with my testing (screenshots of the positive outcomes in expandable section below) - https://github.com/TechAkayy/pg-nuxt-tw-module-first-class-hmr

HMR worked beautifully, and here is a quick summary:

Updates to tailwind config either in the imported module (for eg, colors from tokens.cjs imported in tailwind.config.ts) or directly in tailwind.config.ts - updates the app instantly with HMR. The HMR is consumed by the page's vite client, and the page is reloaded (which is a valid HMR outcome in certain scenario such as this). Stackblitz doesn't show the page reload (toolbar reload icon), but can be noticed in the devtools console. Locally, the browser does show the page reload in it's toolbar icon.
Updates to vue SFC files - updates the app instantly with HMR without page-reload (as expected, as before, as always).
During my testing, nuxt dev-server didn't restart which is perfect & amazing 👍, and this is the desired outcome (vanilla tw setup without module also works this way in the original reproduction).
Two quick questions, when kick-starting the dev-server, in the logs - 1. There is a warning about functional plugins, wondering how to avoid to it, also I believe it should say configPath instead of cssPath? 2. There is a reference to tailwind.config.cjs even though there is not such file involved, this could be confusing to the user (or may be not, not sure)?

The screenshots from my testing are within this expandable section -

Test screenshots (nuxt dev-server didn't restart during these tests)

1. `pnpm install` Pasted Graphic 7

2. `pnpm dev` Pasted Graphic 8

3. Added my themes folder (from the original reproduction), imported it within tailwind config and extended tw theme colors. Result - HMR applied, page reloaded. Ready to dive in

4. Updated `index.vue` page, and `CallToAction.vue` component by using `primary` color from my imported theme. Result - HMR applied, page reloaded, app is updated with the primary color (the big button & a paragraph using color from tailwind config). Ready to dive in

5. Replaced the `primary` color-set in the imported `themes/tokens.cjs` file. Result - HMR applied, page reloaded, app is updated with the new `primary` color-set Ready to dive in

6. Updated `primary[600]` color to purple in the imported `themes/tokens.cjs` file. Result - HMR applied, page reloaded, button background is now purple. Ready to dive in

7. Overrid the `primary[600]` in the tailwind config. Result - HMR applied, page reloaded, button background is now red. Ready to dive in

8. Changed the `bg-primary-600` color in `CallToAction.vue` to use `bg-brand` that's coming from the `theme` layer. Result - HMR applied, NO PAGE RELOAD (as expected), button background is now from the `theme` layer. Updating the color to red in the tailwind config of the `theme` layer applies HMR without a page-reload. Ready to dive in

App-2. (pnpm) The docs at https://stackblitz.com/github/nuxt-modules/tailwindcss/tree/first-class-hmr. The repo with my testing - https://github.com/TechAkayy/pg-nuxt-tw-module-first-class-hmr (same as 1 above). I'm probably doing something wrong, or missing something, please let me know if you can see it.

pnpm dev:docs

App-3. (npm) My own app using nuxt-ui (cc: @benjamincanac) - https://github.com/TechAkayy/pg-nuxtui-tw-module-first-class-hmr. I'm probably doing something wrong, or missing something, please let me know if you can see it.

npm run dev

ineshbose commented 6 months ago

Updates to tailwind config either in the imported module (for eg, colors from tokens.cjs imported in tailwind.config.ts) or directly in tailwind.config.ts - updates the app instantly with HMR. The HMR is consumed by the page's vite client, and the page is reloaded (which is a valid HMR outcome in certain scenario such as this). Stackblitz doesn't show the page reload (toolbar reload icon), but can be noticed in the devtools console. Locally, the browser does show the page reload in it's toolbar icon.

Updates to vue SFC files - updates the app instantly with HMR without page-reload (as expected, as before, as always).

During my testing, nuxt dev-server didn't restart which is perfect & amazing 👍, and this is the desired outcome (vanilla tw setup without module also works this way in the original reproduction).

Thanks for confirming. The page reload is bound to happen as we're changing the (Post)CSS here, but as you pointed, the main part is that the Nuxt server does not need to restart, so the behaviour is the same as vanilla installation now.

Two quick questions, when kick-starting the dev-server, in the logs - 1. There is a warning about functional plugins, wondering how to avoid to it, also I believe it should say configPath instead of cssPath? 2. There is a reference to tailwind.config.cjs even though there is not such file involved, this could be confusing to the user (or may be not, not sure)?

This change unfortunately came with a few caveats. Tailwind CSS configuration includes some non-serializable items in the configuration, such as plugins (defined as runtime JS functions), regex in safelist (this is the issue we're facing with nuxt/ui, for which I pushed an update/workaround in the PR /cc @benjamincanac), etc. For functional plugins, Oxide allows plugins to be defined as strings and that'll be SUPER convenient to developers, but it isn't released yet and I'm not sure when it would. I'm thinking how to best document the changes for this feature and if this becomes a major breaking change or not.

TechAkayy commented 6 months ago

Thanks @ineshbose for the clarifications. Does the below tailwind typography plugin usage consuming some plugin configurations (plz refer to className: 'wysiwyg' in the below screenshot, tailwind way), classify it as a functional plugin?

Does using functional plugin mean that this module can't be used? Or simply HMR won't work?

Also, is the reference to cssPath in this warning message correct? Or should it be configPath?

ineshbose commented 6 months ago

Does the below tailwind typography plugin usage consuming some plugin configurations (plz refer to className: 'wysiwyg' in the below screenshot, tailwind way), classify it as a functional plugin?

Yes, that is a require('@tailwindcss/typography') which imports a function. With oxide, you can just to "@tailwindcss/typography" so that is a string and it'll be fine.

Does using functional plugin mean that this module can't be used? Or simply HMR won't work?

The module can be used, but not with plugins passed in inline config; as a separate configuration it is fine. If you don't have plugins in inline-config, check if any of your modules are adding it.

Also, is the reference to cssPath in this warning message correct? Or should it be configPath?

Yes definitely configPath - thanks for pointing that out!

Will reopen this issue till stable release is made.

MickL commented 5 months ago

Hey @ineshbose what is the status on this one? I am working a new project I having huge trouble setting up the tailwind config. I would like to save my file and see the changes but Nuxt restarts and the changes are no visible. I always need to restart the whole node/bun process.

ineshbose commented 5 months ago

Hey @MickL, just on the final leg for #818 - figuring out invalidating .nuxt/tailwind.config.cjs when the template is regenerated.

ineshbose commented 4 months ago

Change has been matured and landed on Nightly! Please test and share your feedback so we can target a stable release for this 😄 (reopening for now)

MickL commented 4 months ago

First try looks good, no more reloading of the Nuxt app when changing something. LOVE IT :) Any thing to look for in particular that could not work stable?

ineshbose commented 4 months ago

Thanks for testing ❤️

Any thing to look for in particular that could not work stable?

I believe it to be stable; its just trying to find edge cases if any - the fragile parts are mostly using inline plugins (which will disable HMR as fallback) and updating configs using the tailwindcss:loadConfig and tailwindcss:config hooks. You'll see a template generated in .nuxt/tailwind.config.cjs that has all updates along with merging strategy, and any form of invalid syntax would break it - so feel free to experiment 😄

ineshbose commented 4 months ago

I am happy to say that this has been released and this issue can be closed. 😄

nuxt-modules / tailwindcss