fix: support basename and relative routing in loader/action redirects #9447

brophdawg11 · 2022-10-11T20:49:05Z

Builds on top of original PR (#9418) to handle basename in loader/action redirects and also adds support for relative routing in redirects.

Basename example:

let router = createBrowserRouter([{
  path: '/',
  loader() {
    // This will now properly redirect the browser to /base/other
    return redirect('/other');
  },
}], { basename: '/base'}

Relative routing example:

let router = createBrowserRouter([{
  path: '/',
  children: [{
    path: 'parent',
    children: [{
      path: 'child',
      loader() {
        // This will now properly redirect the browser to /parent
        return redirect('..');
      },
    }],
  }]
}]);

Closes #9417

Co-authored-by: Mikaël ANZANO [email protected]

Co-authored-by: Mikaël ANZANO <[email protected]>

changeset-bot · 2022-10-11T20:49:08Z

🦋 Changeset detected

Latest commit: d00e5bc

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 5 packages

Name	Type
@remix-run/router	Patch
react-router	Patch
react-router-dom	Patch
react-router-dom-v5-compat	Patch
react-router-native	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

brophdawg11 · 2022-10-12T19:47:55Z

.changeset/smart-ants-decide.md

+"@remix-run/router": patch
+---
+
+Support basename and relative routing in loader/action redirects


Did this as a patch/bug fix (as opposed to a minor/new feature) since redirects probably should have supported basename and relative routing from the get go. This would be new behavior for remix though when we bring it over - but I would argue non-breaking

brophdawg11 · 2022-10-12T19:48:50Z

packages/router/router.ts

+type FetchLoadMatch = [
+  string,
+  AgnosticDataRouteMatch,
+  AgnosticDataRouteMatch[]


We now track both the single fetcher match and it's hierarchical matched on fetcher.load so that we can do proper relative routing on fetcher revalidation redirects.

brophdawg11 · 2022-10-12T19:49:50Z

packages/router/router.ts

+        matches,
+        router.basename


We now expect the hierarchy of matches and the basename for all calls to callLoaderOrAction, that way we can make changes to the Response Location header as needed.

brophdawg11 · 2022-10-12T20:04:24Z

packages/router/router.ts

+      // Support relative routing in redirects
+      let activeMatches = matches.slice(0, matches.indexOf(match) + 1);
+      let routePathnames = getPathContributingMatches(activeMatches).map(
+        (match) => match.pathnameBase
+      );
+      let requestPath = createURL(request.url).pathname;
+      location = resolveTo(location, routePathnames, requestPath).pathname;
+      invariant(
+        location,
+        `Unable to resolve redirect location: ${result.headers.get("Location")}`
+      );
+
+      // Prepend the basename to the redirect location if we have one
+      if (basename) {
+        let path = createURL(location).pathname;
+        location = path === "/" ? basename : joinPaths([basename, path]);
+      }


Update the redirect location to account for basename and relative routing.

brophdawg11 · 2022-10-12T20:04:58Z

packages/router/router.ts

+      // redirect.  We also update the Location header in place in this flow so
+      // basename and relative routing is taken into account
+      if (isStaticRequest) {
+        result.headers.set("Location", location);


If this was a static request, we also need to replace the Response Location header with the updated value before throwing it back to our server.

brophdawg11 · 2022-10-12T20:06:32Z

packages/router/utils.ts

+      (!match.route.index &&
+        match.pathnameBase !== matches[index - 1].pathnameBase)
+  );
+}


Brought over from react-router so relative routing ignores index and pathless layout routes. IF we don't want this duped, I can export this as an undocumented API from @remix-run/router and import it into react-router

brophdawg11 · 2022-10-12T20:09:24Z

packages/router/router.ts

+        navigation.location.pathname === "/"
+          ? basename
+          : joinPaths([basename, navigation.location.pathname]);
+    }


Ended up removing this original approach in startRedirectNavigation since it doesn't work for static requests where we return the Response itself. Moved the primary logic down into callLoaderOrAction

mjackson · 2022-10-20T19:17:17Z

LGTM 👍

cantti · 2022-12-08T09:20:32Z

Hi, it seems like useFetcher load method still does not respect basename.

machour · 2022-12-08T11:00:32Z

@cantti Can you open a new issue for that? 🙏🏼

cantti · 2022-12-09T06:51:36Z

@machour sure, done.

fix: respect basename in loaders and actions redirects (#9418)

b772d47

Co-authored-by: Mikaël ANZANO <[email protected]>

remix-cla-bot bot added the CLA Signed label Oct 11, 2022

brophdawg11 added 2 commits October 11, 2022 17:24

Merge branch 'dev' into brophdawg11/redirect-basename

f832150

fix: support basename and relative routes in redirects

19e798a

brophdawg11 linked an issue Oct 12, 2022 that may be closed by this pull request

[Bug]: Basename is not respected in loaders and actions redirects #9417

Closed

brophdawg11 added 3 commits October 12, 2022 15:39

ci: add tests for data memory router

03a1177

add changeset

2794404

Bump bundle threshold

6d23d9b

brophdawg11 commented Oct 12, 2022

View reviewed changes

convert invariant to 404 for missing routeId

df92829

brophdawg11 commented Oct 12, 2022

View reviewed changes

brophdawg11 changed the title ~~fix: respect basename in loaders and actions redirects~~ fix: support basename and relative routing in loader/action redirects Oct 12, 2022

mjackson approved these changes Oct 20, 2022

View reviewed changes

brophdawg11 added 2 commits October 20, 2022 16:47

Merge branch 'dev' into brophdawg11/redirect-basename

0b56fb5

Bundle bump

d00e5bc

brophdawg11 merged commit 8f684eb into dev Oct 21, 2022

brophdawg11 deleted the brophdawg11/redirect-basename branch October 21, 2022 15:02

brophdawg11 mentioned this pull request Oct 21, 2022

preserve search/hash in processed redirects #9489

Merged

github-actions bot mentioned this pull request Oct 21, 2022

chore: Update version for release (pre) #9490

Merged

github-actions bot mentioned this pull request Nov 1, 2022

chore: Update version for release #9536

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: support basename and relative routing in loader/action redirects #9447

fix: support basename and relative routing in loader/action redirects #9447

brophdawg11 commented Oct 11, 2022 •

edited

Loading

changeset-bot bot commented Oct 11, 2022 •

edited

Loading

brophdawg11 Oct 12, 2022

brophdawg11 Oct 12, 2022

brophdawg11 Oct 12, 2022

brophdawg11 Oct 12, 2022

brophdawg11 Oct 12, 2022

brophdawg11 Oct 12, 2022

brophdawg11 Oct 12, 2022

mjackson commented Oct 20, 2022

cantti commented Dec 8, 2022

machour commented Dec 8, 2022

cantti commented Dec 9, 2022

fix: support basename and relative routing in loader/action redirects #9447

fix: support basename and relative routing in loader/action redirects #9447

Conversation

brophdawg11 commented Oct 11, 2022 • edited Loading

changeset-bot bot commented Oct 11, 2022 • edited Loading

🦋 Changeset detected

brophdawg11 Oct 12, 2022

Choose a reason for hiding this comment

brophdawg11 Oct 12, 2022

Choose a reason for hiding this comment

brophdawg11 Oct 12, 2022

Choose a reason for hiding this comment

brophdawg11 Oct 12, 2022

Choose a reason for hiding this comment

brophdawg11 Oct 12, 2022

Choose a reason for hiding this comment

brophdawg11 Oct 12, 2022

Choose a reason for hiding this comment

brophdawg11 Oct 12, 2022

Choose a reason for hiding this comment

mjackson commented Oct 20, 2022

cantti commented Dec 8, 2022

machour commented Dec 8, 2022

cantti commented Dec 9, 2022

brophdawg11 commented Oct 11, 2022 •

edited

Loading

changeset-bot bot commented Oct 11, 2022 •

edited

Loading