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

HMR for config imports and latency improvement #682

Closed
TechAkayy opened this issue May 19, 2023 · 17 comments · Fixed by #795 or #818
Closed

HMR for config imports and latency improvement #682

TechAkayy opened this issue May 19, 2023 · 17 comments · Fixed by #795 or #818
Assignees

Comments

@TechAkayy
Copy link

TechAkayy commented May 19, 2023

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

  1. Clone both repos, install deps and start dev-servers.

  2. 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
  3. 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
  4. 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.

@TechAkayy TechAkayy added the bug Something isn't working label May 19, 2023
@ineshbose
Copy link
Collaborator

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 ineshbose changed the title HMR issues when using tailwind with & without our module HMR for config imports and latency improvement May 19, 2023
@ineshbose ineshbose removed the bug Something isn't working label May 19, 2023
@ineshbose
Copy link
Collaborator

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
Copy link
Author

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
Copy link

Pezhvak commented Jul 21, 2023

Any updates on fixing this?

@ineshbose
Copy link
Collaborator

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
Copy link
Collaborator

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

@ineshbose
Copy link
Collaborator

Hi @TechAkayy

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

@TechAkayy
Copy link
Author

TechAkayy commented Feb 22, 2024

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
  1. pnpm dev
Pasted Graphic 8
  1. 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 Vite server hmr 245 files in 566 177ms
  1. 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
  1. 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
  1. 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
  1. Overrid the primary[600] in the tailwind config. Result - HMR applied, page reloaded, button background is now red.
Ready to dive in
  1. 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 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

Pasted Graphic 19 500

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

Pasted Graphic 18 Pasted Graphic 21 500

@ineshbose
Copy link
Collaborator

  • 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
Copy link
Author

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?

image

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

@ineshbose
Copy link
Collaborator

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
Copy link

MickL commented Apr 11, 2024

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
Copy link
Collaborator

ineshbose commented Apr 11, 2024

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

@ineshbose
Copy link
Collaborator

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)

@ineshbose ineshbose reopened this Apr 12, 2024
@MickL
Copy link

MickL commented Apr 12, 2024

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
Copy link
Collaborator

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
Copy link
Collaborator

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants