vanilla-extract icon indicating copy to clipboard operation
vanilla-extract copied to clipboard

@vanilla-extract/vite-plugin loads css files multiple times in Storybook when using theme

Open SamBernaerdtOntoforce opened this issue 1 year ago • 25 comments

Describe the bug

Minimal Problem: When using themes, vite loads a new (filename).css.ts.vanilla.css?t=1728553686586 file for every other file that uses itin the development build of storybook.

image

Expected: Each .css.ts file should only be loaded once (until it changes and causes a hot reload)

Problem: This might not be an issue on the small reproducible project, but on a larger project, where multiple theme files are composited, each of the theme files is loaded for each component using a theme value. This causes so many file loads that it freezes storybook.

image

Reproduction

https://github.com/SamBernaerdtOntoforce/vanilla-extract-theme-and-storybook

System Info

System:
    OS: macOS 14.7
    CPU: (12) arm64 Apple M2 Pro
    Memory: 279.81 MB / 32.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 21.7.3 - ~/.nvm/versions/node/v21.7.3/bin/node
    Yarn: 1.22.19 - ~/.nvm/versions/node/v21.7.3/bin/yarn
    npm: 10.5.0 - ~/.nvm/versions/node/v21.7.3/bin/npm
    pnpm: 8.8.0 - ~/.nvm/versions/node/v21.7.3/bin/pnpm
  Browsers:
    Chrome: 129.0.6668.100
    Edge: 129.0.2792.79
    Safari: 18.0
  npmPackages:
    @vanilla-extract/css: ^1.16.0 => 1.16.0
    @vanilla-extract/vite-plugin: ^4.0.16 => 4.0.16
    vite: ^5.4.8 => 5.4.8

Used Package Manager

yarn

Logs

No response

Validations

SamBernaerdtOntoforce avatar Oct 10 '24 11:10 SamBernaerdtOntoforce

Thanks for the reproduction! It definitely looks like there could be an optimization here. This doesn't seem related specifically to Storybook as I can reproduce the issue using just a vite dev server.

It seems that the issue starts once HMR has kicked in at least once. Different files that share the same dependency (e.g. shared.css.ts) end up with different timestamped imports of that file, causing both of them to be loaded, even though only 1 is necessary.

I'm also seeing some inconsistency in how many .vanilla.css files are loaded. Occasionally each file is loaded only once: image

However most of the time the same file is loaded a few times (but with different timestamps): image

This could be a result of how vite loads modules during development, and maybe there's nothing we can do to change it. I'll need to dig into it some more.

askoufis avatar Dec 11 '24 04:12 askoufis

I am experiencing this exact same issue. I'm working in a pnpm monorepo and just migrated our storybook to the vite builder. I've tweaked my vite configuration a lot of different ways:

  • preserveSymLinks: true is a non-starter as it blows up so many other transitive dependency imports
  • adding several libs to optimizeChunks.include seems to have alleviated some of the issue, because dependency re-optimization is not causing as much HMR churn, but it's still an issue.

I suspect that breaking up some of my barrel files will also help with the impact of this bug, but ultimately won't address the root cause.

It's very problematic and a total performance killer in dev mode in a large project with a lot of VE CSS. I'm investigating, but I'm new to the internals of Vanilla Extract and Vite so any guidance/pointers would be appreciated.

hartz89 avatar May 16 '25 00:05 hartz89

I have opened PR #1591 to address this issue with a new plugin option. Please let me know if you have feedback on the approach. Initial builds + HMR were working as expected in my local testing.

hartz89 avatar May 20 '25 16:05 hartz89

Also worth noting that, even without this PR, the following config change drastically improved startup times because it minized dependency re-optimization and related HMR churn:

export default defineConfig({
  plugins: [vanillaExtractPlugin()],
  optimizeDeps: {
    include: [
      '@vanilla-extract/recipes/createRuntimeFn',
      '@vanilla-extract/sprinkles/createRuntimeSprinkles',
    ],
  }
});

I wonder if the plugin should do that by default, or if it's better to let consumers opt-in. In any event, would it be a worthwhile addition to the Vite plugin docs?

hartz89 avatar May 20 '25 20:05 hartz89

