Updates for loaderSubmission back compat

remix-run · Dec 8, 2022 · bca3696 · bca3696
1 parent 3036f2e
commit bca3696
Show file tree

Hide file tree

Showing 3 changed files with 68 additions and 95 deletions.
diff --git 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
@@ -947,18 +947,7 @@ export function useActionData<T = AppData>(): SerializeFrom<T> | undefined {
  * @see https://remix.run/api/remix#usetransition
  */
 export function useTransition(): Transition {
-  let currentLocation = useLocation();
   let navigation = useNavigation();
-  let lastNavigationRef = React.useRef<Navigation | null>(null);
-  let lastNavigation = lastNavigationRef.current;
-  lastNavigationRef.current = navigation;
-
-  // TODO: Should we populate navigation.formData on <Form method="get"> 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
@@ -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,