-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
FOUC when using custom servers #7878
Comments
Ok another option that I just tried out that works - splitting the configureServer middleware into two parts. Part one includes all the non request handling logic, and is always run. Part two actually handles the request, and is only run if user is not providing their own server. Critical bits that the end user needs for their own custom server can be exposed on the req (or could expose a type safe WeakMap end user can use, that is keyed to requests). Something along these lines: type ViteReqCtx = {
build: ServerBuild;
criticalCss: string | undefined;
}
const viteReqCtx = new WeakMap<IncomingMessage, ViteReqCtx>();
// For end user to use in their app
export const getViteReqCtx = (req: IncomingMessage) => ViteReqCtx;
const remixVitePlugin = {
// ...
configureServer(vite) {
vite.httpServer?.on("listening", () => {
setTimeout(showUnstableWarning, 50);
});
return () => {
vite.middlewares.use(async (req, res, next) => {
try {
// Invalidate all virtual modules
vmods.forEach((vmod) => {
let mod = vite.moduleGraph.getModuleById(
VirtualModule.resolve(vmod)
);
if (mod) {
vite.moduleGraph.invalidateModule(mod);
}
});
let { url } = req;
let [pluginConfig, build] = await Promise.all([
resolvePluginConfig(),
vite.ssrLoadModule(serverEntryId) as Promise<ServerBuild>,
]);
let criticalCss = await getStylesForUrl(
vite,
pluginConfig,
cssModulesManifest,
build,
url
);
viteReqCtx.set(req, {
build,
criticalCss,
});
next();
} catch (error) {
next(error);
}
});
// Let user servers handle SSR requests in middleware mode
if (vite.config.server.middlewareMode) return;
vite.middlewares.use(async (req, res, next) => {
let { build, criticalCss } = viteReqCtx.get(req)!;
try {
let handle = createRequestHandler(build, {
mode: "development",
criticalCss,
});
await handle(req, res);
} catch (error) {
next(error);
}
});
};
}
} Then in the end user's custom server, they can get access to the critical css for the request, to pass into the remix request handlers. Although I noticed that at least the express adapter does not expose the criticalCss option (the node one does). OR we could make this an adapter concern, pop these props on to the Or to make it more runtime agnostic... could key the vite ctx by req url? |
On second thought - might make more sense for the adapters to handle adding the critical css, since they already do that for prod? Could still accomplish this by popping the criticalCss info on the request context - the two step middleware described above is still relevant. Otherwise it's just more dev specific logic that the end user has to manage in their custom server. |
https://remix.run/docs/en/main/future/vite#fix-up-css-imports They explicitly said they solved this issue in the Remix plugin for Vite. However, I can reproduce the issue 🤔 |
@markdalgleish no problem! Ok so I see you went with the two step middleware + popping critical css on the req (or, res.locals in your pr) ctx for use in adapters approach. This works for express apps - any reason we couldn't get it to work more generically and pop the info somewhere on the Reason I'm asking is because I'm actually using Hono. But this would be relevant to cloudflare, deno, etc - anything that operates on web standard req/res. Vite's a little awkward to use with these frameworks since it expects connect middleware, but it's not too difficult to plug connect middleware into things like Hono, which is what I'm doing during development. Can share a more concrete example if that would be helpful. Thanks for getting things going w that PR! |
@marbemac My PR was just a short term solution since it means our Vite Express template is fixed without introducing any new public APIs. In terms of a more general solution, if you can share a non-Express repo that would be super helpful to get me started on it quickly :) |
Makes sense, it's a good first step :). I'm booked up today but will try and put together a basic playground repo to frame up the issue early next week. |
🤖 Hello there, We just published version Thanks! |
🤖 Hello there, We just published version Thanks! |
Just re-opening this to track non-Express custom servers since the fix that went out was a more targeted short term fix for Express. |
Currently facing that issue, I’m using Fastify as custom server 👀 |
if you’re using |
Tried that but getting this error 👀 |
@mcansh the problem was because I was using |
Awesome work @hi-ogawa and @markdalgleish! Looks great, love the approach. Only snag I ran into was to remember to explicitely set the 👌 💯 |
What version of Remix are you using?
2.2.0
Are all your remix dependencies & dev-dependencies using the same version?
Steps to Reproduce
npx create-remix@latest --template remix-run/remix/templates/unstable-vite-express
Expected Behavior
No flash of unstyled content when using custom servers.
Actual Behavior
There is a flash of unstyled content when using custom servers.
This might be tricky to solve - the long standing approach to SSR w vite (the approach described in Vite's docs) is IMHO pretty cumbersome, would be amazing if remix could somehow smooth over the custom server + vite experience.
But that's a bit off topic, regarding this specific problem - one option is to somehow expose
getStylesForUrl
to the end user so that they can pass in the critical styles to their request handler similar to how the remix vite plugin does on request. The problem is thatgetStylesForUrl
requirescssModulesManifest
, which is scoped to theremixVitePlugin
plugin.The larger issue is that longer term, with the current custom server approach, any logic/features that remix's vite plugin covers in the
configureServer
middleware will need to be replicated by the end user in their custom server.I'd be willing to try and tackle a solution here if ya'll have any preferred direction or other ideas!
The text was updated successfully, but these errors were encountered: