Skip to content

Commit 69d916c

Browse files
committed
fix(): handle incremental render with error
1 parent 3838aaf commit 69d916c

File tree

4 files changed

+218
-20
lines changed

4 files changed

+218
-20
lines changed

packages/runtime/src/internal/Router.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@ export class Router {
8585
#navConfig?: {
8686
breadcrumb?: BreadcrumbItemConf[];
8787
};
88+
#bootstrapFailed = false;
8889

8990
constructor(storyboards: Storyboard[]) {
9091
this.#storyboards = storyboards;
@@ -231,7 +232,12 @@ export class Router {
231232
ignoreRendering = true;
232233
}
233234

234-
if (!ignoreRendering && !location.state?.noIncremental) {
235+
// Note: dot not perform incremental render when bootstrap failed.
236+
if (
237+
!ignoreRendering &&
238+
!location.state?.noIncremental &&
239+
!this.#bootstrapFailed
240+
) {
235241
ignoreRendering =
236242
await this.#rendererContext?.didPerformIncrementalRender(
237243
location,
@@ -491,13 +497,17 @@ export class Router {
491497
} catch (error) {
492498
// eslint-disable-next-line no-console
493499
console.error("Router failed:", error);
500+
if (isBootstrap) {
501+
this.#bootstrapFailed = true;
502+
}
494503
const result = await routeHelper.catch(error, renderRoot, isBootstrap);
495504
if (!result) {
496505
return;
497506
}
498507
({ failed, output } = result);
499508
}
500509
renderRoot.child = output.node;
510+
this.#bootstrapFailed = false;
501511

502512
cleanUpPreviousRender();
503513

packages/runtime/src/internal/Runtime.spec.ts

Lines changed: 172 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -460,6 +460,62 @@ const getBootstrapData = (options?: {
460460
},
461461
],
462462
},
463+
{
464+
path: "${APP.homepage}/sub-routes-with-error",
465+
incrementalSubRoutes: true,
466+
type: "bricks",
467+
bricks: [
468+
{
469+
brick: "h1",
470+
properties: {
471+
textContent: "Sub-routes with error",
472+
},
473+
},
474+
{
475+
brick: "div",
476+
slots: {
477+
"": {
478+
type: "routes",
479+
routes: [
480+
{
481+
path: "${APP.homepage}/sub-routes-with-error/ok",
482+
type: "bricks",
483+
bricks: [
484+
{
485+
brick: "p",
486+
properties: {
487+
textContent: "OK",
488+
},
489+
},
490+
],
491+
},
492+
{
493+
path: "${APP.homepage}/sub-routes-with-error/fail",
494+
context: [
495+
{
496+
name: "myFailedData",
497+
resolve: {
498+
useProvider: "my-timeout-provider",
499+
args: [0, "oops", true],
500+
},
501+
},
502+
],
503+
type: "bricks",
504+
bricks: [
505+
{
506+
brick: "p",
507+
properties: {
508+
textContent: "<% CTX.myFailedData %>",
509+
},
510+
},
511+
],
512+
},
513+
],
514+
},
515+
},
516+
},
517+
],
518+
},
463519
{
464520
path: "${APP.homepage}/block",
465521
exact: true,
@@ -589,9 +645,9 @@ const getBootstrapData = (options?: {
589645
});
590646

591647
const myTimeoutProvider = jest.fn(
592-
(timeout: number, result?: unknown) =>
593-
new Promise((resolve) => {
594-
setTimeout(() => resolve(result), timeout);
648+
(timeout: number, result?: unknown, fail?: boolean) =>
649+
new Promise((resolve, reject) => {
650+
setTimeout(() => (fail ? reject : resolve)(result), timeout);
595651
})
596652
);
597653
customElements.define(
@@ -844,7 +900,7 @@ describe("Runtime", () => {
844900
});
845901
});
846902

847-
test("incremental sub-router rendering", async () => {
903+
test("incremental sub-routes rendering", async () => {
848904
createRuntime().initialize(getBootstrapData());
849905
getHistory().push("/app-a/sub-routes/1");
850906
await getRuntime().bootstrap();
@@ -1401,6 +1457,118 @@ describe("Runtime", () => {
14011457
});
14021458
});
14031459

1460+
test("incremental sub-routes with error", async () => {
1461+
consoleError.mockReturnValueOnce();
1462+
createRuntime().initialize(getBootstrapData());
1463+
getHistory().push("/app-a/sub-routes-with-error/ok");
1464+
await getRuntime().bootstrap();
1465+
await (global as any).flushPromises();
1466+
expect(document.body.children).toMatchInlineSnapshot(`
1467+
HTMLCollection [
1468+
<div
1469+
id="main-mount-point"
1470+
>
1471+
<h1>
1472+
Sub-routes with error
1473+
</h1>
1474+
<div>
1475+
<p>
1476+
OK
1477+
</p>
1478+
</div>
1479+
</div>,
1480+
<div
1481+
id="portal-mount-point"
1482+
/>,
1483+
]
1484+
`);
1485+
expect(consoleError).toHaveBeenCalledTimes(0);
1486+
1487+
getHistory().push("/app-a/sub-routes-with-error/fail");
1488+
await (global as any).flushPromises();
1489+
await new Promise((resolve) => setTimeout(resolve));
1490+
expect(document.body.children).toMatchInlineSnapshot(`
1491+
HTMLCollection [
1492+
<div
1493+
id="main-mount-point"
1494+
>
1495+
<h1>
1496+
Sub-routes with error
1497+
</h1>
1498+
<div>
1499+
<div
1500+
data-error-boundary=""
1501+
>
1502+
<div>
1503+
Oops! Something went wrong: oops
1504+
</div>
1505+
</div>
1506+
</div>
1507+
</div>,
1508+
<div
1509+
id="portal-mount-point"
1510+
/>,
1511+
]
1512+
`);
1513+
expect(consoleError).toHaveBeenCalledTimes(1);
1514+
1515+
getHistory().push("/app-a/sub-routes-with-error/ok");
1516+
await (global as any).flushPromises();
1517+
expect(document.body.children).toMatchInlineSnapshot(`
1518+
HTMLCollection [
1519+
<div
1520+
id="main-mount-point"
1521+
>
1522+
<h1>
1523+
Sub-routes with error
1524+
</h1>
1525+
<div>
1526+
<p>
1527+
OK
1528+
</p>
1529+
</div>
1530+
</div>,
1531+
<div
1532+
id="portal-mount-point"
1533+
/>,
1534+
]
1535+
`);
1536+
expect(consoleError).toHaveBeenCalledTimes(1);
1537+
});
1538+
1539+
test("bootstrap error should prevent incremental render", async () => {
1540+
consoleError.mockReturnValueOnce();
1541+
createRuntime().initialize(getBootstrapData());
1542+
getHistory().push("/app-a/sub-routes-with-error/fail");
1543+
await expect(() => getRuntime().bootstrap()).rejects.toMatchInlineSnapshot(
1544+
`"oops"`
1545+
);
1546+
expect(consoleError).toHaveBeenCalledTimes(1);
1547+
1548+
getHistory().push("/app-a/sub-routes-with-error/ok");
1549+
await (global as any).flushPromises();
1550+
expect(document.body.children).toMatchInlineSnapshot(`
1551+
HTMLCollection [
1552+
<div
1553+
id="main-mount-point"
1554+
>
1555+
<h1>
1556+
Sub-routes with error
1557+
</h1>
1558+
<div>
1559+
<p>
1560+
OK
1561+
</p>
1562+
</div>
1563+
</div>,
1564+
<div
1565+
id="portal-mount-point"
1566+
/>,
1567+
]
1568+
`);
1569+
expect(consoleError).toHaveBeenCalledTimes(1);
1570+
});
1571+
14041572
test("abstract routes rendering", async () => {
14051573
createRuntime().initialize(getBootstrapData());
14061574
getHistory().push("/app-a/abstract-routes/1");

packages/runtime/src/internal/data/DataStore.ts

Lines changed: 26 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import type {
88
import { hasOwnProperty, isObject } from "@next-core/utils/general";
99
import { strictCollectMemberUsage } from "@next-core/utils/storyboard";
1010
import EventTarget from "@ungap/event-target";
11+
import { pull } from "lodash";
1112
import { eventCallbackFactory, listenerFactory } from "../bindListeners.js";
1213
import { asyncCheckIf, checkIf } from "../compute/checkIf.js";
1314
import {
@@ -55,18 +56,20 @@ export interface DataStoreItem {
5556
deps: string[];
5657
}
5758

59+
type PendingStackItem = ReturnType<typeof resolveDataStore>;
60+
5861
export class DataStore<T extends DataStoreType = "CTX"> {
5962
private readonly type: T;
6063
private readonly data = new Map<string, DataStoreItem>();
6164
private readonly changeEventType: string;
62-
private readonly pendingStack: Array<ReturnType<typeof resolveDataStore>> =
63-
[];
65+
private readonly pendingStack: Array<PendingStackItem> = [];
6466
public readonly hostBrick?: RuntimeBrick;
6567
private readonly stateStoreId?: string;
6668
private batchUpdate = false;
6769
private batchUpdateContextsNames: string[] = [];
6870
private readonly rendererContext?: RendererContext;
6971
private routeMap = new WeakMap<RouteConf, Set<string>>();
72+
private routeStackMap = new WeakMap<RouteConf, Set<PendingStackItem>>();
7073

7174
// 把 `rendererContext` 放在参数列表的最后,并作为可选,以减少测试文件的调整
7275
constructor(
@@ -325,6 +328,16 @@ export class DataStore<T extends DataStoreType = "CTX"> {
325328
this.type,
326329
isStrictMode(runtimeContext)
327330
);
331+
if (Array.isArray(routePath)) {
332+
for (const route of routePath) {
333+
const stack = this.routeStackMap.get(route);
334+
if (stack) {
335+
stack.add(pending);
336+
} else {
337+
this.routeStackMap.set(route, new Set([pending]));
338+
}
339+
}
340+
}
328341
this.pendingStack.push(pending);
329342
}
330343
}
@@ -349,14 +362,6 @@ export class DataStore<T extends DataStoreType = "CTX"> {
349362
}
350363

351364
async waitForAll(): Promise<void> {
352-
// Silent each pending contexts, since the error is handled by batched `pendingResult`
353-
for (const { pendingContexts } of this.pendingStack) {
354-
for (const p of pendingContexts.values()) {
355-
p.catch(() => {
356-
/* No-op */
357-
});
358-
}
359-
}
360365
for (const { pendingResult } of this.pendingStack) {
361366
await pendingResult;
362367
}
@@ -556,13 +561,24 @@ export class DataStore<T extends DataStoreType = "CTX"> {
556561
return true;
557562
}
558563

564+
/**
565+
* For sub-routes to be incrementally rendered,
566+
* dispose their data and pending tasks.
567+
*/
559568
disposeDataInRoutes(routes: RouteConf[]) {
569+
//
560570
for (const route of routes) {
561571
const names = this.routeMap.get(route);
562572
if (names !== undefined) {
563573
for (const name of names) {
564574
this.data.delete(name);
565575
}
576+
this.routeMap.delete(route);
577+
}
578+
const stack = this.routeStackMap.get(route);
579+
if (stack !== undefined) {
580+
pull(this.pendingStack, ...stack);
581+
this.routeStackMap.delete(route);
566582
}
567583
}
568584
}

packages/runtime/src/internal/data/resolveDataStore.ts

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -36,12 +36,14 @@ export function resolveDataStore(
3636
const deferredContexts = new Map<string, DeferredContext>();
3737
const pendingContexts = new Map(
3838
[...new Set(contextConfs.map((contextConf) => contextConf.name))].map(
39-
(contextName) => [
40-
contextName,
41-
new Promise<void>((resolve, reject) => {
39+
(contextName) => {
40+
const promise = new Promise<void>((resolve, reject) => {
4241
deferredContexts.set(contextName, { resolve, reject });
43-
}),
44-
]
42+
});
43+
// The pending context will be caught by the renderer.
44+
promise.catch(() => {});
45+
return [contextName, promise];
46+
}
4547
)
4648
);
4749

@@ -110,6 +112,8 @@ export function resolveDataStore(
110112
}
111113
throw error;
112114
});
115+
// The pending result will be caught by the renderer.
116+
pendingResult.catch(() => {});
113117
return { pendingResult, pendingContexts };
114118
}
115119

0 commit comments

Comments
 (0)