From bc00b05c7822f12fca43fc53560baa06b628aa69 Mon Sep 17 00:00:00 2001 From: Jacob Ebey Date: Mon, 19 Dec 2022 18:15:06 -0800 Subject: [PATCH 01/10] initial attempt at exposing required data to ssr implementations --- packages/router/__tests__/router-test.ts | 107 +++++++++++++++++++++-- packages/router/router.ts | 54 +++++++++--- packages/router/utils.ts | 18 +++- 3 files changed, 154 insertions(+), 25 deletions(-) diff --git a/packages/router/__tests__/router-test.ts b/packages/router/__tests__/router-test.ts index 1e2a1d0a0c..b233218809 100644 --- a/packages/router/__tests__/router-test.ts +++ b/packages/router/__tests__/router-test.ts @@ -16,6 +16,7 @@ import { createRouter, createStaticHandler, defer, + DEFERRED_SYMBOL, ErrorResponse, IDLE_FETCHER, IDLE_NAVIGATION, @@ -30,6 +31,7 @@ import type { AgnosticIndexRouteObject, AgnosticNonIndexRouteObject, AgnosticRouteObject, + DeferredData, TrackedPromise, } from "../utils"; import { @@ -157,8 +159,9 @@ function isRedirect(result: any) { ); } -interface CustomMatchers { +interface CustomMatchers { trackedPromise(data?: any, error?: any, aborted?: boolean): R; + deferredData(done: boolean, status?: number): R; } declare global { @@ -175,6 +178,27 @@ declare global { // - expect.trackedPromise(null, error) => promise rejected with `error` // - expect.trackedPromise(null, null, true) => promise aborted expect.extend({ + deferredData(received, done, status = 200) { + let deferredData = received as DeferredData; + + return { + message: () => + `expected done to be ${ + done ? "true" : "false" + } and status to be ${status}, instead got done: ${ + deferredData.done ? "true" : "false" + } and status: ${ + typeof deferredData.responseInit?.status === "number" + ? deferredData.responseInit.status + : 200 + }`, + pass: + deferredData.done === done && + (typeof deferredData.responseInit?.status === "number" + ? deferredData.responseInit.status + : 200) === status, + }; + }, trackedPromise(received, data, error, aborted = false) { let promise = received as TrackedPromise; let isTrackedPromise = @@ -10899,8 +10923,7 @@ describe("a router", () => { }); }); - // Note: this is only until we wire up the remix streaming - it("should abort deferred data on load navigations (for now)", async () => { + it("should support document load navigations returning deferred", async () => { let { query } = createStaticHandler(SSR_ROUTES); let context = await query(createRequest("/parent/deferred")); expect(context).toMatchObject({ @@ -10909,19 +10932,29 @@ describe("a router", () => { parent: "PARENT LOADER", deferred: { critical: "loader", - lazy: expect.trackedPromise(null, null, true), + lazy: expect.trackedPromise(), }, }, + activeDeferreds: { + deferred: expect.deferredData(false), + }, errors: null, location: { pathname: "/parent/deferred" }, matches: [{ route: { id: "parent" } }, { route: { id: "deferred" } }], }); await new Promise((r) => setTimeout(r, 10)); - expect( - (context as StaticHandlerContext).loaderData.deferred.lazy instanceof - Promise - ).toBe(true); + + expect(context).toMatchObject({ + loaderData: { + deferred: { + lazy: expect.trackedPromise("lazy"), + }, + }, + activeDeferreds: { + deferred: expect.deferredData(true), + }, + }); }); it("should support document submit navigations", async () => { @@ -12448,6 +12481,64 @@ describe("a router", () => { expect(arg(actionStub).context.sessionId).toBe("12345"); }); + describe("deferred", () => { + let { queryRoute } = createStaticHandler([ + { + id: "deferred", + path: "/deferred", + loader: () => { + return defer({ + critical: "loader", + lazy: new Promise((r) => setTimeout(() => r("lazy"), 10)), + }); + }, + }, + { + id: "deferred-status", + path: "/deferred-status", + loader: () => { + return defer( + { + critical: "loader", + lazy: new Promise((r) => setTimeout(() => r("lazy"), 10)), + }, + 201 + ); + }, + }, + ]); + + it("should return DeferredData on symbol", async () => { + let result = await queryRoute(createRequest("/deferred")); + expect(result).toMatchObject({ + critical: "loader", + lazy: expect.trackedPromise(), + }); + expect(result[DEFERRED_SYMBOL]).deferredData(false); + await new Promise((r) => setTimeout(r, 10)); + expect(result).toMatchObject({ + critical: "loader", + lazy: expect.trackedPromise("lazy"), + }); + expect(result[DEFERRED_SYMBOL]).deferredData(true); + }); + + it("should return DeferredData on symbol with status", async () => { + let result = await queryRoute(createRequest("/deferred-status")); + expect(result).toMatchObject({ + critical: "loader", + lazy: expect.trackedPromise(), + }); + expect(result[DEFERRED_SYMBOL]).deferredData(false, 201); + await new Promise((r) => setTimeout(r, 10)); + expect(result).toMatchObject({ + critical: "loader", + lazy: expect.trackedPromise("lazy"), + }); + expect(result[DEFERRED_SYMBOL]).deferredData(true, 201); + }); + }); + describe("Errors with Status Codes", () => { /* eslint-disable jest/no-conditional-expect */ let { queryRoute } = createStaticHandler([ diff --git a/packages/router/router.ts b/packages/router/router.ts index 8b05e6521d..5c6f93afd2 100644 --- a/packages/router/router.ts +++ b/packages/router/router.ts @@ -308,6 +308,7 @@ export interface StaticHandlerContext { statusCode: number; loaderHeaders: Record; actionHeaders: Record; + activeDeferreds: Record | null; _deepestRenderedBoundaryId?: string | null; } @@ -1998,6 +1999,7 @@ export function createRouter(init: RouterInit): Router { //#region createStaticHandler //////////////////////////////////////////////////////////////////////////////// +export const DEFERRED_SYMBOL = Symbol("deferred"); export function createStaticHandler( routes: AgnosticRouteObject[], opts?: { @@ -2057,6 +2059,7 @@ export function createStaticHandler( statusCode: error.status, loaderHeaders: {}, actionHeaders: {}, + activeDeferreds: null, }; } else if (!matches) { let error = getInternalRouterError(404, { pathname: location.pathname }); @@ -2074,6 +2077,7 @@ export function createStaticHandler( statusCode: error.status, loaderHeaders: {}, actionHeaders: {}, + activeDeferreds: null, }; } @@ -2163,7 +2167,19 @@ export function createStaticHandler( // Pick off the right state value to return let routeData = [result.actionData, result.loaderData].find((v) => v); - return Object.values(routeData || {})[0]; + let data = Object.values(routeData || {})[0]; + + if ( + data === Object.values(result.loaderData || {})[0] && + result.activeDeferreds && + result.activeDeferreds[match.route.id] + ) { + Object.assign(data, { + [DEFERRED_SYMBOL]: result.activeDeferreds[match.route.id], + }); + } + + return data; } async function queryImpl( @@ -2296,6 +2312,7 @@ export function createStaticHandler( statusCode: 200, loaderHeaders: {}, actionHeaders: {}, + activeDeferreds: null, }; } @@ -2391,6 +2408,7 @@ export function createStaticHandler( errors: pendingActionError || null, statusCode: 200, loaderHeaders: {}, + activeDeferreds: null, }; } @@ -2414,22 +2432,18 @@ export function createStaticHandler( throw new Error(`${method}() call aborted`); } - let executedLoaders = new Set(); - results.forEach((result, i) => { - executedLoaders.add(matchesToLoad[i].route.id); - // Can't do anything with these without the Remix side of things, so just - // cancel them for now - if (isDeferredResult(result)) { - result.deferredData.cancel(); - } - }); - + let activeDeferreds = new Map(); // Process and commit output from loaders let context = processRouteLoaderData( matches, matchesToLoad, results, - pendingActionError + pendingActionError, + activeDeferreds + ); + + let executedLoaders = new Set( + matchesToLoad.map((match) => match.route.id) ); // Add a null for any non-loader matches for proper revalidation on the client @@ -2442,6 +2456,10 @@ export function createStaticHandler( return { ...context, matches, + activeDeferreds: + activeDeferreds.size > 0 + ? Object.fromEntries(activeDeferreds.entries()) + : null, }; } @@ -2962,7 +2980,17 @@ function processRouteLoaderData( } else if (isDeferredResult(result)) { activeDeferreds && activeDeferreds.set(id, result.deferredData); loaderData[id] = result.deferredData.data; - // TODO: Add statusCode/headers once we wire up streaming in Remix + let init = result.deferredData.responseInit; + if (init) { + // Error status codes always override success status codes, but if all + // loaders are successful we take the deepest status code. + if (init.status != null && init.status !== 200 && !foundError) { + statusCode = init.status; + } + if (init.headers) { + loaderHeaders[id] = new Headers(init.headers); + } + } } else { loaderData[id] = result.data; // Error status codes always override success status codes, but if all diff --git a/packages/router/utils.ts b/packages/router/utils.ts index 84c8b2e900..0a5de97fd5 100644 --- a/packages/router/utils.ts +++ b/packages/router/utils.ts @@ -1137,13 +1137,16 @@ export class DeferredData { private unlistenAbortSignal: () => void; private subscriber?: (aborted: boolean) => void = undefined; data: Record; + responseInit?: ResponseInit; - constructor(data: Record) { + constructor(data: Record, responseInit?: ResponseInit) { invariant( data && typeof data === "object" && !Array.isArray(data), "defer() only accepts plain objects" ); + this.responseInit = responseInit; + // Set up an AbortController + Promise we can race against to exit early // cancellation let reject: (e: AbortedDeferredError) => void; @@ -1288,9 +1291,16 @@ function unwrapTrackedPromise(value: any) { return value._data; } -export function defer(data: Record) { - return new DeferredData(data); -} +export type DeferFunction = ( + data: Record, + init?: number | ResponseInit +) => DeferredData; + +export const defer: DeferFunction = (data, init = 200) => { + let responseInit = typeof init === "number" ? { status: init } : init; + + return new DeferredData(data, responseInit); +}; export type RedirectFunction = ( url: string, From d1ffd4af01fe53961aa889ff95512829429002c0 Mon Sep 17 00:00:00 2001 From: Jacob Ebey Date: Mon, 19 Dec 2022 19:33:42 -0800 Subject: [PATCH 02/10] - export DeferredData class - add settledKey to DeferredData subscription callback --- packages/router/index.ts | 1 + packages/router/utils.ts | 13 +++++++------ 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/packages/router/index.ts b/packages/router/index.ts index 23bfca95b1..1b1b15a1e1 100644 --- a/packages/router/index.ts +++ b/packages/router/index.ts @@ -30,6 +30,7 @@ export { AbortedDeferredError, ErrorResponse, defer, + DeferredData, generatePath, getToPathname, isRouteErrorResponse, diff --git a/packages/router/utils.ts b/packages/router/utils.ts index 0a5de97fd5..be1327da78 100644 --- a/packages/router/utils.ts +++ b/packages/router/utils.ts @@ -1135,7 +1135,8 @@ export class DeferredData { private controller: AbortController; private abortPromise: Promise; private unlistenAbortSignal: () => void; - private subscriber?: (aborted: boolean) => void = undefined; + private subscriber?: (aborted: boolean, settledKey?: string) => void = + undefined; data: Record; responseInit?: ResponseInit; @@ -1168,7 +1169,7 @@ export class DeferredData { } private trackPromise( - key: string | number, + key: string, value: Promise | unknown ): TrackedPromise | unknown { if (!(value instanceof Promise)) { @@ -1194,7 +1195,7 @@ export class DeferredData { private onSettle( promise: TrackedPromise, - key: string | number, + key: string, error: unknown, data?: unknown ): unknown { @@ -1217,16 +1218,16 @@ export class DeferredData { const subscriber = this.subscriber; if (error) { Object.defineProperty(promise, "_error", { get: () => error }); - subscriber && subscriber(false); + subscriber && subscriber(false, key); return Promise.reject(error); } Object.defineProperty(promise, "_data", { get: () => data }); - subscriber && subscriber(false); + subscriber && subscriber(false, key); return data; } - subscribe(fn: (aborted: boolean) => void) { + subscribe(fn: (aborted: boolean, settledKey?: string) => void) { this.subscriber = fn; } From 7dff110599d2291ee7947e09028e4ca733b5a1ba Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Tue, 20 Dec 2022 12:52:02 -0500 Subject: [PATCH 03/10] minor updates --- packages/router/__tests__/router-test.ts | 271 ++++++++++++++++++----- packages/router/router.ts | 48 ++-- packages/router/utils.ts | 16 +- 3 files changed, 256 insertions(+), 79 deletions(-) diff --git a/packages/router/__tests__/router-test.ts b/packages/router/__tests__/router-test.ts index b233218809..f8f5f192f1 100644 --- a/packages/router/__tests__/router-test.ts +++ b/packages/router/__tests__/router-test.ts @@ -161,7 +161,11 @@ function isRedirect(result: any) { interface CustomMatchers { trackedPromise(data?: any, error?: any, aborted?: boolean): R; - deferredData(done: boolean, status?: number): R; + deferredData( + done: boolean, + status?: number, + headers?: Record + ): R; } declare global { @@ -172,33 +176,39 @@ declare global { } } -// Custom matcher for asserting deferred promise results inside of `toEqual()` -// - expect.trackedPromise() => pending promise -// - expect.trackedPromise(value) => promise resolved with `value` -// - expect.trackedPromise(null, error) => promise rejected with `error` -// - expect.trackedPromise(null, null, true) => promise aborted expect.extend({ - deferredData(received, done, status = 200) { + // Custom matcher for asserting deferred promise results for static handler + // - expect(val).deferredData(false) => Unresolved promise + // - expect(val).deferredData(false) => Resolved promise + // - expect(val).deferredData(false, 201, { 'x-custom': 'yes' }) + // => Unresolved promise with status + headers + // - expect(val).deferredData(true, 201, { 'x-custom': 'yes' }) + // => Resolved promise with status + headers + deferredData(received, done, status = 200, headers = {}) { let deferredData = received as DeferredData; return { message: () => - `expected done to be ${ - done ? "true" : "false" - } and status to be ${status}, instead got done: ${ - deferredData.done ? "true" : "false" - } and status: ${ - typeof deferredData.responseInit?.status === "number" - ? deferredData.responseInit.status - : 200 - }`, + `expected done=${String( + done + )}/status=${status}/headers=${JSON.stringify(headers)}, ` + + `instead got done=${String(deferredData.done)}/status=${ + deferredData.statusCode + }/headers=${JSON.stringify( + Object.fromEntries(deferredData.headers.entries()) + )}`, pass: deferredData.done === done && - (typeof deferredData.responseInit?.status === "number" - ? deferredData.responseInit.status - : 200) === status, + deferredData.statusCode === status && + JSON.stringify(Object.fromEntries(deferredData.headers.entries())) === + JSON.stringify(headers), }; }, + // Custom matcher for asserting deferred promise results inside of `toEqual()` + // - expect.trackedPromise() => pending promise + // - expect.trackedPromise(value) => promise resolved with `value` + // - expect.trackedPromise(null, error) => promise rejected with `error` + // - expect.trackedPromise(null, null, true) => promise aborted trackedPromise(received, data, error, aborted = false) { let promise = received as TrackedPromise; let isTrackedPromise = @@ -10759,14 +10769,32 @@ describe("a router", () => { { id: "deferred", path: "deferred", - loader: () => - defer({ + loader: ({ request }) => { + if (new URL(request.url).searchParams.has("reject")) { + return defer({ + critical: "loader", + lazy: new Promise((_, r) => + setTimeout(() => r(new Error("broken!")), 10) + ), + }); + } + if (new URL(request.url).searchParams.has("status")) { + return defer( + { + critical: "loader", + lazy: new Promise((r) => setTimeout(() => r("lazy"), 10)), + }, + { status: 201, headers: { "X-Custom": "yes" } } + ); + } + return defer({ critical: "loader", lazy: new Promise((r) => setTimeout(() => r("lazy"), 10)), - }), + }); + }, action: () => defer({ - critical: "action", + critical: "critical", lazy: new Promise((r) => setTimeout(() => r("lazy"), 10)), }), }, @@ -11505,6 +11533,127 @@ describe("a router", () => { expect(arg(childStub).context.sessionId).toBe("12345"); }); + describe("deferred", () => { + let { query } = createStaticHandler(SSR_ROUTES); + + it("should return DeferredData on symbol", async () => { + let context = (await query( + createRequest("/parent/deferred") + )) as StaticHandlerContext; + expect(context).toMatchObject({ + loaderData: { + parent: "PARENT LOADER", + deferred: { + critical: "loader", + lazy: expect.trackedPromise(), + }, + }, + activeDeferreds: { + deferred: expect.deferredData(false), + }, + }); + await new Promise((r) => setTimeout(r, 10)); + expect(context).toMatchObject({ + loaderData: { + parent: "PARENT LOADER", + deferred: { + critical: "loader", + lazy: expect.trackedPromise("lazy"), + }, + }, + activeDeferreds: { + deferred: expect.deferredData(true), + }, + }); + }); + + it("should return rejected DeferredData on symbol", async () => { + let context = (await query( + createRequest("/parent/deferred?reject") + )) as StaticHandlerContext; + expect(context).toMatchObject({ + loaderData: { + parent: "PARENT LOADER", + deferred: { + critical: "loader", + lazy: expect.trackedPromise(), + }, + }, + activeDeferreds: { + deferred: expect.deferredData(false), + }, + }); + await new Promise((r) => setTimeout(r, 10)); + expect(context).toMatchObject({ + loaderData: { + parent: "PARENT LOADER", + deferred: { + critical: "loader", + lazy: expect.trackedPromise(undefined, new Error("broken!")), + }, + }, + activeDeferreds: { + deferred: expect.deferredData(true), + }, + }); + }); + + it("should return DeferredData on symbol with status + headers", async () => { + let context = (await query( + createRequest("/parent/deferred?status") + )) as StaticHandlerContext; + expect(context).toMatchObject({ + loaderData: { + parent: "PARENT LOADER", + deferred: { + critical: "loader", + lazy: expect.trackedPromise(), + }, + }, + activeDeferreds: { + deferred: expect.deferredData(false, 201, { + "x-custom": "yes", + }), + }, + }); + await new Promise((r) => setTimeout(r, 10)); + expect(context).toMatchObject({ + loaderData: { + parent: "PARENT LOADER", + deferred: { + critical: "loader", + lazy: expect.trackedPromise("lazy"), + }, + }, + activeDeferreds: { + deferred: expect.deferredData(true, 201, { + "x-custom": "yes", + }), + }, + }); + }); + + it("does not support deferred on submissions", async () => { + let context = (await query( + createSubmitRequest("/parent/deferred") + )) as StaticHandlerContext; + expect(context.actionData).toEqual(null); + expect(context.loaderData).toEqual({ + parent: null, + deferred: null, + }); + expect(context.activeDeferreds).toEqual(null); + expect(context.errors).toEqual({ + parent: new ErrorResponse( + 400, + "Bad Request", + new Error("defer() is not supported in actions"), + true + ), + }); + }); + }); + describe("statusCode", () => { it("should expose a 200 status code by default", async () => { let { query } = createStaticHandler([ @@ -12482,34 +12631,10 @@ describe("a router", () => { }); describe("deferred", () => { - let { queryRoute } = createStaticHandler([ - { - id: "deferred", - path: "/deferred", - loader: () => { - return defer({ - critical: "loader", - lazy: new Promise((r) => setTimeout(() => r("lazy"), 10)), - }); - }, - }, - { - id: "deferred-status", - path: "/deferred-status", - loader: () => { - return defer( - { - critical: "loader", - lazy: new Promise((r) => setTimeout(() => r("lazy"), 10)), - }, - 201 - ); - }, - }, - ]); + let { queryRoute } = createStaticHandler(SSR_ROUTES); it("should return DeferredData on symbol", async () => { - let result = await queryRoute(createRequest("/deferred")); + let result = await queryRoute(createRequest("/parent/deferred")); expect(result).toMatchObject({ critical: "loader", lazy: expect.trackedPromise(), @@ -12523,19 +12648,59 @@ describe("a router", () => { expect(result[DEFERRED_SYMBOL]).deferredData(true); }); - it("should return DeferredData on symbol with status", async () => { - let result = await queryRoute(createRequest("/deferred-status")); + it("should return rejected DeferredData on symbol", async () => { + let result = await queryRoute( + createRequest("/parent/deferred?reject") + ); expect(result).toMatchObject({ critical: "loader", lazy: expect.trackedPromise(), }); - expect(result[DEFERRED_SYMBOL]).deferredData(false, 201); + expect(result[DEFERRED_SYMBOL]).deferredData(false); + await new Promise((r) => setTimeout(r, 10)); + expect(result).toMatchObject({ + critical: "loader", + lazy: expect.trackedPromise(null, new Error("broken!")), + }); + expect(result[DEFERRED_SYMBOL]).deferredData(true); + }); + + it("should return DeferredData on symbol with status + headers", async () => { + let result = await queryRoute( + createRequest("/parent/deferred?status") + ); + expect(result).toMatchObject({ + critical: "loader", + lazy: expect.trackedPromise(), + }); + expect(result[DEFERRED_SYMBOL]).deferredData(false, 201, { + "x-custom": "yes", + }); await new Promise((r) => setTimeout(r, 10)); expect(result).toMatchObject({ critical: "loader", lazy: expect.trackedPromise("lazy"), }); - expect(result[DEFERRED_SYMBOL]).deferredData(true, 201); + expect(result[DEFERRED_SYMBOL]).deferredData(true, 201, { + "x-custom": "yes", + }); + }); + + it("does not support deferred on submissions", async () => { + try { + await queryRoute(createSubmitRequest("/parent/deferred")); + expect(false).toBe(true); + } catch (e) { + // eslint-disable-next-line jest/no-conditional-expect + expect(e).toEqual( + new ErrorResponse( + 400, + "Bad Request", + new Error("defer() is not supported in actions"), + true + ) + ); + } }); }); diff --git a/packages/router/router.ts b/packages/router/router.ts index 5c6f93afd2..ef9e25714e 100644 --- a/packages/router/router.ts +++ b/packages/router/router.ts @@ -1118,7 +1118,7 @@ export function createRouter(init: RouterInit): Router { } if (isDeferredResult(result)) { - throw new Error("defer() is not supported in actions"); + throw getInternalRouterError(400, { type: "defer-action" }); } return { @@ -1412,7 +1412,7 @@ export function createRouter(init: RouterInit): Router { } if (isDeferredResult(actionResult)) { - invariant(false, "defer() is not supported in actions"); + throw getInternalRouterError(400, { type: "defer-action" }); } // Start the data load for current matches, or the next location if we're @@ -1594,7 +1594,7 @@ export function createRouter(init: RouterInit): Router { router.basename ); - // Deferred isn't supported or fetcher loads, await everything and treat it + // Deferred isn't supported for fetcher loads, await everything and treat it // as a normal load. resolveDeferredData will return undefined if this // fetcher gets aborted, so we just leave result untouched and short circuit // below if that happens @@ -2000,6 +2000,7 @@ export function createRouter(init: RouterInit): Router { //////////////////////////////////////////////////////////////////////////////// export const DEFERRED_SYMBOL = Symbol("deferred"); + export function createStaticHandler( routes: AgnosticRouteObject[], opts?: { @@ -2292,7 +2293,14 @@ export function createStaticHandler( } if (isDeferredResult(result)) { - throw new Error("defer() is not supported in actions"); + let error = getInternalRouterError(400, { type: "defer-action" }); + if (isRouteRequest) { + throw error; + } + result = { + type: ResultType.error, + error, + }; } if (isRouteRequest) { @@ -2922,7 +2930,7 @@ function processRouteLoaderData( matchesToLoad: AgnosticDataRouteMatch[], results: DataResult[], pendingError: RouteData | undefined, - activeDeferreds?: Map + activeDeferreds: Map ): { loaderData: RouterState["loaderData"]; errors: RouterState["errors"] | null; @@ -2977,22 +2985,14 @@ function processRouteLoaderData( if (result.headers) { loaderHeaders[id] = result.headers; } - } else if (isDeferredResult(result)) { - activeDeferreds && activeDeferreds.set(id, result.deferredData); - loaderData[id] = result.deferredData.data; - let init = result.deferredData.responseInit; - if (init) { - // Error status codes always override success status codes, but if all - // loaders are successful we take the deepest status code. - if (init.status != null && init.status !== 200 && !foundError) { - statusCode = init.status; - } - if (init.headers) { - loaderHeaders[id] = new Headers(init.headers); - } - } } else { - loaderData[id] = result.data; + if (isDeferredResult(result)) { + activeDeferreds.set(id, result.deferredData); + loaderData[id] = result.deferredData.data; + } else { + loaderData[id] = result.data; + } + // Error status codes always override success status codes, but if all // loaders are successful we take the deepest status code. if ( @@ -3067,11 +3067,11 @@ function processLoaderData( } else if (isRedirectResult(result)) { // Should never get here, redirects should get processed above, but we // keep this to type narrow to a success result in the else - throw new Error("Unhandled fetcher revalidation redirect"); + invariant(false, "Unhandled fetcher revalidation redirect"); } else if (isDeferredResult(result)) { // Should never get here, deferred data should be awaited for fetchers // in resolveDeferredResults - throw new Error("Unhandled fetcher deferred data"); + invariant(false, "Unhandled fetcher deferred data"); } else { let doneFetcher: FetcherStates["Idle"] = { state: "idle", @@ -3162,10 +3162,12 @@ function getInternalRouterError( pathname, routeId, method, + type, }: { pathname?: string; routeId?: string; method?: string; + type?: "defer-action"; } = {} ) { let statusText = "Unknown Server Error"; @@ -3178,6 +3180,8 @@ function getInternalRouterError( `You made a ${method} request to "${pathname}" but ` + `did not provide a \`loader\` for route "${routeId}", ` + `so there is no way to handle the request.`; + } else if (type === "defer-action") { + errorMessage = "defer() is not supported in actions"; } else { errorMessage = "Cannot submit binary form data using GET"; } diff --git a/packages/router/utils.ts b/packages/router/utils.ts index be1327da78..0e9d3ca55b 100644 --- a/packages/router/utils.ts +++ b/packages/router/utils.ts @@ -31,6 +31,8 @@ export interface SuccessResult { export interface DeferredResult { type: ResultType.deferred; deferredData: DeferredData; + statusCode?: number; + headers?: Headers; } /** @@ -1138,7 +1140,8 @@ export class DeferredData { private subscriber?: (aborted: boolean, settledKey?: string) => void = undefined; data: Record; - responseInit?: ResponseInit; + statusCode: number; + headers: Headers; constructor(data: Record, responseInit?: ResponseInit) { invariant( @@ -1146,8 +1149,6 @@ export class DeferredData { "defer() only accepts plain objects" ); - this.responseInit = responseInit; - // Set up an AbortController + Promise we can race against to exit early // cancellation let reject: (e: AbortedDeferredError) => void; @@ -1166,6 +1167,13 @@ export class DeferredData { }), {} ); + + this.statusCode = + responseInit && responseInit.status ? responseInit.status : 200; + this.headers = + responseInit && responseInit.status + ? new Headers(responseInit.headers) + : new Headers(); } private trackPromise( @@ -1297,7 +1305,7 @@ export type DeferFunction = ( init?: number | ResponseInit ) => DeferredData; -export const defer: DeferFunction = (data, init = 200) => { +export const defer: DeferFunction = (data, init = {}) => { let responseInit = typeof init === "number" ? { status: init } : init; return new DeferredData(data, responseInit); From 31810b29f36776b1966754c377d9b9263001d290 Mon Sep 17 00:00:00 2001 From: Jacob Ebey Date: Tue, 20 Dec 2022 19:20:17 -0800 Subject: [PATCH 04/10] chore: expose pendingKeys and deferredKeys as public API on DeferredData --- packages/router/utils.ts | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/packages/router/utils.ts b/packages/router/utils.ts index 0e9d3ca55b..63fa48783d 100644 --- a/packages/router/utils.ts +++ b/packages/router/utils.ts @@ -1133,7 +1133,7 @@ export interface TrackedPromise extends Promise { export class AbortedDeferredError extends Error {} export class DeferredData { - private pendingKeys: Set = new Set(); + private pendingKeysSet: Set = new Set(); private controller: AbortController; private abortPromise: Promise; private unlistenAbortSignal: () => void; @@ -1142,6 +1142,7 @@ export class DeferredData { data: Record; statusCode: number; headers: Headers; + deferredKeys: string[] = []; constructor(data: Record, responseInit?: ResponseInit) { invariant( @@ -1184,7 +1185,8 @@ export class DeferredData { return value; } - this.pendingKeys.add(key); + this.deferredKeys.push(key); + this.pendingKeysSet.add(key); // We store a little wrapper promise that will be extended with // _data/_error props upon resolve/reject @@ -1216,7 +1218,7 @@ export class DeferredData { return Promise.reject(error); } - this.pendingKeys.delete(key); + this.pendingKeysSet.delete(key); if (this.done) { // Nothing left to abort! @@ -1241,7 +1243,7 @@ export class DeferredData { cancel() { this.controller.abort(); - this.pendingKeys.forEach((v, k) => this.pendingKeys.delete(k)); + this.pendingKeysSet.forEach((v, k) => this.pendingKeysSet.delete(k)); let subscriber = this.subscriber; subscriber && subscriber(true); } @@ -1264,7 +1266,7 @@ export class DeferredData { } get done() { - return this.pendingKeys.size === 0; + return this.pendingKeysSet.size === 0; } get unwrappedData() { @@ -1281,6 +1283,10 @@ export class DeferredData { {} ); } + + get pendingKeys() { + return Array.from(this.pendingKeysSet); + } } function isTrackedPromise(value: any): value is TrackedPromise { From 8a62a4002a668340d3f34fd5bc7f2d51fc4f0731 Mon Sep 17 00:00:00 2001 From: Jacob Ebey Date: Thu, 22 Dec 2022 14:56:00 -0800 Subject: [PATCH 05/10] fix: allow multiple DeferredData subscriptions fix: allow unsubscribe from DeferredData --- packages/router/utils.ts | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/packages/router/utils.ts b/packages/router/utils.ts index 63fa48783d..a360449b91 100644 --- a/packages/router/utils.ts +++ b/packages/router/utils.ts @@ -1137,8 +1137,8 @@ export class DeferredData { private controller: AbortController; private abortPromise: Promise; private unlistenAbortSignal: () => void; - private subscriber?: (aborted: boolean, settledKey?: string) => void = - undefined; + private subscribers: Set<(aborted: boolean, settledKey?: string) => void> = + new Set(); data: Record; statusCode: number; headers: Headers; @@ -1225,27 +1225,30 @@ export class DeferredData { this.unlistenAbortSignal(); } - const subscriber = this.subscriber; if (error) { Object.defineProperty(promise, "_error", { get: () => error }); - subscriber && subscriber(false, key); + this.emit(false, key); return Promise.reject(error); } Object.defineProperty(promise, "_data", { get: () => data }); - subscriber && subscriber(false, key); + this.emit(false, key); return data; } + private emit(aborted: boolean, settledKey?: string) { + this.subscribers.forEach((subscriber) => subscriber(aborted, settledKey)); + } + subscribe(fn: (aborted: boolean, settledKey?: string) => void) { - this.subscriber = fn; + this.subscribers.add(fn); + return () => this.subscribers.delete(fn); } cancel() { this.controller.abort(); this.pendingKeysSet.forEach((v, k) => this.pendingKeysSet.delete(k)); - let subscriber = this.subscriber; - subscriber && subscriber(true); + this.emit(true); } async resolveData(signal: AbortSignal) { From 05cc767ab25d3c0a6eb7697b3663b8aeec49df9a Mon Sep 17 00:00:00 2001 From: Jacob Ebey Date: Fri, 23 Dec 2022 09:50:48 -0800 Subject: [PATCH 06/10] make API unsafe --- packages/router/__tests__/router-test.ts | 14 +++++++------- packages/router/index.ts | 2 +- packages/router/router.ts | 4 ++-- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/packages/router/__tests__/router-test.ts b/packages/router/__tests__/router-test.ts index f8f5f192f1..ab76130210 100644 --- a/packages/router/__tests__/router-test.ts +++ b/packages/router/__tests__/router-test.ts @@ -16,7 +16,7 @@ import { createRouter, createStaticHandler, defer, - DEFERRED_SYMBOL, + UNSAFE_DEFERRED_SYMBOL, ErrorResponse, IDLE_FETCHER, IDLE_NAVIGATION, @@ -12639,13 +12639,13 @@ describe("a router", () => { critical: "loader", lazy: expect.trackedPromise(), }); - expect(result[DEFERRED_SYMBOL]).deferredData(false); + expect(result[UNSAFE_DEFERRED_SYMBOL]).deferredData(false); await new Promise((r) => setTimeout(r, 10)); expect(result).toMatchObject({ critical: "loader", lazy: expect.trackedPromise("lazy"), }); - expect(result[DEFERRED_SYMBOL]).deferredData(true); + expect(result[UNSAFE_DEFERRED_SYMBOL]).deferredData(true); }); it("should return rejected DeferredData on symbol", async () => { @@ -12656,13 +12656,13 @@ describe("a router", () => { critical: "loader", lazy: expect.trackedPromise(), }); - expect(result[DEFERRED_SYMBOL]).deferredData(false); + expect(result[UNSAFE_DEFERRED_SYMBOL]).deferredData(false); await new Promise((r) => setTimeout(r, 10)); expect(result).toMatchObject({ critical: "loader", lazy: expect.trackedPromise(null, new Error("broken!")), }); - expect(result[DEFERRED_SYMBOL]).deferredData(true); + expect(result[UNSAFE_DEFERRED_SYMBOL]).deferredData(true); }); it("should return DeferredData on symbol with status + headers", async () => { @@ -12673,7 +12673,7 @@ describe("a router", () => { critical: "loader", lazy: expect.trackedPromise(), }); - expect(result[DEFERRED_SYMBOL]).deferredData(false, 201, { + expect(result[UNSAFE_DEFERRED_SYMBOL]).deferredData(false, 201, { "x-custom": "yes", }); await new Promise((r) => setTimeout(r, 10)); @@ -12681,7 +12681,7 @@ describe("a router", () => { critical: "loader", lazy: expect.trackedPromise("lazy"), }); - expect(result[DEFERRED_SYMBOL]).deferredData(true, 201, { + expect(result[UNSAFE_DEFERRED_SYMBOL]).deferredData(true, 201, { "x-custom": "yes", }); }); diff --git a/packages/router/index.ts b/packages/router/index.ts index 1b1b15a1e1..0d8fc1051d 100644 --- a/packages/router/index.ts +++ b/packages/router/index.ts @@ -30,7 +30,7 @@ export { AbortedDeferredError, ErrorResponse, defer, - DeferredData, + DeferredData as UNSAFE_DeferredData, generatePath, getToPathname, isRouteErrorResponse, diff --git a/packages/router/router.ts b/packages/router/router.ts index ef9e25714e..8c757828cd 100644 --- a/packages/router/router.ts +++ b/packages/router/router.ts @@ -1999,7 +1999,7 @@ export function createRouter(init: RouterInit): Router { //#region createStaticHandler //////////////////////////////////////////////////////////////////////////////// -export const DEFERRED_SYMBOL = Symbol("deferred"); +export const UNSAFE_DEFERRED_SYMBOL = Symbol("deferred"); export function createStaticHandler( routes: AgnosticRouteObject[], @@ -2176,7 +2176,7 @@ export function createStaticHandler( result.activeDeferreds[match.route.id] ) { Object.assign(data, { - [DEFERRED_SYMBOL]: result.activeDeferreds[match.route.id], + [UNSAFE_DEFERRED_SYMBOL]: result.activeDeferreds[match.route.id], }); } From 9f58ee465f4194679869d31213289fb1f18e37fe Mon Sep 17 00:00:00 2001 From: Jacob Ebey Date: Fri, 23 Dec 2022 13:05:24 -0800 Subject: [PATCH 07/10] chore: un-flatten response init --- packages/router/__tests__/router-test.ts | 11 ++++++----- packages/router/utils.ts | 10 ++-------- 2 files changed, 8 insertions(+), 13 deletions(-) diff --git a/packages/router/__tests__/router-test.ts b/packages/router/__tests__/router-test.ts index ab76130210..20f7b1b6aa 100644 --- a/packages/router/__tests__/router-test.ts +++ b/packages/router/__tests__/router-test.ts @@ -193,15 +193,16 @@ expect.extend({ done )}/status=${status}/headers=${JSON.stringify(headers)}, ` + `instead got done=${String(deferredData.done)}/status=${ - deferredData.statusCode + deferredData.init!.status || 200 }/headers=${JSON.stringify( - Object.fromEntries(deferredData.headers.entries()) + Object.fromEntries(new Headers(deferredData.init!.headers).entries()) )}`, pass: deferredData.done === done && - deferredData.statusCode === status && - JSON.stringify(Object.fromEntries(deferredData.headers.entries())) === - JSON.stringify(headers), + (deferredData.init!.status || 200) === status && + JSON.stringify( + Object.fromEntries(new Headers(deferredData.init!.headers).entries()) + ) === JSON.stringify(headers), }; }, // Custom matcher for asserting deferred promise results inside of `toEqual()` diff --git a/packages/router/utils.ts b/packages/router/utils.ts index a360449b91..d8e089591d 100644 --- a/packages/router/utils.ts +++ b/packages/router/utils.ts @@ -1140,8 +1140,7 @@ export class DeferredData { private subscribers: Set<(aborted: boolean, settledKey?: string) => void> = new Set(); data: Record; - statusCode: number; - headers: Headers; + init?: ResponseInit; deferredKeys: string[] = []; constructor(data: Record, responseInit?: ResponseInit) { @@ -1169,12 +1168,7 @@ export class DeferredData { {} ); - this.statusCode = - responseInit && responseInit.status ? responseInit.status : 200; - this.headers = - responseInit && responseInit.status - ? new Headers(responseInit.headers) - : new Headers(); + this.init = responseInit; } private trackPromise( From 0f39a901fdb3f86fb54732b914dfc1788e125e46 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Wed, 11 Jan 2023 11:36:24 -0500 Subject: [PATCH 08/10] Bump bundle --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 48f42de25b..b609c2a0aa 100644 --- a/package.json +++ b/package.json @@ -107,7 +107,7 @@ }, "filesize": { "packages/router/dist/router.umd.min.js": { - "none": "37.5 kB" + "none": "38.5 kB" }, "packages/react-router/dist/react-router.production.min.js": { "none": "12.5 kB" From fc93ca86d9cba83cf387dfdf120692066aac1711 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Wed, 11 Jan 2023 11:57:44 -0500 Subject: [PATCH 09/10] Minor cleanup --- packages/router/index.ts | 7 +++---- packages/router/router.ts | 26 ++++++++++++-------------- 2 files changed, 15 insertions(+), 18 deletions(-) diff --git a/packages/router/index.ts b/packages/router/index.ts index 0d8fc1051d..21670631d3 100644 --- a/packages/router/index.ts +++ b/packages/router/index.ts @@ -1,5 +1,3 @@ -import { convertRoutesToDataRoutes, getPathContributingMatches } from "./utils"; - export type { ActionFunction, ActionFunctionArgs, @@ -30,7 +28,6 @@ export { AbortedDeferredError, ErrorResponse, defer, - DeferredData as UNSAFE_DeferredData, generatePath, getToPathname, isRouteErrorResponse, @@ -59,6 +56,7 @@ export type { Path, To, } from "./history"; + export { Action, createBrowserHistory, @@ -80,6 +78,7 @@ export * from "./router"; /** @internal */ export { + DeferredData as UNSAFE_DeferredData, convertRoutesToDataRoutes as UNSAFE_convertRoutesToDataRoutes, getPathContributingMatches as UNSAFE_getPathContributingMatches, -}; +} from "./utils"; diff --git a/packages/router/router.ts b/packages/router/router.ts index 8c757828cd..afd4c03c52 100644 --- a/packages/router/router.ts +++ b/packages/router/router.ts @@ -2167,20 +2167,19 @@ export function createStaticHandler( } // Pick off the right state value to return - let routeData = [result.actionData, result.loaderData].find((v) => v); - let data = Object.values(routeData || {})[0]; + if (result.actionData) { + return Object.values(result.actionData)[0]; + } - if ( - data === Object.values(result.loaderData || {})[0] && - result.activeDeferreds && - result.activeDeferreds[match.route.id] - ) { - Object.assign(data, { - [UNSAFE_DEFERRED_SYMBOL]: result.activeDeferreds[match.route.id], - }); + if (result.loaderData) { + let data = Object.values(result.loaderData)[0]; + if (result.activeDeferreds?.[match.route.id]) { + data[UNSAFE_DEFERRED_SYMBOL] = result.activeDeferreds[match.route.id]; + } + return data; } - return data; + return undefined; } async function queryImpl( @@ -2440,8 +2439,8 @@ export function createStaticHandler( throw new Error(`${method}() call aborted`); } - let activeDeferreds = new Map(); // Process and commit output from loaders + let activeDeferreds = new Map(); let context = processRouteLoaderData( matches, matchesToLoad, @@ -2450,11 +2449,10 @@ export function createStaticHandler( activeDeferreds ); + // Add a null for any non-loader matches for proper revalidation on the client let executedLoaders = new Set( matchesToLoad.map((match) => match.route.id) ); - - // Add a null for any non-loader matches for proper revalidation on the client matches.forEach((match) => { if (!executedLoaders.has(match.route.id)) { context.loaderData[match.route.id] = null; From f77b4f4241267891299eb6fd63ea8215b92a80f5 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Wed, 11 Jan 2023 12:01:15 -0500 Subject: [PATCH 10/10] Add changeset --- .changeset/eight-tomatoes-breathe.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/eight-tomatoes-breathe.md diff --git a/.changeset/eight-tomatoes-breathe.md b/.changeset/eight-tomatoes-breathe.md new file mode 100644 index 0000000000..3f01aa2d9a --- /dev/null +++ b/.changeset/eight-tomatoes-breathe.md @@ -0,0 +1,5 @@ +--- +"@remix-run/router": minor +--- + +Expose deferred information from createStaticHandler