Support React Server Components in Vite plugin and fix HMR #751

frandiox · 2022-07-14T08:20:35Z

Hi! We are trying to support vanilla-extract in @shopify/hydrogen but we've found that the Vite plugin in this repo is not compatible with React Server Components.

I'm adding here some minimal changes that enable RSC support with the plugin we are making here. Forking your plugin is also possible, if for any reason you don't want to support RSC yet.

The first commit is, to my understanding, a bug fix. Vite uses sometimes ?v=42-like query strings, and I think the regex checked in 0bc2b9f should pass regardless.
The second commit adds an environment variable that is truthy when we do an RSC build. The main problem is that Vite doesn't have a way to differentiate between pure SSR build vs RSC build. That's why for now we need to give it a manual hint. With this variable, we are able to collect styles that are imported in server components.
The third commit should fix HMR does not work with @vanilla-extract/vite-plugin": "^3.2.1 but works in 2.1.3. #735.

Alternatively, I think it's also possible to use a plugin option instead of an env variable but we'd need to add public interface and some extra code.

cc @benjaminsehl @juanpprieto

changeset-bot · 2022-07-14T08:20:39Z

🦋 Changeset detected

Latest commit: 9bb1661

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@vanilla-extract/vite-plugin	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

juanpprieto · 2022-07-19T02:12:19Z

Confirming that the updated plugin fixes HMR on this starter

benjaminsehl · 2022-07-22T02:03:12Z

cc: @markdalgleish — could I get you to take a look at this? 🙏🙏🙏

markdalgleish · 2022-07-23T01:47:09Z

the Vite plugin in this repo is not compatible with React Server Components.

Could you go into a bit more detail on this? How exactly does this plugin fit into an RSC setup? What is the current behaviour compared to your expectations?

Alternatively, I think it's also possible to use a plugin option instead of an env variable but we'd need to add public interface and some extra code.

This would be my preference, and ideally it's an option describing the behaviour of the plugin rather than referencing RSC specifically. The answer to my previous question would help inform this.

frandiox · 2022-07-26T11:47:26Z

@markdalgleish Thanks for checking this!

Could you go into a bit more detail on this? How exactly does this plugin fit into an RSC setup? What is the current behaviour compared to your expectations?

To my understanding, in a traditional SSR setup you have client build + server/ssr build, and both resulting bundles are relatively similar because they include the same React components (one to do SSR, the other one to hydrate in the browser).

In RSC, however, these two builds are quite different: the client bundle only includes "client components", which come from files that are supposed to run in the browser. On the other hand, the server bundle also includes "server components" which are never leaked to the browser.

Our use-case is that we want to support importing vanilla-extract *.css.ts files in server components. Since these components are never discovered by the client build, their corresponding CSS is never generated. In the server build, the plugin logic assumes the CSS has already been generated by the client build and instead it goes to the addFileScope path.

This behavior works in traditional SSR builds. However, in RSC, Vite still enables the SSR flag but what we actually need is to generate the CSS instead of the JS version with addFileScope. Therefore, I'm trying to add here a way to signal the plugin to force emit CSS even on SSR builds.

This would be my preference, and ideally it's an option describing the behaviour of the plugin rather than referencing RSC specifically. The answer to my previous question would help inform this.

I originally went with an environment variable because RSC is still experimental and things might change. This env var is added by the RSC Vite plugin to make other plugins aware of RSC builds.

However, we can instead add a public parameter forceEmitCssInSsrBuild = false or similar, and change the line to if (ssr && (!forceEmitCssInSsrBuild || !config.build?.ssr)) {...}. This way, we force generating CSS in SSR builds and keep the original behavior during dev and client builds.

markdalgleish · 2022-08-02T21:36:55Z

Thanks for the detailed response!

Am I right in saying that your plugin also collects all styles generated across every build and then combines them into a single CSS file? If so, this makes me think that a public option on our Vite plugin like forceEmitCssInSsrBuild probably wouldn't be useful to anyone else because all it would do is generate duplicate styles. As a standalone setting on our plugin it wouldn't seem to make much sense without referencing your plugin anyway.

I feel like I've come around to the idea that this is handled behind the scenes so consumers can use our Vite plugins together and it just works, without having to set some weird flag in their config that nobody else in the world would ever want to use. If this approach takes off, it also means other plugins can silently hook into it to which would be nice. This is a potentially controversial call though, so I'll need to double check with the rest of the team before committing to this.

By the way, one thing I'm wondering is whether the environment variable from your plugin should be namespaced a bit more? e.g. VITE_RSC_BUILD?

frandiox · 2022-08-03T08:31:18Z

@markdalgleish Thanks!

Am I right in saying that your plugin also collects all styles generated across every build and then combines them into a single CSS file?

That's fairly accurate, yes. We currently ignore the client build CSS because it is a subset of what's generated in the RSC build (which includes both client and server components). Then, we emit a single CSS file asset when the RSC build is done.

I feel like I've come around to the idea that this is handled behind the scenes so consumers can use our Vite plugins together and it just works, without having to set some weird flag in their config that nobody else in the world would ever want to use. If this approach takes off, it also means other plugins can silently hook into it to which would be nice.

Right, there's currently no easy way to allow communication across Vite plugins so the env variable seems to be the simplest solution. Plus, since things might still change, keeping it private and undocumented might be the right call.

This is a potentially controversial call though, so I'll need to double check with the rest of the team before committing to this.

No problem, let us know!

By the way, one thing I'm wondering is whether the environment variable from your plugin should be namespaced a bit more? e.g. VITE_RSC_BUILD?