Also worth noting that, even without this PR, the following config change drastically improved startup times because it minized dependency re-optimization and related HMR churn:

export default defineConfig({ plugins: [vanillaExtractPlugin()], optimizeDeps: { include: [ '@vanilla-extract/recipes/createRuntimeFn', '@vanilla-extract/sprinkles/createRuntimeSprinkles', ], } });

I wonder if the plugin should do that by default, or if it's better to let consumers opt-in. In any event, would it be a worthwhile addition to the Vite plugin docs?

This also fixes HMR issues in Storybook with Vite.

oliveryasuna avatar May 25 '25 02:05 oliveryasuna

I believe I've addressed at least part of this issue in this branch. You can try out this fix in @vanilla-extract/[email protected]. It should at least prevent unnecessary loading of the same module multiple times with a different HMR timestamp.

@hartz89 As I mentioned in my discord reply to your message, I'm unsure if a file cache is the correct solution here. The compiler is already caching generated CSS, and ultimately the correct thing to do when a leaf node module is invalidate is to invalidate the module tree upwards. Importer modules can't have any knowledge of what has changed in the leaf node, so the best approach to get accurate HMR is to re-build everything.

I will investigate your optimizeDeps solution too.

@SamBernaerdtOntoforce Please try this out in a larger project if you have one. IMO splitting out your theme tokens in such a granular way is part of the problem here, though without understanding your use case I can't say for certain whether there's a better way to structure them. Regardless, hopefully this fix should help your situation a bit.

askoufis avatar Jul 06 '25 08:07 askoufis

@askoufis tried @vanilla-extract/[email protected] on our project

we have heavily sprinkles based CSS of atomic nature and almost every component's css.ts imports some sprinkle from a pretty complex sprinkles.css.ts file

basically nothing has changed — cold start is very rough, I see like 100 requests to a sprinkles file (see screenshot)

we have everything vanilla extract related added to optmizeDeps.include and that helps a bit

include: [
        '@vanilla-extract/css-utils',
        '@vanilla-extract/css',
        '@vanilla-extract/css/fileScope',
        '@vanilla-extract/dynamic',
        '@vanilla-extract/recipes',
        '@vanilla-extract/recipes/createRuntimeFn',
        '@vanilla-extract/sprinkles',
        '@vanilla-extract/sprinkles/createRuntimeSprinkles',
],
Image

after first initial rought sequence everything is kind of ok (usually no more than 2 requests to sprinkles, one of them with a timestamp)

Image

but this branch also introduced these warnings to console, you might be interested

11:35:33 AM [vite] (client) Pre-transform error: Internal server error: Soft-invalidated module "SOME_REACT_COMPONENT.tsx" should not have existing transform result
11:35:34 AM [vite] (client) Pre-transform error: Internal server error: Soft-invalidated module "SOME_VANILLA_EXTRACT_STYLESHEET.css.ts" should not have existing transform result
11:35:34 AM [vite] (client) Pre-transform error: Internal server error: Soft-invalidated module "SOME_PLAIN_TYPESCRIPT_FILE.ts" should not have existing transform result

it's absent in main branch

happy to test possible new fixes

UPD have tested HMR of a very deeply nested sprinkle and seems like situation is different (and better): in main branch I had to actually restart dev server to see any changes in this sprinkle, in this branch it's HMRed but multiple requests you see on the screenshot above are happening all the time, on each change

kompot avatar Jul 06 '25 08:07 kompot

@kompot Thanks for the feedback, this is very useful. Maybe not invalidating the first time a module is transformed will improve cold start? I'll have to test that out.

Those soft-invalidation errors are strange - I wouldn't have expected them to show up as a result of the fix.

Would something like 100 identical react components all importing the same sprinkles file be a good enough reproduction of your situation?

askoufis avatar Jul 06 '25 10:07 askoufis

Maybe not invalidating the first time a module is transformed will improve cold start? I'll have to test that out

not sure how the whole mechanism actually works but maybe last update time should be tracked and if multiple modules have requested the same vanilla extract module then they should wait for the same, only once created, transformation promise

judging only by my screenshot I can guess that t query string parameter means time the module was requested

Would something like 100 identical react components all importing the same sprinkles file be a good enough reproduction of your situation?

