fix/refactor: ensure smooth error recovery (#2186)

fwouts · Nov 6, 2023 · b73affc · b73affc
1 parent a6b1e18
commit b73affc
Show file tree

Hide file tree

Showing 13 changed files with 242 additions and 121 deletions.
diff --git a/core/src/previewer.ts b/core/src/previewer.ts
@@ -88,16 +88,17 @@ export class Previewer {
       promise: (async () => {
         const router = express.Router();
         router.get(/^\/.*:[^/]+\/$/, async (req, res, next) => {
+          const accept = req.header("Accept");
           if (req.url.includes("?html-proxy")) {
             next();
             return;
           }
-          const previewableId = req.path.substring(1, req.path.length - 1);
-          if (req.header("Accept") === "text/x-vite-ping") {
+          if (accept === "text/x-vite-ping") {
             // This is triggered as part of HMR. Exit early.
             res.writeHead(204).end();
             return;
           }
+          const previewableId = req.path.substring(1, req.path.length - 1);
           if (!this.viteManager) {
             res.status(404).end(`Uh-Oh! Vite server is not running.`);
             return;

diff --git a/core/src/vite/plugins/preview-script.ts b/core/src/vite/plugins/preview-script.ts
@@ -0,0 +1,95 @@
+import type * as vite from "vite";
+
+export type PreviewScriptOptions = {
+  previewablePath: string;
+  previewableName: string;
+  wrapperPath: string | null;
+  wrapperName: string | null;
+  detectedGlobalCssFilePaths: string[];
+};
+
+const PATH_PREFIX = "/__previewjs_internal__/preview.js?p=";
+
+export function previewScriptPlugin(): vite.Plugin {
+  return {
+    name: "previewjs:preview-script",
+    load(id) {
+      if (!id.startsWith(PATH_PREFIX)) {
+        return;
+      }
+      const base64EncodedOptions = id.substring(PATH_PREFIX.length);
+      const options: PreviewScriptOptions = JSON.parse(
+        Buffer.from(base64EncodedOptions, "base64url").toString("utf8")
+      );
+      return previewScriptSource(options);
+    },
+  };
+}
+
+function previewScriptSource({
+  previewablePath,
+  previewableName,
+  wrapperPath,
+  wrapperName,
+  detectedGlobalCssFilePaths,
+}: PreviewScriptOptions) {
+  return `
+import { initListeners, initPreview } from "/__previewjs_internal__/index.ts";
+
+initListeners();
+
+import.meta.hot.accept();
+
+let refresh = () => {};
+
+window.__PREVIEWJS_IFRAME__.refresh = (options) => {
+  refresh(options);
+};
+
+import.meta.hot.accept(["/${previewablePath}"], ([previewableModule]) => {
+  if (previewableModule) {
+    refresh({
+      previewableModule,
+    });
+  }
+});
+
+${
+  wrapperPath
+    ? `
+const wrapperModulePromise = import(/* @vite-ignore */ "/${wrapperPath}?t=" + Date.now());
+import.meta.hot.accept(["/${wrapperPath}"], ([wrapperModule]) => {
+  if (wrapperModule) {
+    refresh({
+      wrapperModule,
+    });
+  }
+});
+`
+    : `
+const wrapperModulePromise = Promise.all([${detectedGlobalCssFilePaths
+        .map(
+          (cssFilePath) =>
+            `import(/* @vite-ignore */ "/${cssFilePath.replace(
+              /\\/g,
+              "/"
+            )}").catch(() => null)`
+        )
+        .join(",")}]).then(() => null);
+`
+}
+
+// Important: the wrapper must be loaded first as it may monkey-patch
+// modules imported by the component module.
+wrapperModulePromise.then(wrapperModule => {
+  import(/* @vite-ignore */ "/${previewablePath}?t=" + Date.now()).then(previewableModule => {
+    refresh = initPreview({
+      previewableModule,
+      previewableName: ${JSON.stringify(previewableName)},
+      wrapperModule,
+      wrapperName: ${JSON.stringify(wrapperName)},
+    });
+  });
+});
+`;
+}
diff --git a/core/src/vite/vite-manager.ts b/core/src/vite/vite-manager.ts
@@ -29,6 +29,10 @@ import { generateHtmlError } from "../html-error.js";
 import type { FrameworkPlugin } from "../plugins/framework.js";
 import { cssModulesWithoutSuffixPlugin } from "./plugins/css-modules-without-suffix-plugin.js";
 import { localEval } from "./plugins/local-eval.js";
+import {
+  previewScriptPlugin,
+  type PreviewScriptOptions,
+} from "./plugins/preview-script.js";
 import { publicAssetImportPluginPlugin } from "./plugins/public-asset-import-plugin.js";
 import { virtualPlugin } from "./plugins/virtual-plugin.js";
 
@@ -160,7 +164,7 @@ export class ViteManager {
         );
         const { config, viteServer } = state;
         const { filePath, name: previewableName } = decodePreviewableId(id);
-        const componentPath = filePath.replace(/\\/g, "/");
+        const previewablePath = filePath.replace(/\\/g, "/");
         const wrapper = config.wrapper;
         const wrapperPath =
           wrapper &&
@@ -171,66 +175,16 @@ export class ViteManager {
           url,
           template.replace(
             "<!-- %OPTIONAL_HEAD_CONTENT% -->",
-            `
-    <script type="module">
-    import { initListeners, initPreview } from "/__previewjs_internal__/index.ts";
-
-    initListeners();
-
-    import.meta.hot.accept();
-
-    let refresh = () => {};
-
-    window.__PREVIEWJS_IFRAME__.refresh = (options) => {
-      refresh(options);
-    };
-
-    import.meta.hot.accept(["/${componentPath}"], ([previewableModule]) => {
-      if (previewableModule) {
-        refresh({
-          previewableModule,
-        });
-      }
-    });
-
-    ${
-      wrapperPath
-        ? `
-    const wrapperModulePromise = import(/* @vite-ignore */ "/${wrapperPath}");
-    import.meta.hot.accept(["/${wrapperPath}"], ([wrapperModule]) => {
-      if (wrapperModule) {
-        refresh({
-          wrapperModule,
-        });
-      }
-    });
-    `
-        : `
-    const wrapperModulePromise = Promise.all([${config.detectedGlobalCssFilePaths
-      .map(
-        (cssFilePath) =>
-          `import(/* @vite-ignore */ "/${cssFilePath.replace(
-            /\\/g,
-            "/"
-          )}").catch(() => null)`
-      )
-      .join(",")}]).then(() => null);
-    `
-    }
-
-    // Important: the wrapper must be loaded first as it may monkey-patch
-    // modules imported by the component module.
-    wrapperModulePromise.then(wrapperModule => {
-      import(/* @vite-ignore */ "/${componentPath}").then(previewableModule => {
-        refresh = initPreview({
-          previewableModule,
-          previewableName: ${JSON.stringify(previewableName)},
-          wrapperModule,
-          wrapperName: ${JSON.stringify(wrapper?.componentName || null)},
-        });
-      });
-    });
-    </script>`
+            `<script type="module" src="/__previewjs_internal__/preview.js?p=${Buffer.from(
+              JSON.stringify({
+                previewablePath,
+                previewableName,
+                wrapperPath,
+                wrapperName: wrapper?.componentName || null,
+                detectedGlobalCssFilePaths: config.detectedGlobalCssFilePaths,
+              } satisfies PreviewScriptOptions),
+              "utf8"
+            ).toString("base64url")}"></script>`
           )
         );
       } catch (e) {
@@ -369,6 +323,7 @@ export class ViteManager {
           viteTsconfigPaths({
             root: this.options.rootDir,
           }),
+          previewScriptPlugin(),
           virtualPlugin({
             logger: this.options.logger,
             reader: this.options.reader,

diff --git a/framework-plugins/react/preview/index.tsx b/framework-plugins/react/preview/index.tsx
@@ -40,7 +40,7 @@ export const loadRenderer: RendererLoader = async ({
     : Previewable;
   const Renderer = (props: any) => {
     return (
-      <ErrorBoundary key={renderId} renderId={renderId}>
+      <ErrorBoundary renderId={renderId}>
         <Wrapper>
           {decorators.reduce(
             (component, decorator) => () => decorator(component),

diff --git a/framework-plugins/react/tests/console.spec.ts b/framework-plugins/react/tests/console.spec.ts
@@ -43,10 +43,7 @@ for (const reactVersion of reactVersions()) {
           }`
         );
         await preview.iframe.waitForSelector("#update-2");
-        await preview.expectLoggedMessages.toMatch(
-          ["Render 1", "Render 2"],
-          "log"
-        );
+        await preview.expectLoggedMessages.toMatch(["Render 2"], "log");
       });
     });
   });

diff --git a/framework-plugins/react/tests/error-handling.spec.ts b/framework-plugins/react/tests/error-handling.spec.ts
@@ -50,6 +50,45 @@ test.describe.parallel("react/error handling", () => {
         await preview.iframe.waitForSelector(".App");
       });
 
+      test("recovers well from errors", async (preview) => {
+        await preview.fileManager.update(
+          "src/App.tsx",
+          `export function Foo() {
+          return <p className="init">Foo</p>
+        }`
+        );
+        await preview.show("src/App.tsx:Foo");
+        await preview.iframe.waitForSelector(".init");
+
+        const append = 'return <p className="end">Bar</p>;';
+        for (let i = 0; i < append.length; i++) {
+          const partialAppend = append.slice(0, i);
+          await preview.fileManager.update(
+            "src/App.tsx",
+            `export function Foo() {
+              ${partialAppend}
+              return <p>Foo</p>
+            }`,
+
+            {
+              inMemoryOnly: false,
+            }
+          );
+          try {
+            await preview.expectErrors.toMatch(
+              reactVersion < 18 && i >= 6 && i <= 8
+                ? ["Nothing was returned from render"]
+                : (i > 0 && i < 6) || (i > 8 && i < append.length - 1)
+                ? ["App.tsx"]
+                : []
+            );
+          } catch (e) {
+            throw new Error(`Failure at index ${i}: ${e}`);
+          }
+        }
+        await preview.iframe.waitForSelector(".end");
+      });
+
       test("fails correctly when encountering broken module imports before update", async (preview) => {
         await preview.fileManager.update(
           "src/App.tsx",

diff --git a/framework-plugins/vue2/tests/console.spec.ts b/framework-plugins/vue2/tests/console.spec.ts
@@ -50,6 +50,6 @@ test.describe.parallel("vue2/console", () => {
       </script>`
     );
     await preview.iframe.waitForSelector(".App-updated-2");
-    await preview.expectLoggedMessages.toMatch(["Render 1", "Render 2"], "log");
+    await preview.expectLoggedMessages.toMatch(["Render 2"], "log");
   });
 });
diff --git a/framework-plugins/vue3/tests/console.spec.ts b/framework-plugins/vue3/tests/console.spec.ts
@@ -50,6 +50,6 @@ test.describe.parallel("vue3/console", () => {
       </script>`
     );
     await preview.iframe.waitForSelector(".App-updated-2");
-    await preview.expectLoggedMessages.toMatch(["Render 1", "Render 2"], "log");
+    await preview.expectLoggedMessages.toMatch(["Render 2"], "log");
   });
 });
diff --git a/iframe/preview/__previewjs_internal__/index.ts b/iframe/preview/__previewjs_internal__/index.ts
@@ -6,9 +6,15 @@ import { runRenderer } from "./run-renderer";
 import { setState } from "./state";
 import { setupViteHmrListener } from "./vite-hmr-listener";
 
+let listenersInitialized = false;
+
 // Important: initListeners() must be invoked before we try to load any modules
 // that might fail to load, such as a component, so we can intercept Vite errors.
 export function initListeners() {
+  if (listenersInitialized) {
+    return;
+  }
+  listenersInitialized = true;
   setupViteHmrListener();
   setUpLogInterception();
   setUpLinkInterception();
@@ -32,9 +38,7 @@ export function initPreview({
   }
   let renderId = 0;
 
-  async function runNewRender({
-    triggeredByViteInvalidate = false,
-  }: { triggeredByViteInvalidate?: boolean } = {}) {
+  async function runNewRender() {
     const rootHtml = root.innerHTML;
     try {
       renderId += 1;
@@ -45,7 +49,6 @@ export function initPreview({
         previewableModule,
         previewableName,
         renderId,
-        triggeredByViteInvalidate,
         shouldAbortRender: () => renderId !== thisRenderId,
         loadRenderer,
       });
@@ -71,14 +74,14 @@ export function initPreview({
     kind: "bootstrapped",
   });
 
-  return (options) => {
+  return (options = {}) => {
     if (options.previewableModule) {
       previewableModule = options.previewableModule;
     }
     if (options.wrapperModule) {
       wrapperModule = options.wrapperModule;
     }
     // eslint-disable-next-line no-console
-    runNewRender(options).catch(console.error);
+    runNewRender().catch(console.error);
   };
 }
diff --git a/iframe/preview/__previewjs_internal__/logs.ts b/iframe/preview/__previewjs_internal__/logs.ts
@@ -51,15 +51,13 @@ export function setUpLogInterception() {
   };
   window.onunhandledrejection = (event) => {
     const message = formatError(event.reason);
-    if (
-      message.includes("Failed to fetch dynamically imported module") ||
-      message.includes("Failed to reload")
-    ) {
-      return;
-    }
     window.__PREVIEWJS_IFRAME__.reportEvent({
       kind: "error",
-      source: "renderer",
+      source:
+        message.includes("Failed to fetch dynamically imported module") ||
+        message.includes("Failed to reload")
+          ? "vite"
+          : "renderer",
       message,
     });
   };