From ee8ad35054a7f06ab87ba628fb38a84bfb501350 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Thu, 8 Dec 2022 18:01:42 -0500 Subject: [PATCH] Updates to back-compat for useFetcher/useFetchers --- integration/fetcher-state-test.ts | 270 +++++++++++++++++++++ packages/remix-eslint-config/rules/core.js | 11 +- packages/remix-react/components.tsx | 202 +++++++++------ packages/remix-react/index.tsx | 4 +- 4 files changed, 412 insertions(+), 75 deletions(-) create mode 100644 integration/fetcher-state-test.ts diff --git a/integration/fetcher-state-test.ts b/integration/fetcher-state-test.ts new file mode 100644 index 00000000000..0a467ad6bc3 --- /dev/null +++ b/integration/fetcher-state-test.ts @@ -0,0 +1,270 @@ +import { test, expect } from "@playwright/test"; +import { ServerMode } from "@remix-run/server-runtime/mode"; + +import { createAppFixture, createFixture, js } from "./helpers/create-fixture"; +import type { Fixture, AppFixture } from "./helpers/create-fixture"; +import { PlaywrightFixture } from "./helpers/playwright-fixture"; + +// idle, done, actionReload are tested during the testing of these flows +const TYPES = { + actionSubmission: "actionSubmission", + loaderSubmission: "loaderSubmission", + actionRedirect: "actionRedirect", + normalLoad: "normalLoad", +}; + +test.describe("fetcher states", () => { + let fixture: Fixture; + let appFixture: AppFixture; + + test.beforeAll(async () => { + fixture = await createFixture({ + files: { + "app/root.jsx": js` + import { useMemo, useRef } from "react"; + import { Outlet, Scripts, useFetchers } from "@remix-run/react"; + + export default function Comp() { + // Only gonna use a single fetcher in any given test but this way + // we can route away from the child route and preserve the info + const [fetcher] = useFetchers(); + const fetcherRef = useRef(); + const states = useMemo(() => { + if (!fetcher) return + const savedStates = fetcherRef.current || []; + savedStates.push({ + state: fetcher.state, + type: fetcher.type, + submission: fetcher.submission ? { + ...fetcher.submission, + formData: Object.fromEntries(fetcher.submission.formData.entries()), + key: undefined + }: undefined, + data: fetcher.data, + }); + fetcherRef.current = savedStates; + return savedStates; + }, [fetcher]); + + return ( + + + + {fetcher && fetcher.state != "idle" && ( +

Loading...

+ )} +

+ + {JSON.stringify(states, null, 2)} + +