well, looks like it might be enough

not sure what causes this request hell though, I mean maybe the fact that our sprinkles are pretty complex and are transformed kinda slow that leads to the next component requesting raw sprinkles file instead of already transformed result (again, no idea how the internals work, just guessing)

considering that it's all plain typescript you could probably add artifical delays in css.ts file itself to test slow transformations?

kompot avatar Jul 06 '25 10:07 kompot

@askoufis I have run vite with DEBUG=vite:* env param and observe that vanilla extract takes a lot time indeed and transforms each file at least twice.

vite:transform 1582.12ms /src/default/components/LicenseMsg/LicenseMsg.instance.css.ts +1ms
  vite:load 0.19ms [plugin] /src/default/components/LicenseMsg/LicenseMsg.instance.css.ts.vanilla.css +3ms
  vite:hmr [self-accepts] src/default/components/LicenseMsg/LicenseMsg.instance.css.ts.vanilla.css +3ms
  vite:import-analysis 0.04ms [0 imports rewritten] src/default/components/LicenseMsg/LicenseMsg.instance.css.ts.vanilla.css +0ms
  vite:transform 0.12ms /src/default/components/LicenseMsg/LicenseMsg.instance.css.ts.vanilla.css +0ms
  vite:import-analysis 0.72ms [6 imports rewritten] src/default/components/LicenseMsg/LicenseMsg.instance.css.ts +0ms
  vite:transform 1578.77ms /src/default/components/LicenseMsg/LicenseMsg.instance.css.ts +1ms



  vite:load 0.61ms [fs] src/default/components/limitsAndFees/LimitsAndFeesList.instance.css.ts +1ms
  vite:import-analysis 1.04ms [5 imports rewritten] src/default/components/limitsAndFees/LimitsAndFeesList.instance.css.ts +3ms
  vite:transform 1.96ms src/default/components/limitsAndFees/LimitsAndFeesList.instance.css.ts +2ms
  vite:import-analysis 0.53ms [3 imports rewritten] src/default/components/limitsAndFees/LimitsAndFeesList.instance.css.ts +10ms
  vite:transform 1593.97ms /src/default/components/limitsAndFees/LimitsAndFeesList.instance.css.ts +10ms
  vite:load 0.42ms [plugin] /src/default/styling/rules/layout.css.ts.vanilla.css +12ms
  vite:load 0.43ms [plugin] /src/default/styling/sprinkles.css.ts.vanilla.css +0ms
  vite:load 0.49ms [plugin] /src/default/components/limitsAndFees/LimitsAndFeesList.instance.css.ts.vanilla.css +0ms
  vite:import-analysis 0.96ms [19 imports rewritten] src/default/components/limitsAndFees/LimitsAndFeesList.instance.css.ts +0ms
  vite:transform 1591.28ms /src/default/components/limitsAndFees/LimitsAndFeesList.instance.css.ts +0ms

as you see it tried to transform LimitsAndFeesList.instance.css.ts and LicenseMsg.instance.css.ts twice spending 1500ms each time and that is on MacBook Pro M2 Pro, not a particularly slow machine. Also these lines are just one after another (not different recompilations).

kompot avatar Jul 08 '25 00:07 kompot

When will a fix for this issue be merged?

christhegrand avatar Jul 23 '25 18:07 christhegrand

@christhegrand @kompot @hartz89 @oliveryasuna @SamBernaerdtOntoforce Please try out this snapshot version: @vanilla-extract/[email protected]. In my testing it looks like it fixes the duplicate loading issue, and reduces (potentially eliminates) unnecessary HMR invalidations.

askoufis avatar Aug 27 '25 06:08 askoufis

Confirming 0 extra requests on cold start (after rm -rf ./node_modules/.vite). Also no ts in querystring.

before (with 5.1.1)

Image

after

Image

But observed behavior was broken unfortunately.

With 5.1.1 if I change something deeply nested I get full page reload and changes are applied (with tens of requests to sprinkles.css.ts?ts=XXX). With 5.1.2-vite-hmr-fix-20250826075423 I also get full page reload but nothing changes.

kompot avatar Aug 27 '25 07:08 kompot

