Skip to content

Commit 76e9865

Browse files
authored
fix: More HMR fixes for robustness (#598)
## Avoid js transforms for css ### Context To support SSR having its own module resolution behaviour (e.g. not using `react-server` import condition or RSC react runtimes) while still existing in the same runtime env as the rest of the worker (where `react-sever` import condition and RSC react runtimes should apply), we have separate `worker` and `ssr` environments, then have an "ssr bridge" concept: in the `worker` environment, for all modules to be evaluated in SSR context, we fetch code from the `ssr` environment for that module. We parse the code we fetch from the SSR environment to find all imports vite does (`__vite_ssr_import__`, `__vite_ssr_dynamic_import__`), and add `import`s for these fetches so that vite will build the module graph correctly. ### The fix We do this fetching and transforming too for css modules too - except in these cases the result of the fetch is an empty string, and should remain that way rather than getting any other code added to it, in order to avoid adding js to the css. ## Opt out of custom HMR logic for non js and css files If the js files are not js (e.g. js, jsx, mjs, cjs, ts, mts, etc) and not css, we should ignore them and pass control on to other plugins. We were trying to process .sqlite files during HMR, for e.g, which was resulting in HMR of these modules causing miniflare to crash. ## Skip HMR for SSR env when files are not in "use client" import chains HMR for SSR should only look at "use client" files, or files imported by "use client" files. It was processing for other files (e.g. .sqlite files - while we avoid non js/css files now regardless, it still shows the need for more thorough checking for the use client graph for HMR in SSR env). ## Skip HMR for client env when files are not in "use client" import chains It is possible for the client environment to contain files that are neither marked with "use client" nor imported by files that are. For example, if there is a style.css imported in worker module graph. For this specific example, to be honest, I'm not entirely sure why for this example case, I noticed this when inspecting the module graph. Regardless, it shows the need for more thorough checking for whether the module is in fact part of "use client" import chains. For this reason, we short circuit HMR for the client env if we the file is not in a "use client" import chain. ## Avoid recursive invalidating for SSR modules Turns out we only need to invalidate the SSR modules themselves - not their importers as well - for HMR to work correctly for SSR.
1 parent f773e31 commit 76e9865

File tree

2 files changed

+68
-7
lines changed

2 files changed

+68
-7
lines changed

sdk/src/vite/miniflareHMRPlugin.mts

Lines changed: 59 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,40 @@ const hasEntryAsAncestor = (
4545
return false;
4646
};
4747

48+
const isInUseClientGraph = ({
49+
file,
50+
clientFiles,
51+
server,
52+
}: {
53+
file: string;
54+
clientFiles: Set<string>;
55+
server: ViteDevServer;
56+
}) => {
57+
const id = normalizeModulePath(server.config.root, file);
58+
if (clientFiles.has(id)) {
59+
return true;
60+
}
61+
62+
const modules = server.environments.client.moduleGraph.getModulesByFile(file);
63+
64+
if (!modules) {
65+
return false;
66+
}
67+
68+
for (const m of modules) {
69+
for (const importer of m.importers) {
70+
if (
71+
importer.file &&
72+
isInUseClientGraph({ file: importer.file, clientFiles, server })
73+
) {
74+
return true;
75+
}
76+
}
77+
}
78+
79+
return false;
80+
};
81+
4882
export const miniflareHMRPlugin = (givenOptions: {
4983
clientFiles: Set<string>;
5084
serverFiles: Set<string>;
@@ -73,17 +107,28 @@ export const miniflareHMRPlugin = (givenOptions: {
73107
);
74108
}
75109

110+
if (!isJsFile(ctx.file) && !ctx.file.endsWith(".css")) {
111+
return;
112+
}
113+
76114
if (this.environment.name === "ssr") {
77115
log("SSR update, invalidating recursively", ctx.file);
78-
invalidateModule(ctx.server, "ssr", ctx.file, {
79-
invalidateImportersRecursively: true,
116+
const isUseClientUpdate = isInUseClientGraph({
117+
file: ctx.file,
118+
clientFiles,
119+
server: ctx.server,
80120
});
121+
122+
if (!isUseClientUpdate) {
123+
return [];
124+
}
125+
126+
invalidateModule(ctx.server, "ssr", ctx.file);
81127
invalidateModule(
82128
ctx.server,
83129
environment,
84130
VIRTUAL_SSR_PREFIX +
85131
normalizeModulePath(givenOptions.rootDir, ctx.file),
86-
{ invalidateImportersRecursively: true },
87132
);
88133
return [];
89134
}
@@ -152,9 +197,7 @@ export const miniflareHMRPlugin = (givenOptions: {
152197
) ?? [],
153198
);
154199

155-
const isWorkerUpdate =
156-
ctx.file === entry ||
157-
modules.some((module) => hasEntryAsAncestor(module, entry));
200+
const isWorkerUpdate = Boolean(modules);
158201

159202
// The worker needs an update, but this is the client environment
160203
// => Notify for HMR update of any css files imported by in worker, that are also in the client module graph
@@ -179,6 +222,16 @@ export const miniflareHMRPlugin = (givenOptions: {
179222
}
180223
}
181224

225+
const isUseClientUpdate = isInUseClientGraph({
226+
file: ctx.file,
227+
clientFiles,
228+
server: ctx.server,
229+
});
230+
231+
if (!isUseClientUpdate && !ctx.file.endsWith(".css")) {
232+
return [];
233+
}
234+
182235
return ctx.modules;
183236
}
184237

sdk/src/vite/ssrBridgePlugin.mts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import type { Plugin, ViteDevServer } from "vite";
22
import debug from "debug";
33
import { SSR_BRIDGE_PATH } from "../lib/constants.mjs";
44
import { findSsrImportSpecifiers } from "./findSsrSpecifiers.mjs";
5+
import { isJsFile } from "./isJsFile.mjs";
56

67
const log = debug("rwsdk:vite:ssr-bridge-plugin");
78

@@ -152,6 +153,13 @@ export const ssrBridgePlugin = ({
152153
log("Fetch module result: id=%s, result=%O", realId, result);
153154

154155
const code = "code" in result ? result.code : undefined;
156+
157+
if (realId.endsWith(".css")) {
158+
process.env.VERBOSE &&
159+
log("Not a JS file, returning code: %s", code);
160+
return code ?? "";
161+
}
162+
155163
log("Fetched SSR module code length: %d", code?.length || 0);
156164

157165
const { imports, dynamicImports } = findSsrImportSpecifiers(
@@ -169,7 +177,7 @@ export const ssrBridgePlugin = ({
169177
const switchCases = allSpecifiers
170178
.map(
171179
(specifier) =>
172-
` case "${specifier}": import("${VIRTUAL_SSR_PREFIX}${specifier}");`,
180+
` case "${specifier}": void import("${VIRTUAL_SSR_PREFIX}${specifier}");`,
173181
)
174182
.join("\n");
175183

0 commit comments

Comments
 (0)