From ee0ac078cfe6eb0ab3e2a177a87518d9008283e6 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Mon, 18 Jul 2022 10:49:46 -0400 Subject: [PATCH 01/14] chore: remove Deferrable/ResolvedDeferrable types for raw Promises --- examples/data-router/src/App.tsx | 11 +++++------ packages/react-router-dom/index.tsx | 1 - packages/react-router-native/index.tsx | 1 - packages/react-router/index.ts | 3 +-- packages/react-router/lib/components.tsx | 3 +-- packages/react-router/lib/hooks.tsx | 11 +---------- 6 files changed, 8 insertions(+), 22 deletions(-) diff --git a/examples/data-router/src/App.tsx b/examples/data-router/src/App.tsx index 37f564b130..57d27e5cf2 100644 --- a/examples/data-router/src/App.tsx +++ b/examples/data-router/src/App.tsx @@ -1,7 +1,6 @@ import React from "react"; import { type ActionFunction, - type Deferrable, type LoaderFunction, DataBrowserRouter, Deferred, @@ -238,11 +237,11 @@ function Todo() { interface DeferredRouteLoaderData { critical1: string; critical2: string; - lazyResolved: Deferrable; - lazy1: Deferrable; - lazy2: Deferrable; - lazy3: Deferrable; - lazyError: Deferrable; + lazyResolved: Promise; + lazy1: Promise; + lazy2: Promise; + lazy3: Promise; + lazyError: Promise; } const rand = () => Math.round(Math.random() * 100); diff --git a/packages/react-router-dom/index.tsx b/packages/react-router-dom/index.tsx index 5a45117932..1baec91ad0 100644 --- a/packages/react-router-dom/index.tsx +++ b/packages/react-router-dom/index.tsx @@ -63,7 +63,6 @@ export type { ActionFunctionArgs, DataMemoryRouterProps, DataRouteMatch, - Deferrable, DeferredProps, Fetcher, Hash, diff --git a/packages/react-router-native/index.tsx b/packages/react-router-native/index.tsx index 699ca3091c..e50285341e 100644 --- a/packages/react-router-native/index.tsx +++ b/packages/react-router-native/index.tsx @@ -27,7 +27,6 @@ export type { ActionFunctionArgs, DataMemoryRouterProps, DataRouteMatch, - Deferrable, DeferredProps, Fetcher, Hash, diff --git a/packages/react-router/index.ts b/packages/react-router/index.ts index 6eca1ddd64..a0cab90224 100644 --- a/packages/react-router/index.ts +++ b/packages/react-router/index.ts @@ -68,7 +68,7 @@ import { NavigationContext, RouteContext, } from "./lib/context"; -import type { Deferrable, NavigateFunction } from "./lib/hooks"; +import type { NavigateFunction } from "./lib/hooks"; import { useHref, useInRouterContext, @@ -102,7 +102,6 @@ export type { ActionFunctionArgs, DataMemoryRouterProps, DataRouteMatch, - Deferrable, DeferredProps, Fetcher, Hash, diff --git a/packages/react-router/lib/components.tsx b/packages/react-router/lib/components.tsx index 1e83aa42bf..72a2619ac8 100644 --- a/packages/react-router/lib/components.tsx +++ b/packages/react-router/lib/components.tsx @@ -30,7 +30,6 @@ import { DataRouterStateContext, DeferredContext, } from "./context"; -import type { ResolvedDeferrable } from "./hooks"; import { useDeferredData, useInRouterContext, @@ -431,7 +430,7 @@ function DataRoutes({ } export interface DeferredResolveRenderFunction { - (data: ResolvedDeferrable): JSX.Element; + (data: Awaited): JSX.Element; } export interface DeferredProps diff --git a/packages/react-router/lib/hooks.tsx b/packages/react-router/lib/hooks.tsx index df8cec4dc3..1bdd07487b 100644 --- a/packages/react-router/lib/hooks.tsx +++ b/packages/react-router/lib/hooks.tsx @@ -724,21 +724,12 @@ export function useRouteError() { return state.errors?.[thisRoute.route.id]; } -export type Deferrable = never | T | Promise; -export type ResolvedDeferrable = T extends null | undefined - ? T - : T extends Deferrable - ? T2 extends Promise - ? T3 - : T2 - : T; - /** * Returns the happy-path data from the nearest ancestor value */ export function useDeferredData() { let value = React.useContext(DeferredContext); - return value as ResolvedDeferrable; + return value as Awaited; } const alreadyWarned: Record = {}; From 55ffecf5392391ebe6b76cd73c9971d5f75e04d3 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Mon, 18 Jul 2022 12:41:07 -0400 Subject: [PATCH 02/14] feat: support deferred arrays --- packages/router/__tests__/router-test.ts | 48 +++++++++++++++++++- packages/router/router.ts | 9 +--- packages/router/utils.ts | 56 ++++++++++++++++-------- 3 files changed, 86 insertions(+), 27 deletions(-) diff --git a/packages/router/__tests__/router-test.ts b/packages/router/__tests__/router-test.ts index 3ff909b4d8..3453e67185 100644 --- a/packages/router/__tests__/router-test.ts +++ b/packages/router/__tests__/router-test.ts @@ -7162,7 +7162,7 @@ describe("a router", () => { }); describe("deferred", () => { - it("should support returning deferred responses", async () => { + it("should support returning deferred responses (object)", async () => { let t = setup({ routes: [ { @@ -7226,6 +7226,52 @@ describe("a router", () => { }); }); + it("should support returning deferred responses (array)", async () => { + let t = setup({ + routes: [ + { + id: "index", + index: true, + }, + { + id: "lazy", + path: "lazy", + loader: true, + }, + ], + initialEntries: ["/"], + }); + + let A = await t.navigate("/lazy"); + + let dfd1 = defer(); + let dfd2 = defer(); + let dfd3 = defer(); + dfd1.resolve("Immediate data"); + await A.loaders.lazy.resolve( + deferred(["1", "2", dfd1.promise, dfd2.promise, dfd3.promise]) + ); + expect(t.router.state.loaderData).toEqual({ + lazy: [ + "1", + "2", + "Immediate data", + expect.any(Promise), + expect.any(Promise), + ], + }); + + await dfd2.resolve("2"); + expect(t.router.state.loaderData).toEqual({ + lazy: ["1", "2", "Immediate data", "2", expect.any(Promise)], + }); + + await dfd3.resolve("3"); + expect(t.router.state.loaderData).toEqual({ + lazy: ["1", "2", "Immediate data", "2", "3"], + }); + }); + it("should cancel outstanding deferreds on a new navigation", async () => { let t = setup({ routes: [ diff --git a/packages/router/router.ts b/packages/router/router.ts index 485fdc2d41..7d800a8b8b 100644 --- a/packages/router/router.ts +++ b/packages/router/router.ts @@ -1069,20 +1069,15 @@ export function createRouter(init: RouterInit): Router { // Wire up subscribers to update loaderData as promises settle activeDeferreds.forEach((deferredData, routeId) => { - deferredData.subscribe((aborted, loaderDataKey, data) => { + deferredData.subscribe((aborted) => { if (aborted) { activeDeferreds.delete(routeId); return; } - // This will always be defined here, but TS doesn't know that - invariant(loaderDataKey, "Missing loaderDataKey in subscribe"); updateState({ loaderData: { ...state.loaderData, - [routeId]: { - ...state.loaderData[routeId], - [loaderDataKey]: data, - }, + [routeId]: deferredData.data, }, }); // Remove this instance if all promises have settled diff --git a/packages/router/utils.ts b/packages/router/utils.ts index 6dbed087e7..708b40bf10 100644 --- a/packages/router/utils.ts +++ b/packages/router/utils.ts @@ -852,37 +852,55 @@ export const json: JsonFunction = (data, init = {}) => { }; export class DeferredData { - private pendingKeys: Set = new Set(); + private pendingKeys: Set = new Set(); private cancelled: boolean = false; - private subscriber?: (aborted: boolean, key?: string, data?: any) => void = - undefined; - data: RouteData = {}; + private subscriber?: (aborted: boolean) => void = undefined; + data: RouteData | Array; constructor(data: Record) { - Object.entries(data).forEach(([key, value]) => { - // Store all data in our internal copy and track promise keys - this.data[key] = value; - if (value instanceof Promise) { - this.pendingKeys.add(key); - value.then( - (data) => this.onSettle(key, null, data), - (error) => this.onSettle(key, error) - ); - } - }); + // Store all data in our internal copy and track promise keys + if (Array.isArray(data)) { + this.data = [...data]; + data.forEach((value, index) => this.trackPromise(index, value)); + } else { + this.data = { ...data }; + Object.entries(data).forEach(([key, value]) => + this.trackPromise(key, value) + ); + } } - private onSettle(key: string, error: any, data?: any) { + private trackPromise(key: string | number, value: any) { + if (value instanceof Promise) { + this.pendingKeys.add(key); + value.then( + (data) => this.onSettle(key, null, data), + (error) => this.onSettle(key, error) + ); + } + } + + private onSettle(key: string | number, error: any, data?: any) { if (this.cancelled) { return; } this.pendingKeys.delete(key); let value = error ? new DeferredError(error) : data; - this.data[key] = value; - this.subscriber?.(false, key, value); + if (Array.isArray(this.data)) { + invariant(typeof key === "number", "expected key to be a number"); + let data = [...this.data]; + data[key] = value; + this.data = data; + } else { + this.data = { + ...this.data, + [key]: value, + }; + } + this.subscriber?.(false); } - subscribe(fn: (aborted: boolean, key?: string, data?: any) => void) { + subscribe(fn: (aborted: boolean) => void) { this.subscriber = fn; } From 4d94cfb10aef8859bbcf2a76fab8e040c417d750 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Mon, 18 Jul 2022 12:54:15 -0400 Subject: [PATCH 03/14] chore:" remove typings from Deferred and useDeferredData To be added back in once we figur eout typing for useLoaderData in 6.5 --- packages/react-router/lib/components.tsx | 29 ++++++++++++------------ packages/react-router/lib/hooks.tsx | 4 ++-- 2 files changed, 16 insertions(+), 17 deletions(-) diff --git a/packages/react-router/lib/components.tsx b/packages/react-router/lib/components.tsx index 72a2619ac8..180ae52925 100644 --- a/packages/react-router/lib/components.tsx +++ b/packages/react-router/lib/components.tsx @@ -429,14 +429,14 @@ function DataRoutes({ return useRoutes(routes || createRoutesFromChildren(children), location); } -export interface DeferredResolveRenderFunction { - (data: Awaited): JSX.Element; +export interface DeferredResolveRenderFunction { + (data: Awaited): JSX.Element; } -export interface DeferredProps - extends Omit { - children: React.ReactNode | DeferredResolveRenderFunction; - value: Data; +export interface DeferredProps { + children: React.ReactNode | DeferredResolveRenderFunction; + value: any; + fallback: React.SuspenseProps["fallback"]; errorElement?: React.ReactNode; } @@ -444,19 +444,19 @@ export interface DeferredProps * Component to use for rendering lazily loaded data from returning deferred() * in a loader function */ -export function Deferred({ +export function Deferred({ children, value, fallback, errorElement, -}: DeferredProps) { +}: DeferredProps) { return ( {typeof children === "function" ? ( } + children={children as DeferredResolveRenderFunction} /> ) : ( children @@ -497,17 +497,16 @@ function DeferredWrapper({ children, errorElement }: DeferredWrapperProps) { return <>{children}; } -export interface ResolveDeferredProps { - children: DeferredResolveRenderFunction; +interface ResolveDeferredProps { + children: DeferredResolveRenderFunction; } /** * @private + * Indirection to leverage useDeferredData for a render-prop API on */ -export function ResolveDeferred({ - children, -}: ResolveDeferredProps) { - return children(useDeferredData()); +function ResolveDeferred({ children }: ResolveDeferredProps) { + return children(useDeferredData()); } /////////////////////////////////////////////////////////////////////////////// diff --git a/packages/react-router/lib/hooks.tsx b/packages/react-router/lib/hooks.tsx index 1bdd07487b..b11d155cf5 100644 --- a/packages/react-router/lib/hooks.tsx +++ b/packages/react-router/lib/hooks.tsx @@ -727,9 +727,9 @@ export function useRouteError() { /** * Returns the happy-path data from the nearest ancestor value */ -export function useDeferredData() { +export function useDeferredData() { let value = React.useContext(DeferredContext); - return value as Awaited; + return value; } const alreadyWarned: Record = {}; From d9cea85b6219057894536d7160964343b221b52c Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Mon, 18 Jul 2022 16:37:00 -0400 Subject: [PATCH 04/14] Handle deferred render errors at the boundary --- .../__tests__/DataMemoryRouter-test.tsx | 325 ++++++++++-------- packages/react-router/lib/components.tsx | 65 +++- packages/react-router/lib/hooks.tsx | 2 +- 3 files changed, 237 insertions(+), 155 deletions(-) diff --git a/packages/react-router/__tests__/DataMemoryRouter-test.tsx b/packages/react-router/__tests__/DataMemoryRouter-test.tsx index acb7c747b3..6558237d21 100644 --- a/packages/react-router/__tests__/DataMemoryRouter-test.tsx +++ b/packages/react-router/__tests__/DataMemoryRouter-test.tsx @@ -1890,7 +1890,11 @@ describe("", () => { }); describe("deferred", () => { - it("allows loaders to returned deferred data (child component)", async () => { + function setupDeferredTest( + hasRouteErrorElement = false, + hasDeferredErrorElement = false, + triggerRenderError = false + ) { let barDefer = defer(); let { container } = render( @@ -1900,6 +1904,7 @@ describe("", () => { path="bar" loader={() => barDefer.promise} element={} + errorElement={hasRouteErrorElement ? : null} /> @@ -1924,17 +1929,41 @@ describe("", () => { return ( <>

{data.critical}

- Loading...

}> + Loading...

} + errorElement={hasDeferredErrorElement ? : null} + > ); } + + function RouteError() { + let error = useRouteError(); + return

Route Error:{error.message}

; + } + function LazyData() { - let data = useDeferredData(); - return

{data}

; + let data = useDeferredData(); + return triggerRenderError ? ( +

{oops.i.did.it.again}

+ ) : ( +

{data}

+ ); } + function LazyError() { + let data = useRouteError(); + return

Handled Error:{data.message}

; + } + + return { container, barDefer }; + } + + it("allows loaders to returned deferred data (child component)", async () => { + let { barDefer, container } = setupDeferredTest(); fireEvent.click(screen.getByText("Link to Bar")); expect(getHtml(container)).toMatchInlineSnapshot(` "
@@ -2008,45 +2037,7 @@ describe("", () => { }); it("allows loaders to returned deferred data (render prop)", async () => { - let barDefer = defer(); - let { container } = render( - - }> - } /> - barDefer.promise} - element={} - /> - - - ); - - function Layout() { - let navigation = useNavigation(); - return ( -
- Link to Bar -

{navigation.state}

- -
- ); - } - - function Foo() { - return

Foo

; - } - function Bar() { - let data = useLoaderData(); - return ( - <> -

{data.critical}

- value={data.lazy} fallback={

Loading...

}> - {(data) =>

{data}

} - - - ); - } + let { barDefer, container } = setupDeferredTest(); fireEvent.click(screen.getByText("Link to Bar")); expect(getHtml(container)).toMatchInlineSnapshot(` @@ -2120,58 +2111,84 @@ describe("", () => { `); }); - it("sends errors to the provided errorElement", async () => { - let barDefer = defer(); - let { container } = render( - - }> - } /> - barDefer.promise} - element={} - /> - - - ); + it("sends data errors to the provided errorElement", async () => { + let { barDefer, container } = setupDeferredTest(true, true, false); - function Layout() { - let navigation = useNavigation(); - return ( + fireEvent.click(screen.getByText("Link to Bar")); + expect(getHtml(container)).toMatchInlineSnapshot(` + "
- Link to Bar -

{navigation.state}

- + + Link to Bar + +

+ loading +

+

+ Foo +

- ); - } +
" + `); - function Foo() { - return

Foo

; - } - function Bar() { - let data = useLoaderData(); - return ( - <> -

{data.critical}

- Loading...

} - errorElement={} + let barValueDfd = defer(); + barDefer.resolve( + deferred({ + critical: "CRITICAL", + lazy: barValueDfd.promise, + }) + ); + await waitFor(() => screen.getByText("idle")); + expect(getHtml(container)).toMatchInlineSnapshot(` + "" + `); + + barValueDfd.reject(new Error("Kaboom!")); + await waitFor(() => screen.getByText(/Kaboom!/)); + expect(getHtml(container)).toMatchInlineSnapshot(` + "
+
+ + Link to Bar + +

+ idle +

+

+ CRITICAL +

+

+ Handled Error: + Error: Kaboom! +

+
+
" + `); + }); + + it("sends unhandled data errors to the nearest route error boundary", async () => { + let { barDefer, container } = setupDeferredTest(true, false, false); fireEvent.click(screen.getByText("Link to Bar")); expect(getHtml(container)).toMatchInlineSnapshot(` @@ -2235,10 +2252,7 @@ describe("", () => { idle

- CRITICAL -

-

- Handled Error: + Route Error: Error: Kaboom!

@@ -2246,55 +2260,84 @@ describe("", () => { `); }); - it("sends unhandled errors to the nearest route error boundary", async () => { - let barDefer = defer(); - let { container } = render( - - }> - } /> - barDefer.promise} - element={} - errorElement={} - /> - - + it("sends render errors to the provided errorElement", async () => { + let { barDefer, container } = setupDeferredTest(true, true, true); + + fireEvent.click(screen.getByText("Link to Bar")); + expect(getHtml(container)).toMatchInlineSnapshot(` + "
+
+ + Link to Bar + +

+ loading +

+

+ Foo +

+
+
" + `); + + let barValueDfd = defer(); + barDefer.resolve( + deferred({ + critical: "CRITICAL", + lazy: barValueDfd.promise, + }) ); + await waitFor(() => screen.getByText("idle")); + expect(getHtml(container)).toMatchInlineSnapshot(` + "
+
+ + Link to Bar + +

+ idle +

+

+ CRITICAL +

+

+ Loading... +

+
+
" + `); - function Layout() { - let navigation = useNavigation(); - return ( + barValueDfd.resolve("LAZY"); + await waitFor(() => !screen.getByText(/Loading.../)); + expect(getHtml(container)).toMatchInlineSnapshot(` + "
- Link to Bar -

{navigation.state}

- + + Link to Bar + +

+ idle +

+

+ CRITICAL +

+

+ Handled Error: + oops is not defined +

- ); - } +
" + `); + }); - function Foo() { - return

Foo

; - } - function Bar() { - let data = useLoaderData(); - return ( - <> -

{data.critical}

- Loading...

}> - - - - ); - } - function LazyData() { - let data = useDeferredData(); - return

{data}

; - } - function RouteError() { - let error = useRouteError(); - return

Route Error:{error.message}

; - } + it("sends unhandled render errors to the nearest route error boundary", async () => { + let { barDefer, container } = setupDeferredTest(true, false, true); fireEvent.click(screen.getByText("Link to Bar")); expect(getHtml(container)).toMatchInlineSnapshot(` @@ -2344,8 +2387,8 @@ describe("", () => { " `); - barValueDfd.reject(new Error("Kaboom!")); - await waitFor(() => screen.getByText(/Kaboom!/)); + barValueDfd.resolve("LAZY"); + await waitFor(() => !screen.getByText(/Loading.../)); expect(getHtml(container)).toMatchInlineSnapshot(` "
@@ -2359,7 +2402,7 @@ describe("", () => {

Route Error: - Error: Kaboom! + oops is not defined

" diff --git a/packages/react-router/lib/components.tsx b/packages/react-router/lib/components.tsx index 180ae52925..5eced6319b 100644 --- a/packages/react-router/lib/components.tsx +++ b/packages/react-router/lib/components.tsx @@ -451,9 +451,9 @@ export function Deferred({ errorElement, }: DeferredProps) { return ( - + - + {typeof children === "function" ? ( - + ); } +type DeferredErrorBoundaryProps = React.PropsWithChildren<{ + value: any; + errorElement?: React.ReactNode; +}>; + +type DeferredErrorBoundaryState = { + error: any; +}; + +class DeferredErrorBoundary extends React.Component< + DeferredErrorBoundaryProps, + DeferredErrorBoundaryState +> { + constructor(props: DeferredErrorBoundaryProps) { + super(props); + this.state = { error: null }; + } + + static getDerivedStateFromError(error: any) { + return { error }; + } + + componentDidCatch(error: any, errorInfo: any) { + console.error( + " caught the following error during render", + error, + errorInfo + ); + } + + render() { + let { children, errorElement, value } = this.props; + // Handle render errors from this.state, or data errors from context + let error = this.state.error || (isDeferredError(value) ? value : null); + + if (error) { + if (errorElement) { + return ( + + ); + } + // Throw to ancestor route-level error boundary + throw error; + } + return ; + } +} + interface DeferredWrapperProps { children: React.ReactNode; errorElement?: React.ReactNode; @@ -478,22 +526,13 @@ interface DeferredWrapperProps { * fallback, or rendering the children/errorElement once the promise resolves * or rejects */ -function DeferredWrapper({ children, errorElement }: DeferredWrapperProps) { +function DeferredWrapper({ children }: DeferredWrapperProps) { let value = React.useContext(DeferredContext); if (value instanceof Promise) { // throw to the suspense boundary throw value; } - if (isDeferredError(value)) { - if (errorElement) { - return <>{errorElement}; - } else { - // Throw to the nearest route-level error boundary - throw value; - } - } - return <>{children}; } diff --git a/packages/react-router/lib/hooks.tsx b/packages/react-router/lib/hooks.tsx index b11d155cf5..2cd151d1a7 100644 --- a/packages/react-router/lib/hooks.tsx +++ b/packages/react-router/lib/hooks.tsx @@ -704,7 +704,7 @@ export function useRouteError() { let deferredValue = React.useContext(DeferredContext); // Return deferred errors if we're inside a Deferred errorElement - if (deferredValue && isDeferredError(deferredValue)) { + if (deferredValue && deferredValue instanceof Error) { return deferredValue; } From 20e3add4fd2ebfd96b35b19929e55eeb02be1917 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Mon, 18 Jul 2022 16:52:39 -0400 Subject: [PATCH 05/14] internal renames --- packages/react-router/lib/components.tsx | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/packages/react-router/lib/components.tsx b/packages/react-router/lib/components.tsx index 5eced6319b..29f2528073 100644 --- a/packages/react-router/lib/components.tsx +++ b/packages/react-router/lib/components.tsx @@ -453,7 +453,7 @@ export function Deferred({ return ( - + {typeof children === "function" ? ( + ); @@ -515,18 +515,13 @@ class DeferredErrorBoundary extends React.Component< } } -interface DeferredWrapperProps { - children: React.ReactNode; - errorElement?: React.ReactNode; -} - /** * @private * Internal wrapper to handle re-throwing the promise to trigger the Suspense * fallback, or rendering the children/errorElement once the promise resolves * or rejects */ -function DeferredWrapper({ children }: DeferredWrapperProps) { +function DeferredThrower({ children }: { children: React.ReactNode }) { let value = React.useContext(DeferredContext); if (value instanceof Promise) { // throw to the suspense boundary @@ -536,15 +531,15 @@ function DeferredWrapper({ children }: DeferredWrapperProps) { return <>{children}; } -interface ResolveDeferredProps { - children: DeferredResolveRenderFunction; -} - /** * @private * Indirection to leverage useDeferredData for a render-prop API on */ -function ResolveDeferred({ children }: ResolveDeferredProps) { +function ResolveDeferred({ + children, +}: { + children: DeferredResolveRenderFunction; +}) { return children(useDeferredData()); } From d51e0ae2bc21c97a10371764c5c4ffc5415c3279 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Mon, 18 Jul 2022 17:16:59 -0400 Subject: [PATCH 06/14] do not handle fallback render errors in deferred error boundary --- .../__tests__/DataMemoryRouter-test.tsx | 85 ++++++++++++++++++- packages/react-router/lib/components.tsx | 47 ++++------ 2 files changed, 101 insertions(+), 31 deletions(-) diff --git a/packages/react-router/__tests__/DataMemoryRouter-test.tsx b/packages/react-router/__tests__/DataMemoryRouter-test.tsx index 6558237d21..15f1bbb62b 100644 --- a/packages/react-router/__tests__/DataMemoryRouter-test.tsx +++ b/packages/react-router/__tests__/DataMemoryRouter-test.tsx @@ -1893,7 +1893,8 @@ describe("", () => { function setupDeferredTest( hasRouteErrorElement = false, hasDeferredErrorElement = false, - triggerRenderError = false + triggerRenderError = false, + triggerFallbackError = false ) { let barDefer = defer(); let { container } = render( @@ -1931,7 +1932,7 @@ describe("", () => {

{data.critical}

Loading...

} + fallback={} errorElement={hasDeferredErrorElement ? : null} > @@ -1945,6 +1946,14 @@ describe("", () => { return

Route Error:{error.message}

; } + function LazyFallback() { + return triggerFallbackError ? ( +

{oops.i.did.it}

+ ) : ( +

Loading...

+ ); + } + function LazyData() { let data = useDeferredData(); return triggerRenderError ? ( @@ -2408,6 +2417,78 @@ describe("", () => { " `); }); + + it("does not handle fallback render errors in the Deferred errorElement", async () => { + let { barDefer, container } = setupDeferredTest(true, true, true, true); + + fireEvent.click(screen.getByText("Link to Bar")); + expect(getHtml(container)).toMatchInlineSnapshot(` + "
+
+ + Link to Bar + +

+ loading +

+

+ Foo +

+
+
" + `); + + let barValueDfd = defer(); + barDefer.resolve( + deferred({ + critical: "CRITICAL", + lazy: barValueDfd.promise, + }) + ); + await waitFor(() => screen.getByText("idle")); + expect(getHtml(container)).toMatchInlineSnapshot(` + "
+
+ + Link to Bar + +

+ idle +

+

+ Route Error: + oops is not defined +

+
+
" + `); + + // Resolving doesn't do anything + barValueDfd.resolve("LAZY"); + await new Promise((r) => setTimeout(r, 1)); + expect(getHtml(container)).toMatchInlineSnapshot(` + "
+
+ + Link to Bar + +

+ idle +

+

+ Route Error: + oops is not defined +

+
+
" + `); + }); }); }); diff --git a/packages/react-router/lib/components.tsx b/packages/react-router/lib/components.tsx index 29f2528073..aba362a1c7 100644 --- a/packages/react-router/lib/components.tsx +++ b/packages/react-router/lib/components.tsx @@ -451,19 +451,17 @@ export function Deferred({ errorElement, }: DeferredProps) { return ( - - - - {typeof children === "function" ? ( - - ) : ( - children - )} - - - + + + {typeof children === "function" ? ( + + ) : ( + children + )} + + ); } @@ -504,6 +502,7 @@ class DeferredErrorBoundary extends React.Component< if (error) { if (errorElement) { + // We have a Deferred errorElement, so provide our error and render it return ( ); @@ -511,24 +510,14 @@ class DeferredErrorBoundary extends React.Component< // Throw to ancestor route-level error boundary throw error; } - return ; - } -} -/** - * @private - * Internal wrapper to handle re-throwing the promise to trigger the Suspense - * fallback, or rendering the children/errorElement once the promise resolves - * or rejects - */ -function DeferredThrower({ children }: { children: React.ReactNode }) { - let value = React.useContext(DeferredContext); - if (value instanceof Promise) { - // throw to the suspense boundary - throw value; - } + if (value instanceof Promise) { + // Throw to the suspense boundary + throw value; + } - return <>{children}; + return ; + } } /** From f323d8763af15d46ddc6a93c6c3e03689fa2020d Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Tue, 19 Jul 2022 09:29:41 -0400 Subject: [PATCH 07/14] Remove Suspense from inside Deferred --- examples/data-router/src/App.tsx | 56 +++++++++++-------- .../__tests__/DataMemoryRouter-test.tsx | 15 ++--- packages/react-router/lib/components.tsx | 36 +++++------- 3 files changed, 56 insertions(+), 51 deletions(-) diff --git a/examples/data-router/src/App.tsx b/examples/data-router/src/App.tsx index 57d27e5cf2..f9b8f6ccaf 100644 --- a/examples/data-router/src/App.tsx +++ b/examples/data-router/src/App.tsx @@ -269,25 +269,35 @@ function DeferredPage() {

{data.critical1}

{data.critical2}

- should not see me!

}> - - - loading 1...

}> - - - loading 2...

}> - - - loading 3...

}> - {(data) =>

{data}

} - - loading (error)...

} - errorElement={} - > - - + + should not see me!

}> + + + + + + loading 1...

}> + + + + + + loading 2...

}> + + + + + + loading 3...

}> + {(data) =>

{data}

} + + + loading (error)...

}> + }> + + + +
); @@ -310,9 +320,11 @@ function DeferredChild() { return (

{data.critical}

- loading child...

}> - - + loading child...

}> + + + +