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

Ensure deterministic SSR builds in @tailwindcss/vite #13457

Merged
merged 8 commits into from
Apr 8, 2024

Conversation

RobinMalfait
Copy link
Member

This PR fixes an issue where running astro build doesn't generate the correct CSS when using @tailwindcss/vite. This issue only seems to happen when running a build, and not when running in dev mode.

To fix this, we apply the same code (waitForRequestsIdle) as we do in the apply: "server" step.

Adding this to the apply: "build" step as well does generate the correct result.

While this does fix the issue, I'm not 100% convinced that this is the correct solution /cc @thecrypticace.

Fixes: #13441

It looks like when running `astro build` we only run this `build` step
and not the `dev` step where we already use the `waitForRequestsIdle`
code.

Adding this to the `build` part as well does generate the correct
result.
@thecrypticace
Copy link
Contributor

I don't think this is the right fix.

  • I'm not certain if waitForRequestsIdle is valid to call during a normal build — rather than just for the dev server.
  • As I understand it, we have to use renderChunk during the build because that's the only way we'll have the guarantee that all files have already been seen.

Even if waitForRequestsIdle() works during a normal build it won't during an SSR build which means we'd be back to needing to use renderChunk for SSR builds and we'd be left with similar problems.

Our setup works in a normal Vite project, and even a basic Astro project. The problem in the project in the issue is caused by a few things:

  1. Importing and using the Image component from astro:assets also breaks
  2. An Astro component containing import.meta.glob(…) in the front matter block causes our implementation renderChunk to not work. Dynamic imports in general seem to have this issue.
  3. However, an eager glob also causes renderChunk to break.

I'm not sure yet if this is a problem specific to Astro or is in Vite itself. I need to do some more investigating.

@Brian-Pob
Copy link

Hi. I'm the person who filed the issue. I did a bit more testing with my project and removed the ProjectCard.astro component that uses astro's Image and the usage of import.meta.glob(...).

The issue still seems to persist when using the latest version of @tailwindcss/vite.

The screenshot below shows that I commented out the component usage but I also completely deleted the file.

https://github.com/Brian-Pob/tailwind-v4-vite/tree/alpha-11-bug-2

CleanShot 2024-04-05 at 13 34 49

@Brian-Pob
Copy link

And please feel free to reach out to me in the Tailwind Discord if you would like me to try anything or run any tests.

@thecrypticace
Copy link
Contributor

@Brian-Pob Importing a Svelte component also seems to do it.

Even then, it appears that when a component statically imports images, the build's CSS is non-deterministic. Sometimes our plugin appears to run at the appropriate time and sometimes it does not.

fwiw, I stress-tested this over ~500 runs and this did not happen for a vanilla Vite installation. Only when astro components are involved.

@thecrypticace
Copy link
Contributor

Notes for me later:

I stripped down the astro build to just enable the following plugins:

  • @astro/plugin-middleware
  • @astrojs/vite-plugin-astro-ssr-manifest
  • astro:build

My guess is that astro:build is possibly doing something with the module graph that is causing this non-determinism possibly because our renderChunk method re-runs the vite:css and vite:css-post plugins on our output. I'm not certain of this but it would be the next thing to test.

@thecrypticace thecrypticace marked this pull request as draft April 6, 2024 00:55
@thecrypticace
Copy link
Contributor

Something interesting I've discovered in my stripped down test is that anytime transform() for an image get's called AFTER the transform() for the CSS containing @tailwind the output is broken.

Which means if we delay resolution to make sure the CSS file is transformed after all images things start working. In my stripped down reproduction a delay of ~100ms is sufficient. In the original reproduction that was provided the delay has to be a good bit longer.

So, in the world of "how and why does this work", this plugin solves the issue:

{
  name: "delay-the-css",
  enforce: "pre",

  async resolveId(id) {
    if (!id.includes(".css")) return;
    await new Promise((resolve) => setTimeout(resolve, 3000));
  },
},

I'm still investigating the root of the problem. I also tested this in a Vite + Vue project and could not reproduce the issue. Even when manually delaying images such that they were transformed after CSS files.

@thecrypticace thecrypticace marked this pull request as ready for review April 8, 2024 20:23
@thecrypticace
Copy link
Contributor

Okay @RobinMalfait I figured this one out. During SSR there is a server with a module graph (when normally there isn't during a build) and we're deleting references to modules that don't exist. Normally this is fine but during an SSR build the module won't get reloaded unless its transform is handled after everything else.

This is what's causing the problem — and the "after everything else" is the cause of the non-deterministic behavior. With this the problem should be resolved entirely.

@thecrypticace thecrypticace changed the title Ensure @tailwindcss/vite#build waits for all files before generating the CSS Ensure deterministic SSR builds in @tailwindcss/vite Apr 8, 2024
Copy link
Member Author

@RobinMalfait RobinMalfait left a comment

Choose a reason for hiding this comment

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

Makes sense what you did here @thecrypticace, nice debugging job on the SSR stuff!

packages/@tailwindcss-vite/src/index.ts Show resolved Hide resolved
packages/@tailwindcss-vite/src/index.ts Show resolved Hide resolved
@thecrypticace thecrypticace merged commit 4a25bc1 into next Apr 8, 2024
1 check passed
@thecrypticace thecrypticace deleted the fix/issue-13441 branch April 8, 2024 22:10
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.

[v4] Styles not being generated on project build
3 participants