4Catalyzer/found

<Redirect> broken with nested routes

nirvdrum opened this issue · 1 comments

We use nested routes throughout to better handle things with found-relay and just general organization improvements. A sample URL is /organizations/:organizationId/parameters. I tried to redirect from /organizations/:organizationId/dataStore to /organizations/:organizationId/parameters with:

<Redirect from="dataStore" to="parameters" status={301} />
<Route path="parameters">
   ...
</Route>

I can show the full hierarchy if needed, but hopefully you can see that the path field here corresponds to the last part of the URL. The issue is, this semi-redirects. The URL will be replaced with the correct one, but a 404 HttpError will also be thrown. It's a bit tricky to trace through, but I can see the initial RedirectException is thrown, but is subsequently rewritten to a 404 HttpError because resolveRenderArgs is unable to find a match for the rewritten match:

const routes = router.matcher.getRoutes(match);
const augmentedMatch = {
...match,
routes,
router, // Convenience access for route components.
context: matchContext,
};
if (!routes) {
// Immediately render a "not found" error if no routes matched.
yield { ...augmentedMatch, error: new HttpError(404) };
return;
}

As far as I can tell, the problem here is because the original match object doesn't contain the full path of the match, only the local path from the nested route. This causes routes to be null and consequently, the 404 HttpError is thrown. Elsewhere, the redirect seems to go through and rewrite the URL, leading to an overall confusing situation.

I found that if I treated the redirect as a flat route, that things worked:

<Redirect from="/organizations/:organizationId/dataStore" to="/organizations/:organizationId/parameters" status={301} />

I'm happy to have a solution, but it'd be nice if we could make use of our existing nesting structure. Alternatively, if nested redirects are not supported, it'd be helpful to have a runtime error that fails entirely. The current half-working, half-failing case makes it unclear what the problem is.

taion commented

Yeah, there isn't really any support for relative routes right now. Essentially the issue is that it's not totally clear what things should be relative to. I'm not sure why we don't just throw an exception in these cases TBH.