fix(dev)!: rework _server.tsx (#486)

This reworks the exports of _server.tsx. According to [this](vitejs/vite#15345 (comment)) transformIndexHtml should be called on the index.html (which is provided automatically) and not the entire result of calling render (after the server code is injected). It hasn’t actually posed a problem but I’d like to get ahead of it and make sure we’re doing it the right way. It really only affects local dev but it’s breaking since what’s exported from _server.tsx has changed. _server.tsx is now expected to export 3 things: ``` export const render = async (pageContext: PageContext<any>) => { const { Page, pageProps } = pageContext; return ReactDOMServer.renderToString(<Page {...pageProps} />); }; export const replacementTag = ""; export const indexHtml = `<!DOCTYPE html> <html lang=""> <head></head> <body> <div id="reactele">${replacementTag}</div> </body> </html>`; ``` Specifically, `indexHtml` is now split out from the server `render`. It also requires a `replacementTag` to know where the server html should be injected into the indexHtml. --------- Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
yext · Feb 6, 2024 · f187078 · f187078
1 parent ea73704
commit f187078
Show file tree

Hide file tree

Showing 8 changed files with 122 additions and 69 deletions.
diff --git a/packages/pages/etc/pages.api.md b/packages/pages/etc/pages.api.md
@@ -9,6 +9,11 @@ import { default as React_2 } from "react";
 // @public
 export type Attributes = Record<string, string>;
 
+// @internal
+export interface ClientRenderTemplate {
+  render(pageContext: PageContext<any>): Promise<string>;
+}
+
 // @internal
 export interface ClientServerRenderTemplates {
   clientRenderTemplatePath: string;
@@ -153,8 +158,10 @@ export type Render<T extends TemplateRenderProps<T>> = (props: T) => string;
 export const renderHeadConfigToString: (headConfig: HeadConfig) => string;
 
 // @internal
-export interface RenderTemplate {
+export interface ServerRenderTemplate {
+  indexHtml: string;
   render(pageContext: PageContext<any>): Promise<string>;
+  replacementTag: string;
 }
 
 // @public

diff --git a/packages/pages/src/common/src/template/hydration.ts b/packages/pages/src/common/src/template/hydration.ts
@@ -100,63 +100,60 @@ const makeAbsolute = (path: string): string => {
  * For the most part, injects data into the <head> tag. It also provides validation.
  *
  * @param clientHydrationString if this is undefined then hydration is skipped
- * @param serverHtml
+ * @param indexHtml
  * @param appLanguage
  * @param headConfig
  * @returns the server template with injected html
  */
-const getCommonInjectedServerHtml = (
+const getCommonInjectedIndexHtml = (
   clientHydrationString: string | undefined,
-  serverHtml: string,
+  indexHtml: string,
   appLanguage: string,
   headConfig?: HeadConfig
 ): string => {
   // Add the language to the <html> tag if it exists
-  serverHtml = serverHtml.replace("<!--app-lang-->", appLanguage);
+  indexHtml = indexHtml.replace("<!--app-lang-->", appLanguage);
 
   if (clientHydrationString) {
-    serverHtml = injectIntoHead(
-      serverHtml,
+    indexHtml = injectIntoEndOfHead(
+      indexHtml,
       `<script type="module">${clientHydrationString}</script>`
     );
   }
 
   if (headConfig) {
-    serverHtml = injectIntoHead(
-      serverHtml,
-      renderHeadConfigToString(headConfig)
-    );
+    indexHtml = injectIntoHead(indexHtml, renderHeadConfigToString(headConfig));
   }
 
-  return serverHtml;
+  return indexHtml;
 };
 
 /**
  * Use for the Vite dev server.
  *
  * @param clientHydrationString
- * @param serverHtml
+ * @param indexHtml
  * @param appLanguage
  * @param headConfig
  * @returns the server template to render in the Vite dev environment
  */
-export const getServerTemplateDev = (
+export const getIndexTemplateDev = (
   clientHydrationString: string | undefined,
-  serverHtml: string,
+  indexHtml: string,
   appLanguage: string,
   headConfig?: HeadConfig
 ): string => {
-  return getCommonInjectedServerHtml(
+  return getCommonInjectedIndexHtml(
     clientHydrationString,
-    serverHtml,
+    indexHtml,
     appLanguage,
     headConfig
   );
 };
 
 /**
  * Used for the Deno plugin execution context. The major difference between this function
- * and {@link getServerTemplateDev} is that it also injects the CSS import tags which is
+ * and {@link getIndexTemplateDev} is that it also injects the CSS import tags which is
  * not required by Vite since those are injected automatically by the Vite dev server.
  *
  * @param clientHydrationString
@@ -176,7 +173,7 @@ export const getServerTemplatePlugin = (
   appLanguage: string,
   headConfig?: HeadConfig
 ) => {
-  let html = getCommonInjectedServerHtml(
+  let html = getCommonInjectedIndexHtml(
     clientHydrationString,
     serverHtml,
     appLanguage,
@@ -249,3 +246,23 @@ const injectIntoHead = (html: string, stringToInject: string): string => {
     html.slice(openingHeadIndex)
   );
 };
+
+const closingHeadTag = "</head>";
+
+/**
+ * Finds the ending </head> tag and injects the input string into it.
+ * @param html
+ */
+const injectIntoEndOfHead = (html: string, stringToInject: string): string => {
+  const closingHeadIndex = html.indexOf(closingHeadTag);
+
+  if (closingHeadIndex === -1) {
+    throw new Error("_server.tsx: No head tag is defined");
+  }
+
+  return (
+    html.slice(0, closingHeadIndex) +
+    stringToInject +
+    html.slice(closingHeadIndex)
+  );
+};
diff --git a/packages/pages/src/common/src/template/internal/_server.tsx b/packages/pages/src/common/src/template/internal/_server.tsx
@@ -4,17 +4,18 @@ import * as ReactDOMServer from "react-dom/server";
 import * as React from "react";
 import { PageContext } from "../types.js";
 
-export { render };
-
-const render = async (pageContext: PageContext<any>) => {
+export const render = async (pageContext: PageContext<any>) => {
   const { Page, pageProps } = pageContext;
-  const viewHtml = ReactDOMServer.renderToString(<Page {...pageProps} />);
 
-  return `<!DOCTYPE html>
+  return ReactDOMServer.renderToString(<Page {...pageProps} />);
+};
+
+export const replacementTag = "<!--YEXT-SERVER-->";
+
+export const indexHtml = `<!DOCTYPE html>
     <html lang="<!--app-lang-->">
       <head></head>
       <body>
-        <div id="reactele">${viewHtml}</div>
+        <div id="reactele">${replacementTag}</div>
       </body>
     </html>`;
-};
diff --git a/packages/pages/src/common/src/template/types.ts b/packages/pages/src/common/src/template/types.ts
@@ -247,11 +247,27 @@ export interface ClientServerRenderTemplates {
 }
 
 /**
- * The type of the client/server render templates.
+ * The type of the server render template.
  *
  * @internal
  */
-export interface RenderTemplate {
+export interface ServerRenderTemplate {
+  /** The render function required by the render templates */
+  render(pageContext: PageContext<any>): Promise<string>;
+
+  /** The index.html entrypoint for your template */
+  indexHtml: string;
+
+  /** The tag in indexHtml to replace with the contents of render */
+  replacementTag: string;
+}
+
+/**
+ * The type of the client render template.
+ *
+ * @internal
+ */
+export interface ClientRenderTemplate {
   /** The render function required by the render templates */
   render(pageContext: PageContext<any>): Promise<string>;
 }

diff --git a/packages/pages/src/dev/server/middleware/sendAppHTML.ts b/packages/pages/src/dev/server/middleware/sendAppHTML.ts
@@ -1,7 +1,7 @@
 import { ViteDevServer } from "vite";
 import { TemplateModuleInternal } from "../../../common/src/template/internal/types.js";
 import {
-  RenderTemplate,
+  ServerRenderTemplate,
   TemplateRenderProps,
 } from "../../../common/src/template/types.js";
 import { getLang } from "../../../common/src/template/head.js";
@@ -11,7 +11,7 @@ import { getGlobalClientServerRenderTemplates } from "../../../common/src/templa
 import { ProjectStructure } from "../../../common/src/project/structure.js";
 import {
   getHydrationTemplateDev,
-  getServerTemplateDev,
+  getIndexTemplateDev,
 } from "../../../common/src/template/hydration.js";
 
 /**
@@ -38,19 +38,6 @@ export default async function sendAppHTML(
     projectStructure.getTemplatePaths()
   );
 
-  const serverRenderTemplateModule = (await vite.ssrLoadModule(
-    clientServerRenderTemplates.serverRenderTemplatePath
-  )) as RenderTemplate;
-
-  const getServerHtml = async () => {
-    // using this wrapper function prevents SRR client-server mistmatches if
-    // the template modifies props
-    return await serverRenderTemplateModule.render({
-      Page: templateModuleInternal.default!,
-      pageProps: props,
-    });
-  };
-
   const headConfig = templateModuleInternal.getHeadConfig
     ? templateModuleInternal.getHeadConfig(props)
     : undefined;
@@ -62,18 +49,36 @@ export default async function sendAppHTML(
     templateModuleInternal.config.hydrate
   );
 
-  const clientInjectedServerHtml = getServerTemplateDev(
+  const serverRenderTemplateModule = (await vite.ssrLoadModule(
+    clientServerRenderTemplates.serverRenderTemplatePath
+  )) as ServerRenderTemplate;
+
+  const clientInjectedIndexHtml = getIndexTemplateDev(
     clientHydrationString,
-    await getServerHtml(),
+    serverRenderTemplateModule.indexHtml,
     getLang(headConfig, props),
     headConfig
   );
 
-  const html = await vite.transformIndexHtml(
+  const transformedIndexHtml = await vite.transformIndexHtml(
     // vite decodes request urls when caching proxy requests so we have to
     // load the transform request with a decoded uri
     decodeURIComponent(pathname),
-    clientInjectedServerHtml
+    clientInjectedIndexHtml
+  );
+
+  const getServerHtml = async () => {
+    // using this wrapper function prevents SSR client-server mistmatches if
+    // the template modifies props
+    return await serverRenderTemplateModule.render({
+      Page: templateModuleInternal.default!,
+      pageProps: props,
+    });
+  };
+
+  const html = transformedIndexHtml.replace(
+    serverRenderTemplateModule.replacementTag,
+    await getServerHtml()
   );
 
   // Send the rendered HTML back.

diff --git a/packages/pages/src/vite-plugin/build/buildStart/rendering/templateUtils.test.tsx b/packages/pages/src/vite-plugin/build/buildStart/rendering/templateUtils.test.tsx
@@ -2,7 +2,7 @@ import { describe, it, expect, vi } from "vitest";
 import React from "react";
 import { TemplateModuleInternal } from "../../../../common/src/template/internal/types.js";
 import {
-  RenderTemplate,
+  ServerRenderTemplate,
   TemplateProps,
   Manifest,
 } from "../../../../common/src/template/types.js";
@@ -40,18 +40,18 @@ const baseProps: TemplateProps = {
   },
 };
 
-const serverRenderTemplate: RenderTemplate = {
+const serverRenderTemplate: ServerRenderTemplate = {
   render: () => {
-    return Promise.resolve(
-      `<!DOCTYPE html>
-        <html lang="<!--app-lang-->">
-          <head></head>
-          <body>
-            <div id="reactele"></div>
-          </body>
-        </html>`
-    );
+    return Promise.resolve("");
   },
+  indexHtml: `<!DOCTYPE html>
+  <html lang="<!--app-lang-->">
+    <head></head>
+    <body>
+      <div id="reactele"><!--REPLACE-ME></div>
+    </body>
+  </html>`,
+  replacementTag: "<!--REPLACE-ME>",
 };
 
 describe("generateResponses", () => {

diff --git a/packages/pages/src/vite-plugin/build/buildStart/rendering/templateUtils.ts b/packages/pages/src/vite-plugin/build/buildStart/rendering/templateUtils.ts
@@ -3,7 +3,7 @@ import {
   TemplateRenderProps,
   Manifest,
   TemplateModule,
-  RenderTemplate,
+  ServerRenderTemplate,
 } from "../../../../common/src/template/types.js";
 import { getRelativePrefixToRootFromPath } from "../../../../common/src/template/paths.js";
 import { reactWrapper } from "./wrapper.js";
@@ -49,7 +49,7 @@ export const readTemplateModules = async (
 /** The render template information needed by the plugin execution */
 export interface PluginRenderTemplates {
   /** The server render module */
-  server: RenderTemplate;
+  server: ServerRenderTemplate;
   /** The client render relative path */
   client: string;
 }
@@ -80,12 +80,14 @@ export const getPluginRenderTemplates = async (
 // caches dynamically imported plugin render template modules. Without this, dynamically imported
 // modules will leak some memory during generation. This can cause issues on a publish with a large
 // number of generations.
-const pluginRenderTemplatesCache = new Map<string, RenderTemplate>();
+const pluginRenderTemplatesCache = new Map<string, ServerRenderTemplate>();
 
-const importRenderTemplate = async (path: string): Promise<RenderTemplate> => {
+const importRenderTemplate = async (
+  path: string
+): Promise<ServerRenderTemplate> => {
   let module = pluginRenderTemplatesCache.get(path);
   if (!module) {
-    module = (await import(path)) as RenderTemplate;
+    module = (await import(path)) as ServerRenderTemplate;
     pluginRenderTemplatesCache.set(path, module);
   }
   return module;

diff --git a/packages/pages/src/vite-plugin/build/buildStart/rendering/wrapper.ts b/packages/pages/src/vite-plugin/build/buildStart/rendering/wrapper.ts
@@ -32,11 +32,6 @@ export const reactWrapper = async <T extends TemplateRenderProps>(
     `${templateModuleInternal.templateName}.tsx`
   );
 
-  const serverHtml = await pluginRenderTemplates.server.render({
-    Page: templateModuleInternal.default!,
-    pageProps: props,
-  });
-
   let clientHydrationString;
   if (hydrate) {
     clientHydrationString = getHydrationTemplate(
@@ -46,9 +41,19 @@ export const reactWrapper = async <T extends TemplateRenderProps>(
     );
   }
 
+  const serverHtml = await pluginRenderTemplates.server.render({
+    Page: templateModuleInternal.default!,
+    pageProps: props,
+  });
+
+  const html = pluginRenderTemplates.server.indexHtml.replace(
+    pluginRenderTemplates.server.replacementTag,
+    serverHtml
+  );
+
   const clientInjectedServerHtml = getServerTemplatePlugin(
     clientHydrationString,
-    serverHtml,
+    html,
     templateFilepath,
     manifest.bundlerManifest,
     getLang(headConfig, props),