+ + + + ); + } + `, + "app/routes/page.jsx": js` + import { redirect } from "@remix-run/node"; + import { useFetcher } from "@remix-run/react"; + export function loader() { + return { from: 'loader' } + } + export async function action({ request }) { + let fd = await request.formData() + if (fd.has('redirect')) { + return redirect('/redirect'); + } + return { from: 'action' } + } + export default function() { + const fetcher = useFetcher(); + return ( + <> + {fetcher.type === 'init' ? + 
+                    {
+                      JSON.stringify({
+                        state: fetcher.state,
+                        type: fetcher.type,
+                        submission: fetcher.submission,
+                        data: fetcher.data,
+                      })
+                    }
+
: + null} + + ${TYPES.actionSubmission} + + + + ${TYPES.loaderSubmission} + + + + ${TYPES.actionRedirect} + + + + + ); + } + `, + "app/routes/redirect.jsx": js` + import { useFetcher } from "@remix-run/react"; + export function loader() { + return { from: 'redirect loader' } + } + export default function() { + return

Redirect

; + } + `, + }, + }); + + appFixture = await createAppFixture(fixture, ServerMode.Development); + }); + + test.afterAll(async () => { + await appFixture.close(); + }); + + test("represents a initial fetcher", async ({ page }) => { + let app = new PlaywrightFixture(appFixture, page); + await app.goto("/page", true); + let text = (await app.getElement("#initial-state")).text(); + expect(JSON.parse(text)).toEqual({ + state: "idle", + type: "init", + }); + }); + + test("represents an actionSubmission fetcher", async ({ page }) => { + let app = new PlaywrightFixture(appFixture, page); + await app.goto("/page", true); + await app.clickElement(`#${TYPES.actionSubmission}`); + await page.waitForSelector("#loading", { state: "hidden" }); + let text = (await app.getElement("#states")).text(); + expect(JSON.parse(text)).toEqual([ + { + state: "submitting", + type: "actionSubmission", + submission: { + formData: { key: "value" }, + action: "/page", + method: "POST", + encType: "application/x-www-form-urlencoded", + }, + }, + { + state: "loading", + type: "actionReload", + submission: { + formData: { key: "value" }, + action: "/page", + method: "POST", + encType: "application/x-www-form-urlencoded", + }, + data: { + from: "action", + }, + }, + { + state: "idle", + type: "done", + data: { + from: "action", + }, + }, + ]); + }); + + test("represents a loaderSubmission fetcher", async ({ page }) => { + let app = new PlaywrightFixture(appFixture, page); + await app.goto("/page", true); + await app.clickElement(`#${TYPES.loaderSubmission}`); + await page.waitForSelector("#loading", { state: "hidden" }); + let text = (await app.getElement("#states")).text(); + expect(JSON.parse(text)).toEqual([ + { + state: "submitting", + type: "loaderSubmission", + submission: { + formData: { key: "value" }, + // TODO: I think this is a bug and will be fixed with RR 6.4 + action: "/page?key=value", + method: "GET", + encType: "application/x-www-form-urlencoded", + }, + }, + { + state: "idle", + type: "done", + data: { + from: "loader", + }, + }, + ]); + }); + + test("represents an actionRedirect fetcher", async ({ page }) => { + let app = new PlaywrightFixture(appFixture, page); + await app.goto("/page", true); + await app.clickElement(`#${TYPES.actionRedirect}`); + await page.waitForSelector("#loading", { state: "hidden" }); + let text = (await app.getElement("#states")).text(); + expect(JSON.parse(text)).toEqual([ + { + state: "submitting", + type: "actionSubmission", + submission: { + formData: { redirect: "yes" }, + action: "/page", + method: "POST", + encType: "application/x-www-form-urlencoded", + }, + }, + { + state: "loading", + type: "actionRedirect", + submission: { + formData: { redirect: "yes" }, + action: "/page", + method: "POST", + encType: "application/x-www-form-urlencoded", + }, + }, + { + state: "idle", + type: "done", + }, + ]); + }); + + test("represents a normalLoad fetcher", async ({ page }) => { + let app = new PlaywrightFixture(appFixture, page); + await app.goto("/page", true); + await app.clickElement(`#${TYPES.normalLoad}`); + await page.waitForSelector("#loading", { state: "hidden" }); + let text = (await app.getElement("#states")).text(); + expect(JSON.parse(text)).toEqual([ + { + state: "loading", + type: "normalLoad", + }, + { + data: { from: "loader" }, + state: "idle", + type: "done", + }, + ]); + }); +}); diff --git a/packages/remix-eslint-config/rules/core.js b/packages/remix-eslint-config/rules/core.js index aff9275c304..8259a65dfbc 100644 --- a/packages/remix-eslint-config/rules/core.js +++ b/packages/remix-eslint-config/rules/core.js @@ -89,7 +89,16 @@ module.exports = { "no-new-object": WARN, "no-octal": WARN, "no-redeclare": ERROR, - "no-restricted-imports": [WARN, ...replaceRemixImportsOptions], + "no-restricted-imports": [ + WARN, + ...replaceRemixImportsOptions, + { + importNames: ["useTransition"], + message: + "useTransition is deprecated in favor of useNavigation as of v1.9.0.", + name: "@remix-run/react", + }, + ], "no-script-url": WARN, "no-self-assign": WARN, "no-self-compare": WARN, diff --git a/packages/remix-react/components.tsx b/packages/remix-react/components.tsx index 52b7fdded82..9a7ef467d75 100644 --- a/packages/remix-react/components.tsx +++ b/packages/remix-react/components.tsx @@ -8,7 +8,7 @@ import type { AgnosticDataRouteMatch, AgnosticDataRouteObject, ErrorResponse, - Navigation, + Fetcher as FetcherRR, } from "@remix-run/router"; import type { LinkProps, @@ -28,6 +28,7 @@ import { isRouteErrorResponse, matchRoutes, useFetcher as useFetcherRR, + useFetchers as useFetchersRR, useActionData as useActionDataRR, useLoaderData as useLoaderDataRR, useMatches as useMatchesRR, @@ -57,7 +58,6 @@ import { } from "./links"; import type { HtmlLinkDescriptor, PrefetchPageDescriptor } from "./links"; import { createHtml } from "./markup"; -import type { RouteData } from "./routeData"; import type { RouteMatchWithMeta, V1_HtmlMetaDescriptor, @@ -72,7 +72,6 @@ import type { TransitionStates, } from "./transition"; import { IDLE_TRANSITION, IDLE_FETCHER } from "./transition"; -import { labeledStatement } from "@babel/types"; function useDataRouterContext() { let context = React.useContext(DataRouterContext); @@ -156,6 +155,15 @@ export function RemixRouteError({ id }: { id: string }) { let location = useLocation(); let { CatchBoundary, ErrorBoundary } = routeModules[id]; + // POC for potential v2 error boundary handling + // if (future.v2_errorBoundary) { + // // Provide defaults for the root route if they are not present + // if (id === "root") { + // ErrorBoundary ||= RemixRootDefaultNewErrorBoundary; + // } + // return + // } + // Provide defaults for the root route if they are not present if (id === "root") { CatchBoundary ||= RemixRootDefaultCatchBoundary; @@ -945,6 +953,7 @@ export function useActionData(): SerializeFrom | undefined { * navigation indicators and optimistic UI on data mutations. * * @see https://remix.run/api/remix#usetransition + * @deprecated Deprecated in favor of useNavigation */ export function useTransition(): Transition { let navigation = useNavigation(); @@ -1090,6 +1099,17 @@ export function useTransition(): Transition { return transition; } +/** + * Provides all fetchers currently on the page. Useful for layouts and parent + * routes that need to provide pending/optimistic UI regarding the fetch. + * + * @see https://remix.run/api/remix#usefetchers + */ +export function useFetchers(): Fetcher[] { + let fetchers = useFetchersRR(); + return fetchers.map(convertRRFetcherToRemixFetcher); +} + export type FetcherWithComponents = Fetcher & { Form: React.ForwardRefExoticComponent< FormProps & React.RefAttributes @@ -1108,7 +1128,12 @@ export function useFetcher(): FetcherWithComponents< SerializeFrom > { let fetcherRR = useFetcherRR(); + return convertRRFetcherToRemixFetcher(fetcherRR); +} +function convertRRFetcherToRemixFetcher( + fetcherRR: ReturnType +) { let { state, formMethod, @@ -1126,8 +1151,13 @@ export function useFetcher(): FetcherWithComponents< ["POST", "PUT", "PATCH", "DELETE"].includes(formMethod.toUpperCase()); if (state === "idle") { - if (data === undefined) { - let fetcher: FetcherStates["Idle"] = IDLE_FETCHER; + if (fetcherRR[" _hasFetcherDoneAnything "] === true) { + let fetcher: FetcherStates["Done"] = { + state: "idle", + type: "done", + submission: undefined, + data, + }; return { Form, submit, @@ -1135,12 +1165,7 @@ export function useFetcher(): FetcherWithComponents< ...fetcher, }; } else { - let fetcher: FetcherStates["Done"] = { - state: "idle", - type: "done", - submission: undefined, - data, - }; + let fetcher: FetcherStates["Idle"] = IDLE_FETCHER; return { Form, submit, @@ -1167,12 +1192,16 @@ export function useFetcher(): FetcherWithComponents< action: formAction, encType: formEncType, formData: formData, - // TODO??? - // This looks like something that's created as a random hash value in - // useSubmitImpl in Remix today. we do not have this key in react router - // as we flattened submissions down onto the fetcher. Can we recreate - // one here in a stable manner? Or do we need to re-add this key to RR? - key: "todo-what-is-this?", + // TODO: this is created as a random hash value in useSubmitImpl in + // Remix today. We do not have this key in react router as we + // flattened submissions down onto the fetcher. We can't recreate in + // this back-compat layer in a stable manner for useFetchers because + // we don't have a stable fetcher identity. So we could: + // - Expose the fetcher key from the router (might make sense if + // we're considering adding useFetcher({ key }) anyway + // - Expose a hidden field with a stable identifier on the fetcher + // like we did for _hasFetcherDoneAnything + key: "todo-not-implemented-yet", }, data: undefined, }; @@ -1183,70 +1212,97 @@ export function useFetcher(): FetcherWithComponents< ...fetcher, }; } else { - // Actively "submitting" to a loader - let fetcher: FetcherStates["SubmittingLoader"] = { - state, - type: "loaderSubmission", - submission: { - method: formMethod.toUpperCase() as LoaderSubmission["method"], - action: formAction, - encType: formEncType, - // TODO: Recreate from params - formData: formData, - // TODO??? - key: "todo-what-is-this?", - }, - data: undefined, - }; - return { - Form, - submit, - load, - ...fetcher, - }; + invariant(false, "nope"); } } if (state === "loading") { - if ( - formMethod && - formAction && - formEncType && - formData && - isActionSubmission - ) { - if (data) { - // In a loading state but we have data - must be an actionReload - let fetcher: FetcherStates["ReloadingAction"] = { - state, - type: "actionReload", - submission: { - method: formMethod.toUpperCase() as ActionSubmission["method"], - action: formAction, - encType: formEncType, - formData: formData, - // TODO??? - key: "todo-what-is-this?", - }, - data: undefined, - }; - return { - Form, - submit, - load, - ...fetcher, - }; + if (formMethod && formAction && formEncType && formData) { + if (isActionSubmission) { + if (data) { + // In a loading state but we have data - must be an actionReload + let fetcher: FetcherStates["ReloadingAction"] = { + state, + type: "actionReload", + submission: { + method: formMethod.toUpperCase() as ActionSubmission["method"], + action: formAction, + encType: formEncType, + formData: formData, + // TODO: this is created as a random hash value in useSubmitImpl in + // Remix today. We do not have this key in react router as we + // flattened submissions down onto the fetcher. We can't recreate in + // this back-compat layer in a stable manner for useFetchers because + // we don't have a stable fetcher identity. So we could: + // - Expose the fetcher key from the router (might make sense if + // we're considering adding useFetcher({ key }) anyway + // - Expose a hidden field with a stable identifier on the fetcher + // like we did for _hasFetcherDoneAnything + key: "todo-not-implemented-yet", + }, + data, + }; + return { + Form, + submit, + load, + ...fetcher, + }; + } else { + let fetcher: FetcherStates["LoadingActionRedirect"] = { + state, + type: "actionRedirect", + submission: { + method: formMethod.toUpperCase() as ActionSubmission["method"], + action: formAction, + encType: formEncType, + formData: formData, + // TODO??? + key: "todo-what-is-this?", + }, + data: undefined, + }; + return { + Form, + submit, + load, + ...fetcher, + }; + } } else { - let fetcher: FetcherStates["LoadingActionRedirect"] = { - state, - type: "actionRedirect", + // The new router fixes a bug in useTransition where the submission + // "action" represents the request URL not the state of the
in + // the DOM. Back-port it here to maintain behavior, but useNavigation + // will fix this bug. + let url = new URL(formAction, window.location.origin); + + // This typing override should be safe since this is only running for + // GET submissions and over in @remix-run/router we have an invariant + // if you have any non-string values in your FormData when we attempt + // to convert them to URLSearchParams + url.search = new URLSearchParams( + formData.entries() as unknown as [string, string][] + ).toString(); + + // Actively "submitting" to a loader + let fetcher: FetcherStates["SubmittingLoader"] = { + state: "submitting", + type: "loaderSubmission", submission: { - method: formMethod.toUpperCase() as ActionSubmission["method"], - action: formAction, + method: formMethod.toUpperCase() as LoaderSubmission["method"], + action: url.pathname + url.search, encType: formEncType, formData: formData, - // TODO??? - key: "todo-what-is-this?", + // TODO: this is created as a random hash value in useSubmitImpl in + // Remix today. We do not have this key in react router as we + // flattened submissions down onto the fetcher. We can't recreate in + // this back-compat layer in a stable manner for useFetchers because + // we don't have a stable fetcher identity. So we could: + // - Expose the fetcher key from the router (might make sense if + // we're considering adding useFetcher({ key }) anyway + // - Expose a hidden field with a stable identifier on the fetcher + // like we did for _hasFetcherDoneAnything + key: "todo-not-implemented-yet", }, data: undefined, }; diff --git a/packages/remix-react/index.tsx b/packages/remix-react/index.tsx index bc3ea2fb230..4edba6613aa 100644 --- a/packages/remix-react/index.tsx +++ b/packages/remix-react/index.tsx @@ -12,16 +12,17 @@ export type { export { Form, Outlet, - useFetchers, useFormAction, useHref, useLocation, useNavigate, + useNavigation, useNavigationType, useOutlet, useOutletContext, useParams, useResolvedPath, + useRevalidator, useSearchParams, useSubmit, } from "react-router-dom"; @@ -43,6 +44,7 @@ export { LiveReload, useTransition, useFetcher, + useFetchers, useLoaderData, useMatches, useActionData,