From bca3696bfcfd16fbf16ef90f8f6564ccbd50288a Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Thu, 8 Dec 2022 12:29:08 -0500 Subject: [PATCH] Updates for loaderSubmission back compat --- integration/transition-state-test.ts | 62 +++++++--------- packages/remix-react/components.tsx | 97 ++++++++++--------------- packages/remix-server-runtime/server.ts | 4 +- 3 files changed, 68 insertions(+), 95 deletions(-) diff --git a/integration/transition-state-test.ts b/integration/transition-state-test.ts index 4efec22f3d8..4502b66e058 100644 --- a/integration/transition-state-test.ts +++ b/integration/transition-state-test.ts @@ -253,14 +253,13 @@ test.describe("rendering", () => { pathname: "/", search: "?redirected", hash: "", - // These were private API for transition manager that are no longer - // needed with the new router so OK to disappear - // state: { - // isRedirect: true, - // setCookie: false, - // type: "loader", - // }, - state: null, + state: { + _isRedirect: true, + // These were private API for transition manager that are no longer + // needed with the new router so OK to disappear + // setCookie: false, + // type: "loader", + }, key: expect.any(String), }, }, @@ -291,9 +290,6 @@ test.describe("rendering", () => { state: null, key: expect.any(String), }, - // TODO This fails because we don't expose the "submission" on loader - // submissions from the new router, but we think we probably should - // even though it's a "loading" navigation submission: { action: `/${STATES.SUBMITTING_LOADER}`, encType: "application/x-www-form-urlencoded", @@ -344,14 +340,13 @@ test.describe("rendering", () => { pathname: "/", search: "?redirected", hash: "", - // These were private API for transition manager that are no longer - // needed with the new router so OK to disappear - // state: { - // isRedirect: true, - // setCookie: false, - // type: "loaderSubmission", - // }, - state: null, + state: { + _isRedirect: true, + // These were private API for transition manager that are no longer + // needed with the new router so OK to disappear + // setCookie: false, + // type: "loader", + }, key: expect.any(String), }, submission: { @@ -457,14 +452,13 @@ test.describe("rendering", () => { pathname: "/", search: "?redirected", hash: "", - // These were private API for transition manager that are no longer - // needed with the new router so OK to disappear - // state: { - // isRedirect: true, - // setCookie: false, - // type: "action", - // }, - state: null, + state: { + _isRedirect: true, + // These were private API for transition manager that are no longer + // needed with the new router so OK to disappear + // setCookie: false, + // type: "loader", + }, key: expect.any(String), }, submission: { @@ -499,15 +493,13 @@ test.describe("rendering", () => { pathname: "/", search: "?redirected", hash: "", - // These were private API for transition manager that are no longer - // needed with the new router so OK to disappear - // state: { - // isRedirect: true, - // setCookie: false, - // type: "fetchAction", - // }, state: { - isFetchActionRedirect: true, + _isRedirect: true, + _isFetchActionRedirect: true, + // These were private API for transition manager that are no longer + // needed with the new router so OK to disappear + // setCookie: false, + // type: "loader", }, key: expect.any(String), }, diff --git a/packages/remix-react/components.tsx b/packages/remix-react/components.tsx index ca01bcc1564..52b7fdded82 100644 --- a/packages/remix-react/components.tsx +++ b/packages/remix-react/components.tsx @@ -947,18 +947,7 @@ export function useActionData(): SerializeFrom | undefined { * @see https://remix.run/api/remix#usetransition */ export function useTransition(): Transition { - let currentLocation = useLocation(); let navigation = useNavigation(); - let lastNavigationRef = React.useRef(null); - let lastNavigation = lastNavigationRef.current; - lastNavigationRef.current = navigation; - - // TODO: Should we populate navigation.formData on
even - // though we've already move the data onto URLSearchParams. - // Reason would be to provide a consistent optimistic UI DX regardless of form method - // Downside is that it arguably deviates from how the browser would handle it since there would - // be no request body/FormData. We _do_ strip formData from the Request passed to your loader - // but we could keep it on the navigation object for DX let { location, state, formMethod, formAction, formEncType, formData } = navigation; @@ -994,33 +983,18 @@ export function useTransition(): Transition { }; return transition; } else { - // TODO if we don't update the router to keep formData on loader - // submissions then we can recreate it here from URLSearchParams - - // Actively "submitting" to a loader - let transition: TransitionStates["SubmittingLoader"] = { - location, - state, - submission: { - method: formMethod.toUpperCase() as LoaderSubmission["method"], - action: formAction, - encType: formEncType, - // TODO: Recreate from params - formData: formData, - key: location.key, - }, - type: "loaderSubmission", - }; - return transition; + // @remix-run/router doesn't mark loader submissions as state: "submitting" + invariant( + false, + "Encountered an unexpected navigation scenario in useTransition()" + ); } } if (state === "loading") { + let { _isRedirect, _isFetchActionRedirect } = location.state || {}; if (formMethod && formAction && formEncType && formData) { - if (formAction === location.pathname + location.search) { - // TODO: How would we detect a redirect to the same location from an - // action? Might need local state ion this hook to track the previous - // "transition" + if (!_isRedirect) { if (isActionSubmission) { // We're reloading the same location after an action submission let transition: TransitionStates["LoadingAction"] = { @@ -1037,12 +1011,20 @@ export function useTransition(): Transition { }; return transition; } else { - // I don't think this is possible? This is just a loader submission - // which goes idle -> submitting -> idle? - invariant( - false, - "Encountered an unexpected navigation scenario in useTransition()" - ); + // Actively "submitting" to a loader + let transition: TransitionStates["SubmittingLoader"] = { + location, + state: "submitting", + submission: { + method: formMethod.toUpperCase() as LoaderSubmission["method"], + action: formAction, + encType: formEncType, + formData: formData, + key: location.key, + }, + type: "loaderSubmission", + }; + return transition; } } else { // Redirecting after a submission @@ -1077,25 +1059,24 @@ export function useTransition(): Transition { return transition; } } - } else if ( - lastNavigation?.state === "loading" && - lastNavigation.location.key !== navigation.location?.key - ) { - let transition: TransitionStates["LoadingRedirect"] = { - location, - state, - submission: undefined, - type: "normalRedirect", - }; - return transition; - } else if (location.state?.isFetchActionRedirect) { - let transition: TransitionStates["LoadingFetchActionRedirect"] = { - location, - state, - submission: undefined, - type: "fetchActionRedirect", - }; - return transition; + } else if (_isRedirect) { + if (_isFetchActionRedirect) { + let transition: TransitionStates["LoadingFetchActionRedirect"] = { + location, + state, + submission: undefined, + type: "fetchActionRedirect", + }; + return transition; + } else { + let transition: TransitionStates["LoadingRedirect"] = { + location, + state, + submission: undefined, + type: "normalRedirect", + }; + return transition; + } } } diff --git a/packages/remix-server-runtime/server.ts b/packages/remix-server-runtime/server.ts index 5d1977f1a73..563fe947204 100644 --- a/packages/remix-server-runtime/server.ts +++ b/packages/remix-server-runtime/server.ts @@ -101,7 +101,7 @@ async function handleDataRequestRR( request: Request ) { try { - let response = await staticHandler.queryRoute(request, routeId); + let response = await staticHandler.queryRoute(request, { routeId }); if (isRedirectResponse(response)) { // We don't have any way to prevent a fetch request from following @@ -302,7 +302,7 @@ async function handleResourceRequestRR( // Note we keep the routeId here to align with the Remix handling of // resource routes which doesn't take ?index into account and just takes // the leaf match - let response = await staticHandler.queryRoute(request, routeId); + let response = await staticHandler.queryRoute(request, { routeId }); // callRouteLoader/callRouteAction always return responses invariant( response instanceof Response,