From 3027b828a8e4e81941fe5d71ee25e3959de5843d Mon Sep 17 00:00:00 2001 From: Valerio Ageno <51341197+Valerioageno@users.noreply.github.com> Date: Sun, 11 May 2025 19:11:30 +0200 Subject: [PATCH] fix: improve critical css generation (#770) --- .../src/fs-routing/generator.ts | 1 + .../tuono-react-vite-plugin/src/styles.ts | 160 ++++++++++-------- .../generator/catch_all/routeTree.expected.ts | 2 + .../tests/generator/mdx/routeTree.expected.ts | 2 + .../routeTree.expected.ts | 6 + .../multi-level/routeTree.expected.ts | 3 + .../single-level/routeTree.expected.ts | 2 + .../src/components/CriticalCss.tsx | 8 +- .../tuono-router/src/components/NotFound.tsx | 4 +- .../src/components/RouteMatch.tsx | 8 +- packages/tuono-router/src/route.ts | 23 ++- 11 files changed, 137 insertions(+), 82 deletions(-) diff --git a/packages/tuono-react-vite-plugin/src/fs-routing/generator.ts b/packages/tuono-react-vite-plugin/src/fs-routing/generator.ts index 636c84e5..95df41d0 100644 --- a/packages/tuono-react-vite-plugin/src/fs-routing/generator.ts +++ b/packages/tuono-react-vite-plugin/src/fs-routing/generator.ts @@ -207,6 +207,7 @@ export async function routeGenerator( rustHandlersNodes.includes(node.path || '') ? 'hasHandler: true' : '', + `filePath: '${node.path || '/'}'`, ] .filter(Boolean) .join(',')} diff --git a/packages/tuono-react-vite-plugin/src/styles.ts b/packages/tuono-react-vite-plugin/src/styles.ts index dbbdabe5..d36cc1fe 100644 --- a/packages/tuono-react-vite-plugin/src/styles.ts +++ b/packages/tuono-react-vite-plugin/src/styles.ts @@ -46,6 +46,94 @@ const isCssUrlWithoutSideEffects = (url: string): boolean => { return false } +const normalizePath = (modulePath: string): string => { + return modulePath.startsWith('node_modules') + ? path.join(process.cwd(), modulePath) + : modulePath +} + +export const getStylesForModule = async ( + viteDevServer: ViteDevServer, + moduleUrl: string, + /** + * All the CSS modules are preloaded and saved in this manifest + */ + cssModulesManifest: Record, +): Promise => { + const styles: Record = {} + const deps: Set = new Set() + + const moduleFilePath = normalizePath(moduleUrl) + try { + let node: ModuleNode | undefined = + await viteDevServer.moduleGraph.getModuleByUrl(moduleFilePath) + + // If the module is only present in the client module graph, the module + // won't have been found on the first request to the server. If so, we + // request the module so it's in the module graph, then try again. + if (!node) { + try { + await viteDevServer.transformRequest(moduleFilePath) + } catch (err) { + console.error(err) + } + + node = await viteDevServer.moduleGraph.getModuleByUrl(moduleFilePath) + } + + if (!node) { + console.error(`Could not resolve module for file: ${moduleFilePath}`) + return + } + await findNodeDependencies(viteDevServer, node, deps) + } catch (error) { + console.error(error) + } + + for (const dep of deps) { + if ( + dep.file && + isCssFile(dep.file) && + !isCssUrlWithoutSideEffects(dep.url) // Ignore styles that resolved as URLs, inline or raw. These shouldn't get injected. + ) { + try { + const css = isCssModulesFile(dep.file) + ? cssModulesManifest[dep.file] + : (( + await viteDevServer.ssrLoadModule( + // We need the ?inline query in Vite v6 when loading CSS in SSR + // since it does not expose the default export for CSS in a + // server environment. + injectQuery(normalizePath(dep.file), 'inline'), + ) + ).default as string) + + if (css === undefined) { + throw new Error() + } + + styles[dep.url] = css + } catch { + // this can happen with dynamically imported modules + console.warn(`Could not load ${dep.file}`) + } + } + } + + return ( + Object.entries(styles) + .map(([fileName, css]) => [ + `\n/* ${fileName + // Escape comment syntax in file paths + .replace(/\/\*/g, '/\\*') + .replace(/\*\//g, '*\\/')} */`, + css, + ]) + .flat() + .join('\n') || undefined + ) +} + /** * This function transform the componentId into a file path. * File extension is not required for the vite.moduleGraph URL search. @@ -80,77 +168,7 @@ export const getStylesForComponentId = async ( const fileUrl = path.join(process.cwd(), relativeFilePath) - const styles: Record = {} - const deps: Set = new Set() - - try { - let node: ModuleNode | undefined = - await viteDevServer.moduleGraph.getModuleByUrl(fileUrl) - - // If the module is only present in the client module graph, the module - // won't have been found on the first request to the server. If so, we - // request the module so it's in the module graph, then try again. - if (!node) { - try { - await viteDevServer.transformRequest(fileUrl) - } catch (err) { - console.error(err) - } - - node = await viteDevServer.moduleGraph.getModuleByUrl(fileUrl) - } - - if (!node) { - console.error(`Could not resolve module for file: ${fileUrl}`) - return - } - await findNodeDependencies(viteDevServer, node, deps) - } catch (error) { - console.error(error) - } - - for (const dep of deps) { - if ( - dep.file && - isCssFile(dep.file) && - !isCssUrlWithoutSideEffects(dep.url) // Ignore styles that resolved as URLs, inline or raw. These shouldn't get injected. - ) { - try { - const css = isCssModulesFile(dep.file) - ? cssModulesManifest[dep.file] - : (( - await viteDevServer.ssrLoadModule( - // We need the ?inline query in Vite v6 when loading CSS in SSR - // since it does not expose the default export for CSS in a - // server environment. - injectQuery(dep.url, 'inline'), - ) - ).default as string) - - if (css === undefined) { - throw new Error() - } - - styles[dep.url] = css - } catch { - // this can happen with dynamically imported modules - console.warn(`Could not load ${dep.file}`) - } - } - } - - return ( - Object.entries(styles) - .map(([fileName, css]) => [ - `\n/* ${fileName - // Escape comment syntax in file paths - .replace(/\/\*/g, '/\\*') - .replace(/\*\//g, '*\\/')} */`, - css, - ]) - .flat() - .join('\n') || undefined - ) + return await getStylesForModule(viteDevServer, fileUrl, cssModulesManifest) } /** diff --git a/packages/tuono-react-vite-plugin/tests/generator/catch_all/routeTree.expected.ts b/packages/tuono-react-vite-plugin/tests/generator/catch_all/routeTree.expected.ts index 29cb3294..b8356ee5 100644 --- a/packages/tuono-react-vite-plugin/tests/generator/catch_all/routeTree.expected.ts +++ b/packages/tuono-react-vite-plugin/tests/generator/catch_all/routeTree.expected.ts @@ -21,12 +21,14 @@ const Postscatchall = createRoute({ component: PostscatchallImport }) const IndexRoute = Index.update({ path: '/', getParentRoute: () => rootRoute, + filePath: '/', }) const PostscatchallRoute = Postscatchall.update({ path: '/posts/[...catch_all]', getParentRoute: () => rootRoute, hasHandler: true, + filePath: '/posts/[...catch_all]', }) // Create and export the route tree diff --git a/packages/tuono-react-vite-plugin/tests/generator/mdx/routeTree.expected.ts b/packages/tuono-react-vite-plugin/tests/generator/mdx/routeTree.expected.ts index e4741257..a71becda 100644 --- a/packages/tuono-react-vite-plugin/tests/generator/mdx/routeTree.expected.ts +++ b/packages/tuono-react-vite-plugin/tests/generator/mdx/routeTree.expected.ts @@ -21,11 +21,13 @@ const Index = createRoute({ component: IndexImport }) const AboutRoute = About.update({ path: '/about', getParentRoute: () => rootRoute, + filePath: '/about', }) const IndexRoute = Index.update({ path: '/', getParentRoute: () => rootRoute, + filePath: '/', }) // Create and export the route tree diff --git a/packages/tuono-react-vite-plugin/tests/generator/multi-level-root-dynamic/routeTree.expected.ts b/packages/tuono-react-vite-plugin/tests/generator/multi-level-root-dynamic/routeTree.expected.ts index d686da5e..c12f6349 100644 --- a/packages/tuono-react-vite-plugin/tests/generator/multi-level-root-dynamic/routeTree.expected.ts +++ b/packages/tuono-react-vite-plugin/tests/generator/multi-level-root-dynamic/routeTree.expected.ts @@ -36,31 +36,37 @@ const PostsMyPost = createRoute({ component: PostsMyPostImport }) const PostslayoutRoute = Postslayout.update({ getParentRoute: () => rootRoute, + filePath: '/posts/__layout', }) const AboutRoute = About.update({ path: '/about', getParentRoute: () => rootRoute, + filePath: '/about', }) const IndexRoute = Index.update({ path: '/', getParentRoute: () => rootRoute, + filePath: '/', }) const PostspostRoute = Postspost.update({ path: '/posts/[post]', getParentRoute: () => PostslayoutRoute, + filePath: '/posts/[post]', }) const PostsIndexRoute = PostsIndex.update({ path: '/posts', getParentRoute: () => PostslayoutRoute, + filePath: '/posts/', }) const PostsMyPostRoute = PostsMyPost.update({ path: '/posts/my-post', getParentRoute: () => PostslayoutRoute, + filePath: '/posts/my-post', }) // Create and export the route tree diff --git a/packages/tuono-react-vite-plugin/tests/generator/multi-level/routeTree.expected.ts b/packages/tuono-react-vite-plugin/tests/generator/multi-level/routeTree.expected.ts index 2190ffbb..daeb8087 100644 --- a/packages/tuono-react-vite-plugin/tests/generator/multi-level/routeTree.expected.ts +++ b/packages/tuono-react-vite-plugin/tests/generator/multi-level/routeTree.expected.ts @@ -25,16 +25,19 @@ const PostsMyPost = createRoute({ component: PostsMyPostImport }) const AboutRoute = About.update({ path: '/about', getParentRoute: () => rootRoute, + filePath: '/about', }) const IndexRoute = Index.update({ path: '/', getParentRoute: () => rootRoute, + filePath: '/', }) const PostsMyPostRoute = PostsMyPost.update({ path: '/posts/my-post', getParentRoute: () => rootRoute, + filePath: '/posts/my-post', }) // Create and export the route tree diff --git a/packages/tuono-react-vite-plugin/tests/generator/single-level/routeTree.expected.ts b/packages/tuono-react-vite-plugin/tests/generator/single-level/routeTree.expected.ts index 3db42380..876c1353 100644 --- a/packages/tuono-react-vite-plugin/tests/generator/single-level/routeTree.expected.ts +++ b/packages/tuono-react-vite-plugin/tests/generator/single-level/routeTree.expected.ts @@ -21,11 +21,13 @@ const Index = createRoute({ component: IndexImport }) const AboutRoute = About.update({ path: '/about', getParentRoute: () => rootRoute, + filePath: '/about', }) const IndexRoute = Index.update({ path: '/', getParentRoute: () => rootRoute, + filePath: '/', }) // Create and export the route tree diff --git a/packages/tuono-router/src/components/CriticalCss.tsx b/packages/tuono-router/src/components/CriticalCss.tsx index a0a24cbc..5144ed63 100644 --- a/packages/tuono-router/src/components/CriticalCss.tsx +++ b/packages/tuono-router/src/components/CriticalCss.tsx @@ -6,7 +6,7 @@ const VITE_PROXY_PATH = '/vite-server' const CRITICAL_CSS_PATH = VITE_PROXY_PATH + '/tuono_internal__critical_css' interface CriticalCssProps { - routeId?: string + routeFilePath?: string mode?: Mode } @@ -16,16 +16,16 @@ interface CriticalCssProps { * since vite does not support CSS injection without JS waterfall */ export function CriticalCss({ - routeId, + routeFilePath, mode, }: CriticalCssProps): JSX.Element | null { - if (!routeId || mode !== 'Dev') { + if (!routeFilePath || mode !== 'Dev') { return null } return ( diff --git a/packages/tuono-router/src/components/NotFound.tsx b/packages/tuono-router/src/components/NotFound.tsx index 9eb9bc30..ba78a7b5 100644 --- a/packages/tuono-router/src/components/NotFound.tsx +++ b/packages/tuono-router/src/components/NotFound.tsx @@ -18,7 +18,7 @@ export function NotFound({ mode }: { mode?: Mode }): JSX.Element | null { if (custom404Route) { return ( <> - + ) @@ -30,7 +30,7 @@ export function NotFound({ mode }: { mode?: Mode }): JSX.Element | null { return ( - + ) diff --git a/packages/tuono-router/src/components/RouteMatch.tsx b/packages/tuono-router/src/components/RouteMatch.tsx index b0cf5680..8fee5275 100644 --- a/packages/tuono-router/src/components/RouteMatch.tsx +++ b/packages/tuono-router/src/components/RouteMatch.tsx @@ -42,7 +42,7 @@ export const RouteMatch = ({ mode={mode} > - + @@ -77,9 +77,13 @@ const TraverseRootComponents = memo( const route = routes[index] as Route const Parent = route.component + // Fallback to the route id if the filePath is not defined + // as is the case for the root route + const routeFilePath = route.filePath || route.id + return ( - + Route path?: string + filePath?: string component: RouteComponent hasHandler?: boolean } @@ -19,11 +20,28 @@ export const ROOT_ROUTE_ID = '__root__' export class Route { options: RouteOptions + /** + * The route id is used to identify the route in the router + * and is used to match the route with the URL. + * + * For now is the `path` + */ id?: string isRoot: boolean + /** + * Used for identify the route by matching the URL + */ path?: string fullPath!: string + /** + * Utility to identify the route in the file system + * Used i.e. for finding the criticalCss to load + * + * The path does not include the file extension + */ + filePath?: string + children?: Array parentRoute?: Route originalIndex?: number @@ -70,11 +88,10 @@ export class Route { id = joinPaths(['/', id]) } - const fullPath = id === ROOT_ROUTE_ID ? '/' : path - + this.filePath = this.options.filePath this.path = path this.id = id - this.fullPath = fullPath || '' + this.fullPath = path || '' } addChildren(routes: Array): this {