TanStack / router

🤖 Fully typesafe Router for React (and friends) w/ built-in caching, 1st class search-param APIs, client-side cache integration and isomorphic rendering.
https://tanstack.com/router
MIT License
8.17k stars 639 forks source link

Redirect in loader causes infinite loop #2142

Closed lukasbash closed 4 weeks ago

lukasbash commented 2 months ago

Describe the bug

Given the situation that any parent route's loader implements some mechanism to redirect to a more precise child route, the redirect throw causes an endless loop as the parents loader is called again and again. In fact I do not really understand why a routes parent is even called if at least one children route has an exact match.

Given I have the following structure:

If the dynamic segment implements a loader and I want to redirect to any specific child route it gets stuck in an endless loop. Using navigate or other methods do not work either.

Your Example Website or App

https://stackblitz.com/edit/github-wdwiyf?file=src%2Fmain.tsx

Steps to Reproduce the Bug or Issue

  1. Open the example
  2. Visit the "Model" tab
  3. Click on the link text in the body
  4. => route loader causes an infinite loop

Expected behavior

I would expect that the library itself evaluates that it does not call any loader function the actual redirect call came from. IF this is not possible, I would assume the redirect to be present as a flag in the loader context (and please don't tell me the only way is to substring the location's pathname or similar).

Screenshots or Videos

No response

Platform

Additional context

I just wonder how any child route of a dynamic segment implements a shared loader logic. I am aware of the fact that it might be possible to implement all routes with duplicated segments on top level like:

but this cannot be the right way, can it? Not only the parent chain is invalid via this approach, but also I have to duplicate the loader functions and precisely change only minimal parts of it.

Moreover I discovered that the redirect function does not share the typed router implementation, activated through typescript's module augmentation. Using version 1.48.1 of the router.

schiller-manuel commented 2 months ago

Moreover I discovered that the redirect function does not share the typed router implementation, activated through typescript's module augmentation.

what do you mean by this?

lukasbash commented 2 months ago

Moreover I discovered that the redirect function does not share the typed router implementation, activated through typescript's module augmentation.

what do you mean by this?

In the editor, it is (so far) the only function which does apparently not use the overridden types of the own router definition. At least it does not suggest any of my routes and I can input basically any to-target.

image

If I use e.g. the navigate's to property it works as expected.

schiller-manuel commented 2 months ago

I cannot reproduce this in VS Code:

Screenshot 2024-08-18 at 18 27 20

SeanCassiere commented 2 months ago

Regarding the typescript in the editor autocompleting the route, it definitely works upon retesting. Please make sure you've registered the types as per the documentation.

Onto, the actual issue presented, whereby you are trying to get throw a redirect to a child route from an already matched parent route in the tree... I did some investigation into this.

Currently, both NextJS and Remix do not support this behaviour that you are referring to. NextJS - https://stackblitz.com/edit/stackblitz-starters-9y4n2q?file=app%2Fmodels%2F%5BmodelId%5D%2Flayout.tsx Remix - https://stackblitz.com/edit/remix-run-remix-ned9wd?file=app%2Froutes%2Fmodels.%24modelId.tsx


The crux of the issue is that is not too dissimilar to looping without an exit condition.

Think of it like this. When you visit /models/1, these are all the active matches, with their callbacks being executed. In this call stack, you throw a redirect to go to /models/1/general.

[
  "__root",
  "models.$modelId", // throwing a redirect here
  "models.$modelId.index"
]

Now when the route is /models/1/general, the application needs to re-evaluate the route-tree and now these are the matches. But the issue is that, the same models.$modelId route is still being evaluated.

[
  "__root",
  "models.$modelId", // throwing a redirect here
  "models.$modelId.general"
]

In this scenario, you'll never reach models.$modelId.general, since you'll ALWAYS be aborting early and calling redirect.

This is programmatically equivalent to:

while (true) {
 //
}

Like with while and for loops, you need an exit condition.


Unless you either track this state or do some simple checking against the pathname, you'll never reach the /models/1/general route.

Something, like this would easily get you out of the infinite loop problem.

const modelIdRoute = createRoute({
  getParentRoute: () => rootRoute,
  path: 'model/$model_id',
  component: function Model() {
    return (
      <div className="p-2">
        Hello from Model!
        <Outlet />
      </div>
    );
  },
  loader: ({ params, location }) => {
    const shouldRedirect = [
      `/model/${params.model_id}`,
      `/model/${params.model_id}/`,
    ].includes(location.pathname);

    if (shouldRedirect) {
      throw redirect({
        to: '/model/$model_id/general',
        params,
      });
    }
  },
});

https://stackblitz.com/edit/github-wdwiyf-mqvec3?file=src%2Fmain.tsx

schiller-manuel commented 4 weeks ago

closed as a solution exists