Merge pull request #4527 from easyops-cn/steve/v3-sub-routes-error

qiaofengxi · web-flow · commit b25d2da695da · 2024-10-23T11:05:07.000+08:00
fix(): handle incremental render with error
diff --git a/packages/runtime/src/internal/Router.ts b/packages/runtime/src/internal/Router.ts
@@ -85,6 +85,7 @@ export class Router {
   #navConfig?: {
     breadcrumb?: BreadcrumbItemConf[];
   };
+  #bootstrapFailed = false;
 
   constructor(storyboards: Storyboard[]) {
     this.#storyboards = storyboards;
@@ -231,7 +232,12 @@ export class Router {
         ignoreRendering = true;
       }
 
-      if (!ignoreRendering && !location.state?.noIncremental) {
+      // Note: dot not perform incremental render when bootstrap failed.
+      if (
+        !ignoreRendering &&
+        !location.state?.noIncremental &&
+        !this.#bootstrapFailed
+      ) {
         ignoreRendering =
           await this.#rendererContext?.didPerformIncrementalRender(
             location,
@@ -491,13 +497,17 @@ export class Router {
       } catch (error) {
         // eslint-disable-next-line no-console
         console.error("Router failed:", error);
+        if (isBootstrap) {
+          this.#bootstrapFailed = true;
+        }
         const result = await routeHelper.catch(error, renderRoot, isBootstrap);
         if (!result) {
           return;
         }
         ({ failed, output } = result);
       }
       renderRoot.child = output.node;
+      this.#bootstrapFailed = false;
 
       cleanUpPreviousRender();
 
diff --git a/packages/runtime/src/internal/Runtime.spec.ts b/packages/runtime/src/internal/Runtime.spec.ts
@@ -460,6 +460,62 @@ const getBootstrapData = (options?: {
             },
           ],
         },
+        {
+          path: "${APP.homepage}/sub-routes-with-error",
+          incrementalSubRoutes: true,
+          type: "bricks",
+          bricks: [
+            {
+              brick: "h1",
+              properties: {
+                textContent: "Sub-routes with error",
+              },
+            },
+            {
+              brick: "div",
+              slots: {
+                "": {
+                  type: "routes",
+                  routes: [
+                    {
+                      path: "${APP.homepage}/sub-routes-with-error/ok",
+                      type: "bricks",
+                      bricks: [
+                        {
+                          brick: "p",
+                          properties: {
+                            textContent: "OK",
+                          },
+                        },
+                      ],
+                    },
+                    {
+                      path: "${APP.homepage}/sub-routes-with-error/fail",
+                      context: [
+                        {
+                          name: "myFailedData",
+                          resolve: {
+                            useProvider: "my-timeout-provider",
+                            args: [0, "oops", true],
+                          },
+                        },
+                      ],
+                      type: "bricks",
+                      bricks: [
+                        {
+                          brick: "p",
+                          properties: {
+                            textContent: "<% CTX.myFailedData %>",
+                          },
+                        },
+                      ],
+                    },
+                  ],
+                },
+              },
+            },
+          ],
+        },
         {
           path: "${APP.homepage}/block",
           exact: true,
@@ -589,9 +645,9 @@ const getBootstrapData = (options?: {
 });
 
 const myTimeoutProvider = jest.fn(
-  (timeout: number, result?: unknown) =>
-    new Promise((resolve) => {
-      setTimeout(() => resolve(result), timeout);
+  (timeout: number, result?: unknown, fail?: boolean) =>
+    new Promise((resolve, reject) => {
+      setTimeout(() => (fail ? reject : resolve)(result), timeout);
     })
 );
 customElements.define(
@@ -844,7 +900,7 @@ describe("Runtime", () => {
     });
   });
 
-  test("incremental sub-router rendering", async () => {
+  test("incremental sub-routes rendering", async () => {
     createRuntime().initialize(getBootstrapData());
     getHistory().push("/app-a/sub-routes/1");
     await getRuntime().bootstrap();
@@ -1401,6 +1457,118 @@ describe("Runtime", () => {
     });
   });
 
+  test("incremental sub-routes with error", async () => {
+    consoleError.mockReturnValueOnce();
+    createRuntime().initialize(getBootstrapData());
+    getHistory().push("/app-a/sub-routes-with-error/ok");
+    await getRuntime().bootstrap();
+    await (global as any).flushPromises();
+    expect(document.body.children).toMatchInlineSnapshot(`
+      HTMLCollection [
+        <div
+          id="main-mount-point"
+        >
+          <h1>
+            Sub-routes with error
+          </h1>
+          <div>
+            <p>
+              OK
+            </p>
+          </div>
+        </div>,
+        <div
+          id="portal-mount-point"
+        />,
+      ]
+    `);
+    expect(consoleError).toHaveBeenCalledTimes(0);
+
+    getHistory().push("/app-a/sub-routes-with-error/fail");
+    await (global as any).flushPromises();
+    await new Promise((resolve) => setTimeout(resolve));
+    expect(document.body.children).toMatchInlineSnapshot(`
+      HTMLCollection [
+        <div
+          id="main-mount-point"
+        >
+          <h1>
+            Sub-routes with error
+          </h1>
+          <div>
+            <div
+              data-error-boundary=""
+            >
+              <div>
+                Oops! Something went wrong: oops
+              </div>
+            </div>
+          </div>
+        </div>,
+        <div
+          id="portal-mount-point"
+        />,
+      ]
+    `);
+    expect(consoleError).toHaveBeenCalledTimes(1);
+
+    getHistory().push("/app-a/sub-routes-with-error/ok");
+    await (global as any).flushPromises();
+    expect(document.body.children).toMatchInlineSnapshot(`
+      HTMLCollection [
+        <div
+          id="main-mount-point"
+        >
+          <h1>
+            Sub-routes with error
+          </h1>
+          <div>
+            <p>
+              OK
+            </p>
+          </div>
+        </div>,
+        <div
+          id="portal-mount-point"
+        />,
+      ]
+    `);
+    expect(consoleError).toHaveBeenCalledTimes(1);
+  });
+
+  test("bootstrap error should prevent incremental render", async () => {
+    consoleError.mockReturnValueOnce();
+    createRuntime().initialize(getBootstrapData());
+    getHistory().push("/app-a/sub-routes-with-error/fail");
+    await expect(() => getRuntime().bootstrap()).rejects.toMatchInlineSnapshot(
+      `"oops"`
+    );
+    expect(consoleError).toHaveBeenCalledTimes(1);
+
+    getHistory().push("/app-a/sub-routes-with-error/ok");
+    await (global as any).flushPromises();
+    expect(document.body.children).toMatchInlineSnapshot(`
+      HTMLCollection [
+        <div
+          id="main-mount-point"
+        >
+          <h1>
+            Sub-routes with error
+          </h1>
+          <div>
+            <p>
+              OK
+            </p>
+          </div>
+        </div>,
+        <div
+          id="portal-mount-point"
+        />,
+      ]
+    `);
+    expect(consoleError).toHaveBeenCalledTimes(1);
+  });
+
   test("abstract routes rendering", async () => {
     createRuntime().initialize(getBootstrapData());
     getHistory().push("/app-a/abstract-routes/1");
diff --git a/packages/runtime/src/internal/data/DataStore.ts b/packages/runtime/src/internal/data/DataStore.ts
@@ -8,6 +8,7 @@ import type {
 import { hasOwnProperty, isObject } from "@next-core/utils/general";
 import { strictCollectMemberUsage } from "@next-core/utils/storyboard";
 import EventTarget from "@ungap/event-target";
+import { pull } from "lodash";
 import { eventCallbackFactory, listenerFactory } from "../bindListeners.js";
 import { asyncCheckIf, checkIf } from "../compute/checkIf.js";
 import {
@@ -55,18 +56,20 @@ export interface DataStoreItem {
   deps: string[];
 }
 
+type PendingStackItem = ReturnType<typeof resolveDataStore>;
+
 export class DataStore<T extends DataStoreType = "CTX"> {
   private readonly type: T;
   private readonly data = new Map<string, DataStoreItem>();
   private readonly changeEventType: string;
-  private readonly pendingStack: Array<ReturnType<typeof resolveDataStore>> =
-    [];
+  private readonly pendingStack: Array<PendingStackItem> = [];
   public readonly hostBrick?: RuntimeBrick;
   private readonly stateStoreId?: string;
   private batchUpdate = false;
   private batchUpdateContextsNames: string[] = [];
   private readonly rendererContext?: RendererContext;
   private routeMap = new WeakMap<RouteConf, Set<string>>();
+  private routeStackMap = new WeakMap<RouteConf, Set<PendingStackItem>>();
 
   // 把 `rendererContext` 放在参数列表的最后，并作为可选，以减少测试文件的调整
   constructor(
@@ -325,6 +328,16 @@ export class DataStore<T extends DataStoreType = "CTX"> {
         this.type,
         isStrictMode(runtimeContext)
       );
+      if (Array.isArray(routePath)) {
+        for (const route of routePath) {
+          const stack = this.routeStackMap.get(route);
+          if (stack) {
+            stack.add(pending);
+          } else {
+            this.routeStackMap.set(route, new Set([pending]));
+          }
+        }
+      }
       this.pendingStack.push(pending);
     }
   }
@@ -349,14 +362,6 @@ export class DataStore<T extends DataStoreType = "CTX"> {
   }
 
   async waitForAll(): Promise<void> {
-    // Silent each pending contexts, since the error is handled by batched `pendingResult`
-    for (const { pendingContexts } of this.pendingStack) {
-      for (const p of pendingContexts.values()) {
-        p.catch(() => {
-          /* No-op */
-        });
-      }
-    }
     for (const { pendingResult } of this.pendingStack) {
       await pendingResult;
     }
@@ -556,13 +561,24 @@ export class DataStore<T extends DataStoreType = "CTX"> {
     return true;
   }
 
+  /**
+   * For sub-routes to be incrementally rendered,
+   * dispose their data and pending tasks.
+   */
   disposeDataInRoutes(routes: RouteConf[]) {
+    //
     for (const route of routes) {
       const names = this.routeMap.get(route);
       if (names !== undefined) {
         for (const name of names) {
           this.data.delete(name);
         }
+        this.routeMap.delete(route);
+      }
+      const stack = this.routeStackMap.get(route);
+      if (stack !== undefined) {
+        pull(this.pendingStack, ...stack);
+        this.routeStackMap.delete(route);
       }
     }
   }
diff --git a/packages/runtime/src/internal/data/resolveDataStore.ts b/packages/runtime/src/internal/data/resolveDataStore.ts
@@ -36,12 +36,14 @@ export function resolveDataStore(
   const deferredContexts = new Map<string, DeferredContext>();
   const pendingContexts = new Map(
     [...new Set(contextConfs.map((contextConf) => contextConf.name))].map(
-      (contextName) => [
-        contextName,
-        new Promise<void>((resolve, reject) => {
+      (contextName) => {
+        const promise = new Promise<void>((resolve, reject) => {
           deferredContexts.set(contextName, { resolve, reject });
-        }),
-      ]
+        });
+        // The pending context will be caught by the renderer.
+        promise.catch(() => {});
+        return [contextName, promise];
+      }
     )
   );
 
@@ -110,6 +112,8 @@ export function resolveDataStore(
       }
       throw error;
     });
+  // The pending result will be caught by the renderer.
+  pendingResult.catch(() => {});
   return { pendingResult, pendingContexts };
 }
 
