diff --git a/.changeset/error-response-type.md b/.changeset/error-response-type.md new file mode 100644 index 0000000000..9ae4c5e438 --- /dev/null +++ b/.changeset/error-response-type.md @@ -0,0 +1,9 @@ +--- +"react-router-dom-v5-compat": patch +"react-router-native": patch +"react-router-dom": patch +"react-router": patch +"@remix-run/router": patch +--- + +Move the `@private` class export `ErrorResponse` to an `UNSAFE_ErrorResponse` export since it is an implementation detail and there should be no construction of `ErrorResponse` instances in userland. This frees us up to export a `type ErrorResponse` which correlates to an instance of the class via `InstanceType`. Userland code should only ever be using `ErrorResponse` as a type and should be type-narrowing via `isRouteErrorResponse`. diff --git a/packages/react-router-dom-v5-compat/index.ts b/packages/react-router-dom-v5-compat/index.ts index 3ba151a738..6b970d291e 100644 --- a/packages/react-router-dom-v5-compat/index.ts +++ b/packages/react-router-dom-v5-compat/index.ts @@ -53,6 +53,7 @@ export type { BrowserRouterProps, DataRouteMatch, DataRouteObject, + ErrorResponse, Fetcher, FetcherWithComponents, FormEncType, diff --git a/packages/react-router-dom/index.tsx b/packages/react-router-dom/index.tsx index 2ebd7cb0bf..65274879ec 100644 --- a/packages/react-router-dom/index.tsx +++ b/packages/react-router-dom/index.tsx @@ -49,7 +49,7 @@ import { createHashHistory, joinPaths, stripBasename, - ErrorResponse, + UNSAFE_ErrorResponse as ErrorResponseImpl, UNSAFE_invariant as invariant, UNSAFE_warning as warning, } from "@remix-run/router"; @@ -92,6 +92,7 @@ export type { unstable_BlockerFunction, DataRouteMatch, DataRouteObject, + ErrorResponse, Fetcher, Hash, IndexRouteObject, @@ -273,7 +274,7 @@ function deserializeErrors( // Hey you! If you change this, please change the corresponding logic in // serializeErrors in react-router-dom/server.tsx :) if (val && val.__type === "RouteErrorResponse") { - serialized[key] = new ErrorResponse( + serialized[key] = new ErrorResponseImpl( val.status, val.statusText, val.data, diff --git a/packages/react-router-native/index.tsx b/packages/react-router-native/index.tsx index 5868993abe..e5d0941719 100644 --- a/packages/react-router-native/index.tsx +++ b/packages/react-router-native/index.tsx @@ -27,6 +27,7 @@ export type { unstable_BlockerFunction, DataRouteMatch, DataRouteObject, + ErrorResponse, Fetcher, Hash, IndexRouteObject, diff --git a/packages/react-router/index.ts b/packages/react-router/index.ts index a9b440d746..7a72bda98b 100644 --- a/packages/react-router/index.ts +++ b/packages/react-router/index.ts @@ -4,6 +4,7 @@ import type { ActionFunctionArgs, Blocker, BlockerFunction, + ErrorResponse, Fetcher, HydrationState, InitialEntry, @@ -127,6 +128,7 @@ export type { AwaitProps, DataRouteMatch, DataRouteObject, + ErrorResponse, Fetcher, FutureConfig, Hash, diff --git a/packages/router/__tests__/router-test.ts b/packages/router/__tests__/router-test.ts index c478952452..11a121095f 100644 --- a/packages/router/__tests__/router-test.ts +++ b/packages/router/__tests__/router-test.ts @@ -19,7 +19,6 @@ import { createStaticHandler, defer, UNSAFE_DEFERRED_SYMBOL, - ErrorResponse, IDLE_FETCHER, IDLE_NAVIGATION, json, @@ -39,6 +38,7 @@ import type { } from "../utils"; import { AbortedDeferredError, + ErrorResponseImpl, isRouteErrorResponse, stripBasename, } from "../utils"; @@ -2678,7 +2678,7 @@ describe("a router", () => { root: "ROOT", }); expect(t.router.state.errors).toEqual({ - root: new ErrorResponse( + root: new ErrorResponseImpl( 404, "Not Found", new Error('No route matches URL "/not-found"'), @@ -2707,7 +2707,7 @@ describe("a router", () => { t.navigate("/not-found"); expect(t.router.state.errors).toEqual({ - root: new ErrorResponse( + root: new ErrorResponseImpl( 404, "Not Found", new Error('No route matches URL "/not-found"'), @@ -2754,7 +2754,7 @@ describe("a router", () => { root: "ROOT*", }); expect(t.router.state.errors).toEqual({ - root: new ErrorResponse( + root: new ErrorResponseImpl( 404, "Not Found", new Error('No route matches URL "/not-found"'), @@ -4034,7 +4034,7 @@ describe("a router", () => { formData: createFormData({ gosh: "dang" }), }); expect(t.router.state.errors).toEqual({ - child: new ErrorResponse( + child: new ErrorResponseImpl( 405, "Method Not Allowed", new Error( @@ -4094,7 +4094,7 @@ describe("a router", () => { }); expect(t.router.state.actionData).toBe(null); expect(t.router.state.errors).toEqual({ - grandchild: new ErrorResponse( + grandchild: new ErrorResponseImpl( 405, "Method Not Allowed", new Error( @@ -4813,7 +4813,7 @@ describe("a router", () => { navigation: IDLE_NAVIGATION, loaderData: {}, errors: { - root: new ErrorResponse( + root: new ErrorResponseImpl( 404, "Not Found", new Error('No route matches URL "/junk"'), @@ -4963,7 +4963,7 @@ describe("a router", () => { search: "", }); expect(t.router.state.errors).toEqual({ - tasks: new ErrorResponse( + tasks: new ErrorResponseImpl( 405, "Method Not Allowed", new Error('Invalid request method "HEAD"'), @@ -5003,7 +5003,7 @@ describe("a router", () => { search: "", }); expect(t.router.state.errors).toEqual({ - tasks: new ErrorResponse( + tasks: new ErrorResponseImpl( 405, "Method Not Allowed", new Error('Invalid request method "OPTIONS"'), @@ -5915,7 +5915,7 @@ describe("a router", () => { }, actionData: null, errors: { - tasks: new ErrorResponse(400, "Bad Request", "broken"), + tasks: new ErrorResponseImpl(400, "Bad Request", "broken"), }, }); }); @@ -5950,7 +5950,7 @@ describe("a router", () => { }, actionData: null, errors: { - tasks: new ErrorResponse(400, "Bad Request", { key: "value" }), + tasks: new ErrorResponseImpl(400, "Bad Request", { key: "value" }), }, }); }); @@ -5985,7 +5985,7 @@ describe("a router", () => { }, actionData: null, errors: { - tasks: new ErrorResponse(400, "Bad Request", { key: "value" }), + tasks: new ErrorResponseImpl(400, "Bad Request", { key: "value" }), }, }); }); @@ -6563,7 +6563,7 @@ describe("a router", () => { }); expect(t.router.state.errors).toMatchInlineSnapshot(` { - "root": ErrorResponse { + "root": ErrorResponseImpl { "data": "Error: Unable to encode submission body", "error": [Error: Unable to encode submission body], "internal": true, @@ -6586,7 +6586,7 @@ describe("a router", () => { }); expect(t.router.state.errors).toMatchInlineSnapshot(` { - "root": ErrorResponse { + "root": ErrorResponseImpl { "data": "Error: Unable to encode submission body", "error": [Error: Unable to encode submission body], "internal": true, @@ -9091,7 +9091,7 @@ describe("a router", () => { await A.loaders.foo.reject(new Response(null, { status: 400 })); expect(A.fetcher).toBe(IDLE_FETCHER); expect(t.router.state.errors).toEqual({ - root: new ErrorResponse(400, undefined, ""), + root: new ErrorResponseImpl(400, undefined, ""), }); }); @@ -9104,7 +9104,7 @@ describe("a router", () => { await A.loaders.foo.reject(new Response(null, { status: 400 })); expect(A.fetcher).toBe(IDLE_FETCHER); expect(t.router.state.errors).toEqual({ - root: new ErrorResponse(400, undefined, ""), + root: new ErrorResponseImpl(400, undefined, ""), }); }); @@ -9117,7 +9117,7 @@ describe("a router", () => { await A.actions.foo.reject(new Response(null, { status: 400 })); expect(A.fetcher).toBe(IDLE_FETCHER); expect(t.router.state.errors).toEqual({ - root: new ErrorResponse(400, undefined, ""), + root: new ErrorResponseImpl(400, undefined, ""), }); }); @@ -9143,7 +9143,7 @@ describe("a router", () => { }); expect(A.fetcher).toBe(IDLE_FETCHER); expect(t.router.state.errors).toEqual({ - root: new ErrorResponse( + root: new ErrorResponseImpl( 405, "Method Not Allowed", new Error( @@ -9172,7 +9172,7 @@ describe("a router", () => { }); expect(A.fetcher).toBe(IDLE_FETCHER); expect(t.router.state.errors).toEqual({ - root: new ErrorResponse( + root: new ErrorResponseImpl( 400, "Bad Request", new Error("Unable to encode submission body"), @@ -9221,7 +9221,7 @@ describe("a router", () => { await t.fetch("/not-found", "key2", "wit"); expect(t.router.getFetcher("key2")).toBe(IDLE_FETCHER); expect(t.router.state.errors).toEqual({ - root: new ErrorResponse( + root: new ErrorResponseImpl( 404, "Not Found", new Error('No route matches URL "/not-found"'), @@ -9246,7 +9246,7 @@ describe("a router", () => { }); expect(t.router.getFetcher("key4")).toBe(IDLE_FETCHER); expect(t.router.state.errors).toEqual({ - wit: new ErrorResponse( + wit: new ErrorResponseImpl( 404, "Not Found", new Error('No route matches URL "/not-found"'), @@ -9257,7 +9257,7 @@ describe("a router", () => { await t.fetch("/not-found", "key5", "wit"); expect(t.router.getFetcher("key5")).toBe(IDLE_FETCHER); expect(t.router.state.errors).toEqual({ - wit: new ErrorResponse( + wit: new ErrorResponseImpl( 404, "Not Found", new Error('No route matches URL "/not-found"'), @@ -9279,7 +9279,7 @@ describe("a router", () => { await t.fetch("/not-found", "key7", "witout"); expect(t.router.getFetcher("key7")).toBe(IDLE_FETCHER); expect(t.router.state.errors).toEqual({ - root: new ErrorResponse( + root: new ErrorResponseImpl( 404, "Not Found", new Error('No route matches URL "/not-found"'), @@ -9650,12 +9650,12 @@ describe("a router", () => { await A.actions.foo.reject(new Response(null, { status: 400 })); expect(t.router.state.errors).toEqual({ - root: new ErrorResponse(400, undefined, ""), + root: new ErrorResponseImpl(400, undefined, ""), }); await B.actions.foo.resolve("B"); expect(t.router.state.errors).toEqual({ - root: new ErrorResponse(400, undefined, ""), + root: new ErrorResponseImpl(400, undefined, ""), }); await B.loaders.root.resolve(null); @@ -9688,7 +9688,7 @@ describe("a router", () => { await B.actions.foo.reject(new Response(null, { status: 400 })); expect(t.router.state.errors).toEqual({ - root: new ErrorResponse(400, undefined, ""), + root: new ErrorResponseImpl(400, undefined, ""), }); }); }); @@ -11426,7 +11426,7 @@ describe("a router", () => { await t.fetch("/parent"); expect(t.router.state.errors).toMatchInlineSnapshot(` { - "parent": ErrorResponse { + "parent": ErrorResponseImpl { "data": "Error: No route matches URL "/parent"", "error": [Error: No route matches URL "/parent"], "internal": true, @@ -11461,7 +11461,7 @@ describe("a router", () => { await t.fetch("/parent?index"); expect(t.router.state.errors).toMatchInlineSnapshot(` { - "parent": ErrorResponse { + "parent": ErrorResponseImpl { "data": "Error: No route matches URL "/parent?index"", "error": [Error: No route matches URL "/parent?index"], "internal": true, @@ -11498,7 +11498,7 @@ describe("a router", () => { }); expect(t.router.state.errors).toMatchInlineSnapshot(` { - "parent": ErrorResponse { + "parent": ErrorResponseImpl { "data": "Error: You made a POST request to "/parent" but did not provide an \`action\` for route "parent", so there is no way to handle the request.", "error": [Error: You made a POST request to "/parent" but did not provide an \`action\` for route "parent", so there is no way to handle the request.], "internal": true, @@ -11535,7 +11535,7 @@ describe("a router", () => { }); expect(t.router.state.errors).toMatchInlineSnapshot(` { - "parent": ErrorResponse { + "parent": ErrorResponseImpl { "data": "Error: You made a POST request to "/parent?index" but did not provide an \`action\` for route "parent", so there is no way to handle the request.", "error": [Error: You made a POST request to "/parent?index" but did not provide an \`action\` for route "parent", so there is no way to handle the request.], "internal": true, @@ -12449,7 +12449,7 @@ describe("a router", () => { }, }); expect(t.router.state.errors).toEqual({ - parent: new ErrorResponse(400, "Bad Request", "broken", false), + parent: new ErrorResponseImpl(400, "Bad Request", "broken", false), }); await parentDfd.resolve("Yep!"); @@ -12555,7 +12555,7 @@ describe("a router", () => { b: "B LOADER", }); expect(t.router.state.errors).toEqual({ - bChild: new ErrorResponse(400, "Bad Request", "broken", false), + bChild: new ErrorResponseImpl(400, "Bad Request", "broken", false), }); }); @@ -14993,7 +14993,7 @@ describe("a router", () => { loaderData: {}, actionData: null, errors: { - index: new ErrorResponse( + index: new ErrorResponseImpl( 404, "Not Found", new Error('No route matches URL "/not/found"'), @@ -15201,7 +15201,7 @@ describe("a router", () => { actionData: null, loaderData: {}, errors: { - root: new ErrorResponse( + root: new ErrorResponseImpl( 405, "Method Not Allowed", new Error( @@ -15228,7 +15228,7 @@ describe("a router", () => { actionData: null, loaderData: {}, errors: { - root: new ErrorResponse( + root: new ErrorResponseImpl( 405, "Method Not Allowed", new Error('Invalid request method "OPTIONS"'), @@ -15505,7 +15505,7 @@ describe("a router", () => { }); expect(context.activeDeferreds).toEqual(null); expect(context.errors).toEqual({ - parent: new ErrorResponse( + parent: new ErrorResponseImpl( 400, "Bad Request", new Error("defer() is not supported in actions"), @@ -16614,7 +16614,7 @@ describe("a router", () => { } catch (e) { // eslint-disable-next-line jest/no-conditional-expect expect(e).toEqual( - new ErrorResponse( + new ErrorResponseImpl( 400, "Bad Request", new Error("defer() is not supported in actions"), @@ -17067,7 +17067,7 @@ describe("a router", () => { expect(currentRouter.state.fetchers.get("key")?.data).toBe(undefined); expect(currentRouter.state.errors).toMatchInlineSnapshot(` { - "root": ErrorResponse { + "root": ErrorResponseImpl { "data": "Error: No route matches URL "/foo"", "error": [Error: No route matches URL "/foo"], "internal": true, @@ -17161,7 +17161,7 @@ describe("a router", () => { // Fetcher should have been revalidated but theown a 404 wince the route was removed expect(currentRouter.state.fetchers.get("key")?.data).toBe(undefined); expect(currentRouter.state.errors).toEqual({ - root: new ErrorResponse( + root: new ErrorResponseImpl( 404, "Not Found", new Error('No route matches URL "/foo"'), diff --git a/packages/router/index.ts b/packages/router/index.ts index 0ec7e20229..4210aadfab 100644 --- a/packages/router/index.ts +++ b/packages/router/index.ts @@ -9,6 +9,7 @@ export type { AgnosticNonIndexRouteObject, AgnosticRouteMatch, AgnosticRouteObject, + ErrorResponse, FormEncType, FormMethod, HTMLFormMethod, @@ -29,7 +30,6 @@ export type { export { AbortedDeferredError, - ErrorResponse, defer, generatePath, getToPathname, @@ -82,6 +82,7 @@ export * from "./router"; export type { RouteManifest as UNSAFE_RouteManifest } from "./utils"; export { DeferredData as UNSAFE_DeferredData, + ErrorResponseImpl as UNSAFE_ErrorResponse, convertRoutesToDataRoutes as UNSAFE_convertRoutesToDataRoutes, getPathContributingMatches as UNSAFE_getPathContributingMatches, } from "./utils"; diff --git a/packages/router/router.ts b/packages/router/router.ts index c5101bcb9e..86e5d94c4e 100644 --- a/packages/router/router.ts +++ b/packages/router/router.ts @@ -35,7 +35,7 @@ import type { V7_MutationFormMethod, } from "./utils"; import { - ErrorResponse, + ErrorResponseImpl, ResultType, convertRoutesToDataRoutes, getPathContributingMatches, @@ -1229,7 +1229,7 @@ export function createRouter(init: RouterInit): Router { submission?: Submission; fetcherSubmission?: Submission; overrideNavigation?: Navigation; - pendingError?: ErrorResponse; + pendingError?: ErrorResponseImpl; startUninterruptedRevalidation?: boolean; preventScrollReset?: boolean; replace?: boolean; @@ -3176,7 +3176,7 @@ function normalizeNavigateOptions( ): { path: string; submission?: Submission; - error?: ErrorResponse; + error?: ErrorResponseImpl; } { // Return location verbatim on non-submission navigations if (!opts || !isSubmissionNavigation(opts)) { @@ -3787,7 +3787,7 @@ async function callLoaderOrAction( if (resultType === ResultType.error) { return { type: resultType, - error: new ErrorResponse(status, result.statusText, data), + error: new ErrorResponseImpl(status, result.statusText, data), headers: result.headers, }; } @@ -4152,7 +4152,7 @@ function getInternalRouterError( } } - return new ErrorResponse( + return new ErrorResponseImpl( status || 500, statusText, new Error(errorMessage), diff --git a/packages/router/utils.ts b/packages/router/utils.ts index 0a1b771771..8a37bbd776 100644 --- a/packages/router/utils.ts +++ b/packages/router/utils.ts @@ -1505,12 +1505,12 @@ export const redirectDocument: RedirectFunction = (url, init) => { * @private * Utility class we use to hold auto-unwrapped 4xx/5xx Response bodies */ -export class ErrorResponse { +export class ErrorResponseImpl { status: number; statusText: string; data: any; - error?: Error; - internal: boolean; + private error?: Error; + private internal: boolean; constructor( status: number, @@ -1530,6 +1530,11 @@ export class ErrorResponse { } } +// We don't want the class exported since usage of it at runtime is an +// implementation detail, but we do want to export the shape so folks can +// build their own abstractions around instances via isRouteErrorResponse() +export type ErrorResponse = InstanceType; + /** * Check if the given error is an ErrorResponse generated from a 4xx/5xx * Response thrown from an action/loader