Prefixing it with VITE_ makes total sense, I'll update the RSC plugin and this PR 👍

mattcompiles · 2022-08-04T22:25:55Z

packages/vite-plugin/src/index.ts

@@ -183,10 +186,16 @@ export function vanillaExtractPlugin({
            cssMap.get(absoluteId) !== source
          ) {
            const { moduleGraph } = server;
-            const module = moduleGraph.getModuleById(absoluteId);
+            const [module] = Array.from(
+              moduleGraph.getModulesByFile(absoluteId) || [],


Not saying this is wrong, just for my own knowledge would like to understand this change more?

Assuming the absoluteId doesn't match up with the vite id but does match a file?

Assuming the absoluteId doesn't match up with the vite id but does match a file?

That's right, according to my tests. A "module id" consists of a file path plus an optional query string that Vite adds for HMR or for other reasons. However, our absoluteId variable here is always created without a query string, so it sometimes doesn't match the module id. It always matches the file path, though.

markdalgleish · 2022-08-04T22:37:50Z

.changeset/soft-lobsters-promise.md

+'@vanilla-extract/vite-plugin': patch
+---
+
+Fix HMR and support React Server Components.


Could you please split this into two changesets?

patch release: Fix HMR

minor release: Add [experimental support for React Server Components](https://github.com/facebook/react/pull/22952)

Done 👍
If you want I can split the PRs as well (extract the env variable in a new PR).
Originally, I did it in one PR because at first I thought the HMR was only broken for RSC 😅

As long as it's two changesets, I'm happy 😊

juanpprieto · 2022-08-17T04:19:12Z

Thanks team. Looking forward to putting together an hydrogen example very soon! 🙏🏼

frandiox added 2 commits July 14, 2022 16:46

fix(vite-plugin): Ignore query-string when checking regexp

0bc2b9f

feat(vite-plugin): Support React Server Components build

13718d3

frandiox mentioned this pull request Jul 14, 2022

Improve CSS support in server components Shopify/hydrogen-v1#1843

Merged

4 tasks

frandiox added 2 commits July 16, 2022 15:54

fix(vite-plugin): HMR not adding proper query strings

05e2974

Changeset

544ecde

frandiox changed the title ~~Support React Server Components in Vite plugin~~ Support React Server Components in Vite plugin and fix HMR Jul 16, 2022

frandiox mentioned this pull request Jul 19, 2022

HMR does not work with @vanilla-extract/vite-plugin": "^3.2.1 but works in 2.1.3. #735

Closed

2 tasks

feat(vite-plugin): Update RSC environment variable

ee57d8e

frandiox requested a review from a team August 3, 2022 08:40

mattcompiles reviewed Aug 4, 2022

View reviewed changes

markdalgleish reviewed Aug 4, 2022

View reviewed changes

frandiox and others added 2 commits August 5, 2022 09:50

Split changesets

2f0a87b

Merge branch 'master' into fd-vite-rsc

9bb1661

mattcompiles approved these changes Aug 5, 2022

View reviewed changes

mattcompiles merged commit 8c4f71b into vanilla-extract-css:master Aug 5, 2022

seek-oss-ci mentioned this pull request Aug 5, 2022

Version Packages #773

Merged

mechairoi mentioned this pull request Aug 25, 2022

vite: Add experimental support for Astro #796

Merged

askoufis mentioned this pull request Dec 6, 2022

Incorrect classnames during SSR #937

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support React Server Components in Vite plugin and fix HMR #751

Support React Server Components in Vite plugin and fix HMR #751

frandiox commented Jul 14, 2022 •

edited

Loading

changeset-bot bot commented Jul 14, 2022 •

edited

Loading

juanpprieto commented Jul 19, 2022

benjaminsehl commented Jul 22, 2022

markdalgleish commented Jul 23, 2022 •

edited

Loading

frandiox commented Jul 26, 2022

markdalgleish commented Aug 2, 2022

frandiox commented Aug 3, 2022

mattcompiles Aug 4, 2022

frandiox Aug 5, 2022

markdalgleish Aug 4, 2022

frandiox Aug 5, 2022

markdalgleish Aug 5, 2022

juanpprieto commented Aug 17, 2022 •

edited

Loading

Support React Server Components in Vite plugin and fix HMR #751

Support React Server Components in Vite plugin and fix HMR #751

Conversation

frandiox commented Jul 14, 2022 • edited Loading

changeset-bot bot commented Jul 14, 2022 • edited Loading

🦋 Changeset detected

juanpprieto commented Jul 19, 2022

benjaminsehl commented Jul 22, 2022

markdalgleish commented Jul 23, 2022 • edited Loading

frandiox commented Jul 26, 2022

markdalgleish commented Aug 2, 2022

frandiox commented Aug 3, 2022

mattcompiles Aug 4, 2022

Choose a reason for hiding this comment

frandiox Aug 5, 2022

Choose a reason for hiding this comment

markdalgleish Aug 4, 2022

Choose a reason for hiding this comment

frandiox Aug 5, 2022

Choose a reason for hiding this comment

markdalgleish Aug 5, 2022

Choose a reason for hiding this comment

juanpprieto commented Aug 17, 2022 • edited Loading

frandiox commented Jul 14, 2022 •

edited

Loading

changeset-bot bot commented Jul 14, 2022 •

edited

Loading

markdalgleish commented Jul 23, 2022 •

edited

Loading

juanpprieto commented Aug 17, 2022 •

edited

Loading