From 55bd2790f078db04a2f7a2a6f85af278efdcf3b1 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Thu, 18 Jul 2024 08:58:44 -0400 Subject: [PATCH 1/4] Remove v7_skipActionErrorRevalidation flag --- .changeset/twelve-cheetahs-pretend.md | 5 +++++ .../__tests__/router/should-revalidate-test.ts | 2 -- packages/react-router/lib/dom/server.tsx | 1 - packages/react-router/lib/dom/ssr/browser.tsx | 4 ---- packages/react-router/lib/router/router.ts | 11 ++--------- 5 files changed, 7 insertions(+), 16 deletions(-) create mode 100644 .changeset/twelve-cheetahs-pretend.md diff --git a/.changeset/twelve-cheetahs-pretend.md b/.changeset/twelve-cheetahs-pretend.md new file mode 100644 index 0000000000..8faf76ef38 --- /dev/null +++ b/.changeset/twelve-cheetahs-pretend.md @@ -0,0 +1,5 @@ +--- +"react-router": major +--- + +Remove `v7_skipActionErrorRevalidation` flag diff --git a/packages/react-router/__tests__/router/should-revalidate-test.ts b/packages/react-router/__tests__/router/should-revalidate-test.ts index 68819067a1..46154cb31c 100644 --- a/packages/react-router/__tests__/router/should-revalidate-test.ts +++ b/packages/react-router/__tests__/router/should-revalidate-test.ts @@ -1202,7 +1202,6 @@ describe("shouldRevalidate", () => { root: "ROOT", }, }, - future: { v7_skipActionErrorRevalidation: true }, }); router.initialize(); @@ -1270,7 +1269,6 @@ describe("shouldRevalidate", () => { root: "ROOT", }, }, - future: { v7_skipActionErrorRevalidation: true }, }); router.initialize(); diff --git a/packages/react-router/lib/dom/server.tsx b/packages/react-router/lib/dom/server.tsx index 39431de869..3539d14373 100644 --- a/packages/react-router/lib/dom/server.tsx +++ b/packages/react-router/lib/dom/server.tsx @@ -302,7 +302,6 @@ export function createStaticRouter( }, get future() { return { - v7_skipActionErrorRevalidation: false, ...opts?.future, }; }, diff --git a/packages/react-router/lib/dom/ssr/browser.tsx b/packages/react-router/lib/dom/ssr/browser.tsx index 1911b438de..bf4dae63af 100644 --- a/packages/react-router/lib/dom/ssr/browser.tsx +++ b/packages/react-router/lib/dom/ssr/browser.tsx @@ -187,10 +187,6 @@ function createHydratedRouter(): RemixRouter { routes, history: createBrowserHistory(), basename: ssrInfo.context.basename, - future: { - // Single fetch enables this underlying behavior - v7_skipActionErrorRevalidation: true, - }, hydrationData, mapRouteProperties, unstable_dataStrategy: getSingleFetchDataStrategy( diff --git a/packages/react-router/lib/router/router.ts b/packages/react-router/lib/router/router.ts index 3082f917d5..55fc4dba18 100644 --- a/packages/react-router/lib/router/router.ts +++ b/packages/react-router/lib/router/router.ts @@ -353,9 +353,7 @@ export type HydrationState = Partial< /** * Future flags to toggle new feature behavior */ -export interface FutureConfig { - v7_skipActionErrorRevalidation: boolean; -} +export interface FutureConfig {} /** * Initialization options for createRouter @@ -812,7 +810,6 @@ export function createRouter(init: RouterInit): Router { // Config driven behavior flags let future: FutureConfig = { - v7_skipActionErrorRevalidation: false, ...init.future, }; // Cleanup function for history @@ -1897,7 +1894,6 @@ export function createRouter(init: RouterInit): Router { activeSubmission, location, initialHydration === true, - future.v7_skipActionErrorRevalidation, isRevalidationRequired, cancelledFetcherLoads, fetchersQueuedForDeletion, @@ -2317,7 +2313,6 @@ export function createRouter(init: RouterInit): Router { submission, nextLocation, false, - future.v7_skipActionErrorRevalidation, isRevalidationRequired, cancelledFetcherLoads, fetchersQueuedForDeletion, @@ -4147,7 +4142,6 @@ function getMatchesToLoad( submission: Submission | undefined, location: Location, isInitialLoad: boolean, - skipActionErrorRevalidation: boolean, isRevalidationRequired: boolean, cancelledFetcherLoads: string[], fetchersQueuedForDeletion: Set, @@ -4180,8 +4174,7 @@ function getMatchesToLoad( let actionStatus = pendingActionResult ? pendingActionResult[1].statusCode : undefined; - let shouldSkipRevalidation = - skipActionErrorRevalidation && actionStatus && actionStatus >= 400; + let shouldSkipRevalidation = actionStatus && actionStatus >= 400; let navigationMatches = boundaryMatches.filter((match, index) => { let { route } = match; From ffd379045c4de8818df07a9b828276349094fc79 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Thu, 18 Jul 2024 09:02:02 -0400 Subject: [PATCH 2/4] Remove lingering Remix v3 future flags --- packages/react-router-dev/config.ts | 12 ++---------- .../__tests__/dom/scroll-restoration-test.tsx | 4 ---- .../__tests__/server-runtime/data-test.ts | 4 ---- .../__tests__/server-runtime/server-test.ts | 11 +---------- packages/react-router/lib/dom/ssr/entry.ts | 6 +----- .../react-router/lib/dom/ssr/routes-test-stub.tsx | 6 +----- 6 files changed, 5 insertions(+), 38 deletions(-) diff --git a/packages/react-router-dev/config.ts b/packages/react-router-dev/config.ts index c1e9ad41df..b8f9ad7222 100644 --- a/packages/react-router-dev/config.ts +++ b/packages/react-router-dev/config.ts @@ -72,11 +72,7 @@ export type ServerBundlesBuildManifest = BaseBuildManifest & { type ServerModuleFormat = "esm" | "cjs"; -interface FutureConfig { - v3_fetcherPersist: boolean; - v3_relativeSplatPath: boolean; - v3_throwAbortReason: boolean; -} +interface FutureConfig {} export type BuildManifest = DefaultBuildManifest | ServerBundlesBuildManifest; @@ -440,11 +436,7 @@ export async function resolveReactRouterConfig({ } } - let future: FutureConfig = { - v3_fetcherPersist: userFuture?.v3_fetcherPersist === true, - v3_relativeSplatPath: userFuture?.v3_relativeSplatPath === true, - v3_throwAbortReason: userFuture?.v3_throwAbortReason === true, - }; + let future: FutureConfig = {}; let reactRouterConfig: ResolvedVitePluginConfig = deepFreeze({ appDirectory, diff --git a/packages/react-router/__tests__/dom/scroll-restoration-test.tsx b/packages/react-router/__tests__/dom/scroll-restoration-test.tsx index 00da9fd9f9..8576f535e1 100644 --- a/packages/react-router/__tests__/dom/scroll-restoration-test.tsx +++ b/packages/react-router/__tests__/dom/scroll-restoration-test.tsx @@ -208,10 +208,6 @@ describe(`ScrollRestoration`, () => { }); let context: FrameworkContextObject = { - future: { - v3_fetcherPersist: false, - v3_relativeSplatPath: false, - }, routeModules: { root: { default: () => null } }, manifest: { routes: { diff --git a/packages/react-router/__tests__/server-runtime/data-test.ts b/packages/react-router/__tests__/server-runtime/data-test.ts index 59ee710e49..b1b58fc424 100644 --- a/packages/react-router/__tests__/server-runtime/data-test.ts +++ b/packages/react-router/__tests__/server-runtime/data-test.ts @@ -23,10 +23,6 @@ describe("loaders", () => { }, }, entry: { module: {} }, - future: { - v3_fetcherPersist: false, - v3_relativeSplatPath: false, - }, } as unknown as ServerBuild; let handler = createRequestHandler(build); diff --git a/packages/react-router/__tests__/server-runtime/server-test.ts b/packages/react-router/__tests__/server-runtime/server-test.ts index 13ccb3f481..d4e749285c 100644 --- a/packages/react-router/__tests__/server-runtime/server-test.ts +++ b/packages/react-router/__tests__/server-runtime/server-test.ts @@ -414,7 +414,7 @@ describe("shared server runtime", () => { expect(spy.console.mock.calls.length).toBe(1); }); - test("aborts request (v3_throwAbortReason)", async () => { + test("aborts request with reason", async () => { let rootLoader = jest.fn(() => { return "root"; }); @@ -435,9 +435,6 @@ describe("shared server runtime", () => { }, }, { - future: { - v3_throwAbortReason: true, - }, handleError: handleErrorSpy, } ); @@ -932,9 +929,6 @@ describe("shared server runtime", () => { }, }, { - future: { - v3_throwAbortReason: true, - }, handleError: handleErrorSpy, } ); @@ -2015,9 +2009,6 @@ describe("shared server runtime", () => { }, }, { - future: { - v3_throwAbortReason: true, - }, handleError: handleErrorSpy, } ); diff --git a/packages/react-router/lib/dom/ssr/entry.ts b/packages/react-router/lib/dom/ssr/entry.ts index 1ab1e3046c..1f6b633b80 100644 --- a/packages/react-router/lib/dom/ssr/entry.ts +++ b/packages/react-router/lib/dom/ssr/entry.ts @@ -40,11 +40,7 @@ export interface EntryContext extends FrameworkContextObject { serverHandoffStream?: ReadableStream; } -export interface FutureConfig { - v3_fetcherPersist: boolean; - v3_relativeSplatPath: boolean; - v3_throwAbortReason: boolean; -} +export interface FutureConfig {} export interface AssetsManifest { entry: { diff --git a/packages/react-router/lib/dom/ssr/routes-test-stub.tsx b/packages/react-router/lib/dom/ssr/routes-test-stub.tsx index 6843308926..8dd0d4bbf2 100644 --- a/packages/react-router/lib/dom/ssr/routes-test-stub.tsx +++ b/packages/react-router/lib/dom/ssr/routes-test-stub.tsx @@ -105,11 +105,7 @@ export function createRoutesStub( if (routerRef.current == null) { remixContextRef.current = { - future: { - v3_fetcherPersist: future?.v3_fetcherPersist === true, - v3_relativeSplatPath: future?.v3_relativeSplatPath === true, - v3_throwAbortReason: future?.v3_throwAbortReason === true, - }, + future: {}, manifest: { routes: {}, entry: { imports: [], module: "" }, From 3d6f7500daa36d19154e68bf3ee933a51cb551bd Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Wed, 24 Jul 2024 16:48:13 -0400 Subject: [PATCH 3/4] Remove irrelevant unit test for prior behavior --- .../router/should-revalidate-test.ts | 374 ++++++------------ 1 file changed, 120 insertions(+), 254 deletions(-) diff --git a/packages/react-router/__tests__/router/should-revalidate-test.ts b/packages/react-router/__tests__/router/should-revalidate-test.ts index 46154cb31c..3a38f0d7c8 100644 --- a/packages/react-router/__tests__/router/should-revalidate-test.ts +++ b/packages/react-router/__tests__/router/should-revalidate-test.ts @@ -1038,269 +1038,135 @@ describe("shouldRevalidate", () => { router.dispose(); }); - describe("skipActionRevalidation", () => { - it("revalidates after actions returning 4xx/5xx responses by default", async () => { - let count = -1; - let responses = [ - new Response("DATA 400", { status: 400 }), - new Response("DATA 500", { status: 500 }), - ]; - let rootLoader = jest.fn(() => "ROOT " + count); - - let history = createMemoryHistory(); - let router = createRouter({ - history, - routes: [ - { - id: "root", - path: "/", - loader: async () => rootLoader(), - children: [ - { - id: "index", - index: true, - hasErrorBoundary: true, - action: () => responses[++count], - }, - ], - }, - ], - hydrationData: { - loaderData: { - root: "ROOT", - }, + it("does not revalidates after actions returning 4xx/5xx responses with flag", async () => { + let count = -1; + let responses = [ + new Response("DATA 400", { status: 400 }), + new Response("DATA 500", { status: 500 }), + ]; + let rootLoader = jest.fn(() => "ROOT " + count); + + let history = createMemoryHistory(); + let router = createRouter({ + history, + routes: [ + { + id: "root", + path: "/", + loader: async () => rootLoader(), + children: [ + { + id: "index", + index: true, + hasErrorBoundary: true, + action: () => responses[++count], + }, + ], + }, + ], + hydrationData: { + loaderData: { + root: "ROOT", }, - }); - router.initialize(); - - router.navigate("/?index", { - formMethod: "post", - formData: createFormData({ gosh: "dang" }), - }); - await tick(); - expect(rootLoader.mock.calls.length).toBe(1); - expect(router.state).toMatchObject({ - location: { pathname: "/" }, - navigation: { state: "idle" }, - loaderData: { root: "ROOT 0" }, - actionData: { index: "DATA 400" }, - errors: null, - }); - - router.navigate("/?index", { - formMethod: "post", - formData: createFormData({ gosh: "dang" }), - }); - await tick(); - expect(rootLoader.mock.calls.length).toBe(2); - expect(router.state).toMatchObject({ - location: { pathname: "/" }, - navigation: { state: "idle" }, - loaderData: { root: "ROOT 1" }, - actionData: { index: "DATA 500" }, - errors: null, - }); - - router.dispose(); + }, }); + router.initialize(); - it("revalidates after actions throwing 4xx/5xx responses by default", async () => { - let count = -1; - let responses = [ - new Response("ERROR 400", { status: 400 }), - new Response("ERROR 500", { status: 500 }), - ]; - let rootLoader = jest.fn(() => "ROOT " + count); - - let history = createMemoryHistory(); - let router = createRouter({ - history, - routes: [ - { - id: "root", - path: "/", - loader: async () => rootLoader(), - children: [ - { - id: "index", - index: true, - hasErrorBoundary: true, - action: () => { - throw responses[++count]; - }, - }, - ], - }, - ], - hydrationData: { - loaderData: { - root: "ROOT", - }, - }, - }); - router.initialize(); - - router.navigate("/?index", { - formMethod: "post", - formData: createFormData({ gosh: "dang" }), - }); - await tick(); - expect(rootLoader.mock.calls.length).toBe(1); - expect(router.state).toMatchObject({ - location: { pathname: "/" }, - navigation: { state: "idle" }, - loaderData: { root: "ROOT 0" }, - actionData: null, - errors: { index: new ErrorResponseImpl(400, "", "ERROR 400") }, - }); - - router.navigate("/?index", { - formMethod: "post", - formData: createFormData({ gosh: "dang" }), - }); - await tick(); - expect(rootLoader.mock.calls.length).toBe(2); - expect(router.state).toMatchObject({ - location: { pathname: "/" }, - navigation: { state: "idle" }, - loaderData: { root: "ROOT 1" }, - actionData: null, - errors: { index: new ErrorResponseImpl(500, "", "ERROR 500") }, - }); - - router.dispose(); + router.navigate("/?index", { + formMethod: "post", + formData: createFormData({ gosh: "dang" }), + }); + await tick(); + expect(rootLoader.mock.calls.length).toBe(0); + expect(router.state).toMatchObject({ + location: { pathname: "/" }, + navigation: { state: "idle" }, + loaderData: { root: "ROOT" }, + actionData: { index: "DATA 400" }, + errors: null, }); - it("does not revalidates after actions returning 4xx/5xx responses with flag", async () => { - let count = -1; - let responses = [ - new Response("DATA 400", { status: 400 }), - new Response("DATA 500", { status: 500 }), - ]; - let rootLoader = jest.fn(() => "ROOT " + count); - - let history = createMemoryHistory(); - let router = createRouter({ - history, - routes: [ - { - id: "root", - path: "/", - loader: async () => rootLoader(), - children: [ - { - id: "index", - index: true, - hasErrorBoundary: true, - action: () => responses[++count], - }, - ], - }, - ], - hydrationData: { - loaderData: { - root: "ROOT", - }, - }, - }); - router.initialize(); - - router.navigate("/?index", { - formMethod: "post", - formData: createFormData({ gosh: "dang" }), - }); - await tick(); - expect(rootLoader.mock.calls.length).toBe(0); - expect(router.state).toMatchObject({ - location: { pathname: "/" }, - navigation: { state: "idle" }, - loaderData: { root: "ROOT" }, - actionData: { index: "DATA 400" }, - errors: null, - }); - - router.navigate("/?index", { - formMethod: "post", - formData: createFormData({ gosh: "dang" }), - }); - await tick(); - expect(rootLoader.mock.calls.length).toBe(0); - expect(router.state).toMatchObject({ - location: { pathname: "/" }, - navigation: { state: "idle" }, - loaderData: { root: "ROOT" }, - actionData: { index: "DATA 500" }, - errors: null, - }); - - router.dispose(); + router.navigate("/?index", { + formMethod: "post", + formData: createFormData({ gosh: "dang" }), }); + await tick(); + expect(rootLoader.mock.calls.length).toBe(0); + expect(router.state).toMatchObject({ + location: { pathname: "/" }, + navigation: { state: "idle" }, + loaderData: { root: "ROOT" }, + actionData: { index: "DATA 500" }, + errors: null, + }); + + router.dispose(); + }); + + it("does not revalidate after actions throwing 4xx/5xx responses with flag", async () => { + let count = -1; + let responses = [ + new Response("ERROR 400", { status: 400 }), + new Response("ERROR 500", { status: 500 }), + ]; + let rootLoader = jest.fn(() => "ROOT " + count); - it("does not revalidate after actions throwing 4xx/5xx responses with flag", async () => { - let count = -1; - let responses = [ - new Response("ERROR 400", { status: 400 }), - new Response("ERROR 500", { status: 500 }), - ]; - let rootLoader = jest.fn(() => "ROOT " + count); - - let history = createMemoryHistory(); - let router = createRouter({ - history, - routes: [ - { - id: "root", - path: "/", - loader: async () => rootLoader(), - children: [ - { - id: "index", - index: true, - hasErrorBoundary: true, - action: () => { - throw responses[++count]; - }, + let history = createMemoryHistory(); + let router = createRouter({ + history, + routes: [ + { + id: "root", + path: "/", + loader: async () => rootLoader(), + children: [ + { + id: "index", + index: true, + hasErrorBoundary: true, + action: () => { + throw responses[++count]; }, - ], - }, - ], - hydrationData: { - loaderData: { - root: "ROOT", - }, + }, + ], + }, + ], + hydrationData: { + loaderData: { + root: "ROOT", }, - }); - router.initialize(); - - router.navigate("/?index", { - formMethod: "post", - formData: createFormData({ gosh: "dang" }), - }); - await tick(); - expect(rootLoader.mock.calls.length).toBe(0); - expect(router.state).toMatchObject({ - location: { pathname: "/" }, - navigation: { state: "idle" }, - loaderData: { root: "ROOT" }, - actionData: null, - errors: { index: new ErrorResponseImpl(400, "", "ERROR 400") }, - }); - - router.navigate("/?index", { - formMethod: "post", - formData: createFormData({ gosh: "dang" }), - }); - await tick(); - expect(rootLoader.mock.calls.length).toBe(0); - expect(router.state).toMatchObject({ - location: { pathname: "/" }, - navigation: { state: "idle" }, - loaderData: { root: "ROOT" }, - actionData: null, - errors: { index: new ErrorResponseImpl(500, "", "ERROR 500") }, - }); - - router.dispose(); + }, + }); + router.initialize(); + + router.navigate("/?index", { + formMethod: "post", + formData: createFormData({ gosh: "dang" }), + }); + await tick(); + expect(rootLoader.mock.calls.length).toBe(0); + expect(router.state).toMatchObject({ + location: { pathname: "/" }, + navigation: { state: "idle" }, + loaderData: { root: "ROOT" }, + actionData: null, + errors: { index: new ErrorResponseImpl(400, "", "ERROR 400") }, }); + + router.navigate("/?index", { + formMethod: "post", + formData: createFormData({ gosh: "dang" }), + }); + await tick(); + expect(rootLoader.mock.calls.length).toBe(0); + expect(router.state).toMatchObject({ + location: { pathname: "/" }, + navigation: { state: "idle" }, + loaderData: { root: "ROOT" }, + actionData: null, + errors: { index: new ErrorResponseImpl(500, "", "ERROR 500") }, + }); + + router.dispose(); }); }); From 3f78745527a678aebc9552f8f0e27123e9ebf340 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Wed, 24 Jul 2024 16:50:01 -0400 Subject: [PATCH 4/4] Update changeset --- .changeset/twelve-cheetahs-pretend.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.changeset/twelve-cheetahs-pretend.md b/.changeset/twelve-cheetahs-pretend.md index 8faf76ef38..0ec37aa33e 100644 --- a/.changeset/twelve-cheetahs-pretend.md +++ b/.changeset/twelve-cheetahs-pretend.md @@ -2,4 +2,7 @@ "react-router": major --- -Remove `v7_skipActionErrorRevalidation` flag +Remove remaining future flags + +- React Router `v7_skipActionErrorRevalidation` +- Remix `v3_fetcherPersist`, `v3_relativeSplatPath`, `v3_throwAbortReason`