@kompot Ah, I thought I'd tested the deeply nested use case. I guess it doesn't work. Would it be possible for you to make a minimal reproduction of your specific situation? That would help a lot.

askoufis avatar Aug 27 '25 09:08 askoufis

@askoufis Here's minimal repo to reproduce https://github.com/kompot/vanilla-extract-vite-plugin-hmr-testcase

pnpm install
pnpm start

It will start with 5.1.2-vite-hmr-fix-20250826075423. Open root page and you'll see 3 headers.

Image

updating this border-radius should HMR red's rectangular border radius https://github.com/kompot/vanilla-extract-vite-plugin-hmr-testcase/blob/51ce9a4acbccacda28a8c95ab0eabd9799ae54a4/src/styling/rules/border.css.ts#L3

but it does not (hard reload does not help)

with 5.1.1 everything works as it should.

kompot avatar Aug 27 '25 16:08 kompot

@kompot Thank you for that. It has helped me narrow down the issue to a tricky edge case. I'll report back if I come up with a solution.

askoufis avatar Aug 31 '25 23:08 askoufis

This was really slowing down the Vite dev server in our case, but can also confirm the @vanilla-extract/[email protected] snapshot resolves this for us.

ramiroazar avatar Sep 19 '25 01:09 ramiroazar

@ramiroazar @vanilla-extract/[email protected] is a more recent snapshot that fixes some invalidation edge cases.

@kompot The newer snapshot almost completely eliminates unnecessary invalidations in your test repo, however for some reason App.tsx is invalidated twice. However, it's better than the current state, so I might make a PR for it regardless.

askoufis avatar Sep 19 '25 01:09 askoufis

Thanks, @askoufis, @vanilla-extract/[email protected] is working great,

I agree that this is not a Storybook specific issue by the way.

ramiroazar avatar Sep 19 '25 01:09 ramiroazar

FWIW I've been trying this out for several days now and have no longer been experiencing any issues. Thanks for publishing this snapshot version.

ryanone avatar Oct 28 '25 16:10 ryanone

@christhegrand @kompot @hartz89 @oliveryasuna @SamBernaerdtOntoforce Please try out this snapshot version: @vanilla-extract/[email protected]. In my testing it looks like it fixes the duplicate loading issue, and reduces (potentially eliminates) unnecessary HMR invalidations.

Apologies for the late reply.

I've just updated to this snapshot and it looks like this resolved the issue. Thanks a lot for the fix!

sambernaerdtphysibel avatar Nov 17 '25 15:11 sambernaerdtphysibel

@askoufis I just gave @vanilla-extract/[email protected] a spin and it seems to be working great, vastly improved the initial page load and HMR on my vite apps!

If there's any hesitation around the stability of the change, would it make sense to control the behavior with a configuration option / "future flag" for the initial release? Would be happy to use it in dev for a few months to sus out any potential corner cases, but I can't really do that with a snapshot version.

hartz89 avatar Nov 24 '25 22:11 hartz89

5.1.2-vite-hmr-fix-20250911065547 improves things, still not perfect, but i see it did not make 5.1.3, will this get an official patch?

sparta-dan avatar Nov 26 '25 13:11 sparta-dan

5.1.2-vite-hmr-fix-20250911065547 improves things, still not perfect, but i see it did not make 5.1.3, will this get an official patch?

@sparta-dan What behaviour are you seeing that you don't expect? Any examples/reproductions would be extremely helpful so I find more edge cases.

askoufis avatar Nov 26 '25 22:11 askoufis

@askoufis we are on a vite app in an NX monorepo. we are having multiple requests for each css.ts file too. dozens per file in the monorepo, causing our page reload time to be ~40s+. the 5.1.2-vite-hmr-fix-20250911065547 version reduces the duplicates greatly, and gets us to 2-3s page reloads. It still has some duplication, but it is a great improvement. 5.1.3 did not include the hotfix, and still causes us issues.

We are using this hotfix version for our SaaS but would like to use a formal version. One of my colleagues is digging into the issue and hotfix to further optimise and avoid any duplication... hopefully we will have a PR for you to help the cause.

Many thanks,

Image

sparta-dan avatar Dec 01 '25 13:12 sparta-dan