Skip to content

[Bug]: RR7 - With basename configured, .data requests against root index not correctly routed #12295

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
mpqmpqm opened this issue Nov 15, 2024 · 18 comments · Fixed by #12898
Closed
Labels

Comments

@mpqmpqm
Copy link

mpqmpqm commented Nov 15, 2024

What version of React Router are you using?

7

Steps to Reproduce

Minimal repro

  1. Configure vite.config.base and vite.config.reactRouter.basename.
  2. Add a Link component to the root index, or an action against the root index.
  3. Observe that .data requests against root index are routed to localhost:5173/${basename}.data, rather than e.g. localhost:5173/${basename}/_root.data.
  4. Observe that .data requests against root index 404.

Expected Behavior

.data requests against root index should resolve accounting for configured basename.

Actual Behavior

.data requests against root index 404. .data requests against nested routes resolve as expected.

In minimal repro above, try clicking links to Home and submitting action on that route. Then try the same at About. Observe that About works while Home (root index) doesn't.

@mpqmpqm mpqmpqm added the bug label Nov 15, 2024
@mpqmpqm
Copy link
Author

mpqmpqm commented Nov 18, 2024

Here are the patches I issued to fix this in my Remix 2.14.0 project:

diff --git a/node_modules/@remix-run/react/dist/esm/single-fetch.js b/node_modules/@remix-run/react/dist/esm/single-fetch.js
index 96f10cd..4126369 100644
--- a/node_modules/@remix-run/react/dist/esm/single-fetch.js
+++ b/node_modules/@remix-run/react/dist/esm/single-fetch.js
@@ -300,6 +300,8 @@ function singleFetchUrl(reqUrl) {
   let url = typeof reqUrl === "string" ? new URL(reqUrl, window.location.origin) : reqUrl;
   if (url.pathname === "/") {
     url.pathname = "_root.data";
+  } else if (url.pathname === window.__remixContext.basename) {
+    url.pathname = window.__remixContext.basename.concat("_root.data");
   } else {
     url.pathname = `${url.pathname.replace(/\/$/, "")}.data`;
   }
diff --git a/node_modules/@remix-run/server-runtime/dist/server.js b/node_modules/@remix-run/server-runtime/dist/server.js
index 7e639d9..f2af2db 100644
--- a/node_modules/@remix-run/server-runtime/dist/server.js
+++ b/node_modules/@remix-run/server-runtime/dist/server.js
@@ -128,7 +128,12 @@ const createRequestHandler = (build, mode$1) => {
       }
     } else if (_build.future.v3_singleFetch && url.pathname.endsWith(".data")) {
       let handlerUrl = new URL(request.url);
-      handlerUrl.pathname = handlerUrl.pathname.replace(/\.data$/, "").replace(/^\/_root$/, "/");
+      handlerUrl.pathname = handlerUrl.pathname.replace(/\.data$/, "").replace(
+        _build.basename ?
+          /\/_root$/ :
+          /^\/_root$/,
+        "/"
+      );
       let singleFetchMatches = routeMatching.matchServerRoutes(routes, handlerUrl.pathname, _build.basename);
       response = await handleSingleFetchRequest(serverMode, _build, staticHandler, request, handlerUrl, loadContext, handleError);
       if (_build.entry.module.handleDataRequest) {

@LeoDanielsson
Copy link

LeoDanielsson commented Dec 14, 2024

We ran into this issue when upgrading our Remix app (without single fetch) to RR7.

It works locally with basename configured but failed in the production environment since our reverse proxy wouldn't route /my-app.data requests to our application as it's technically a different path than /my-app.

We ended up working around it by configuring our reverse proxy to route /my-app.data to the app (in addition to /my-app/*).

It works, but it would definitely be cleaner if the requests were /my-app/_root.data

@nikrad7
Copy link

nikrad7 commented Dec 17, 2024

Hello there! We encounter same issue after enable singleFetch future at our roadmap to react-router 7.

We use cookie based remix-auth authorization with cookie path option (which equals /basename). Since index route is secure and protected by auth middleware, this bug leads to infinite redirect loop between index page and login page. Problem is that browser will not append cookies header for request with /basename.data since it doesn't match path option, and auth middleware will return singleFetch client redirect to login page route. And for second client request cookies will be sended, since login route match browser cookie path logic (/basename/login), and result of request will be redirect back to index page.

Workaroung
Patch provided by @mpqmpqm is worked, but with slight additional fixes (we at latest, remix 2.15.1 version):

  1. basename may be without trailing slash, so we ensure that forward slash will be added before concat basename and _root.data patname postfix (at single-fetch.js):
function singleFetchUrl(reqUrl) {
  let url = typeof reqUrl === "string" ? new URL(reqUrl, window.location.origin) : reqUrl;
  const basename = window.__remixContext.basename;
  if (url.pathname === "/") {
    url.pathname = "_root.data";
  } else if (basename && (url.pathname === basename || url.pathname === `${basename}/`)) {
    url.pathname = basename.endsWith("/") ? basename.concat("_root.data") : basename.concat("/_root.data");
  } else {
    url.pathname = `${url.pathname.replace(/\/$/, "")}.data`;
  }
  return url;
}
  1. patch both files (server.js, single-fetch.js) code entries at dist and dist\esm. At least when using pnpm as repo and patch utility.

@skrhlm
Copy link

skrhlm commented Dec 18, 2024

could possibly be related to this?
#12536

@mattoni
Copy link

mattoni commented Jan 3, 2025

I'm also running into this issue after migrating a remix app into RR7. I noticed it when using the browser's back button to go back to the entry page, and getting a 404. Also does not happen in dev, but only in a production build.

@manzaloros
Copy link

manzaloros commented Jan 9, 2025

I'm also having this issue.

I noticed it when using the browser's back button to go back to the entry page, and getting a 404. Also does not happen in dev, but only in a production build.

It happens in Remix v2 latest when the v3_singleFetch is enabled. We have a baseURL defined:

// vite.config.ts
  remix({
        basename: "/my-basename/",

This is a blocker to upgrade from Remix to React Router 7.

Is there any fix for this, other than a patch?

@manzaloros
Copy link

manzaloros commented Jan 9, 2025

@ryanflorence , is the team aware of this bug / is there any fix planned?

I attempted to patch React Router, but this didn't solve the 404 issue with a basename:

index 53d57562cbbe0a80bc80a88fc930b84679c805e2..73c737041b92b573ad9a77492412315ffdbc63d7 100644
--- a/dist/production/index.js
+++ b/dist/production/index.js
@@ -5952,8 +5952,11 @@ function singleFetchUrl(reqUrl) {
     // don't assume window is available
     typeof window === "undefined" ? "server://singlefetch/" : window.location.origin
   ) : reqUrl;
+  const basename = window.__reactRouterContext.basename;
   if (url.pathname === "/") {
     url.pathname = "_root.data";
+  } else if (basename && (url.pathname === basename || url.pathname === `${basename}/`)) {
+    url.pathname = basename.endsWith("/") ? basename.concat("_root.data") : basename.concat("/_root.data");
   } else {
     url.pathname = `${url.pathname.replace(/\/$/, "")}.data`;
   }
@@ -9152,7 +9155,12 @@ var createRequestHandler = (build, mode) => {
     let response;
     if (url.pathname.endsWith(".data")) {
       let handlerUrl = new URL(request.url);
-      handlerUrl.pathname = handlerUrl.pathname.replace(/\.data$/, "").replace(/^\/_root$/, "/");
+      handlerUrl.pathname = handlerUrl.pathname.replace(/\.data$/, "").replace(
+                _build.basename ?
+                  /\/_root$/ :
+                  /^\/_root$/,
+                "/"
+      );
       let singleFetchMatches = matchServerRoutes(
         routes,
         handlerUrl.pathname,

@skrhlm
Copy link

skrhlm commented Jan 10, 2025

My thought here was that the x.data-files should be moved inside of it's corresponding path folder, so that each page would be self-contained no matter what basename configuration is chosen.

For example, this structure:

- about.data
- about/
- - index.html
- - contact/
- - - index.html
- - contact.data

would instead be:

- about/
- - index.html
- - about.data
- - contact/
- - - index.html
- - - contact.data

@rjaguilar
Copy link

rjaguilar commented Jan 27, 2025

I'm having the same issue with the additional issue that my form action that is set to /${basename} gets sent to /${basename}.data instead of /${basename}/_root.data

@Aleuck
Copy link
Contributor

Aleuck commented Jan 28, 2025

For React Router v7, I had to create this patch (based on @mpqmpqm's) for it to work consistently in local and production environments.

diff --git a/node_modules/react-router/dist/development/chunk-SYFQ2XB5.mjs b/node_modules/react-router/dist/development/chunk-SYFQ2XB5.mjs
index 25fce4e..85d529b 100644
--- a/node_modules/react-router/dist/development/chunk-SYFQ2XB5.mjs
+++ b/node_modules/react-router/dist/development/chunk-SYFQ2XB5.mjs
@@ -5811,6 +5811,8 @@ function singleFetchUrl(reqUrl) {
   ) : reqUrl;
   if (url.pathname === "/") {
     url.pathname = "_root.data";
+  } else if (url.pathname === window.__reactRouterContext.basename) {
+    url.pathname = window.__reactRouterContext.basename.concat("_root.data");
   } else {
     url.pathname = `${url.pathname.replace(/\/$/, "")}.data`;
   }
@@ -9009,7 +9011,12 @@ var createRequestHandler = (build, mode) => {
     let response;
     if (url.pathname.endsWith(".data")) {
       let handlerUrl = new URL(request.url);
-      handlerUrl.pathname = handlerUrl.pathname.replace(/\.data$/, "").replace(/^\/_root$/, "/");
+      handlerUrl.pathname = handlerUrl.pathname.replace(/\.data$/, "").replace(
+        _build.basename ?
+          /\/_root$/ :
+          /^\/_root$/,
+        "/"
+      );
       let singleFetchMatches = matchServerRoutes(
         routes,
         handlerUrl.pathname,
diff --git a/node_modules/react-router/dist/development/dom-export.js b/node_modules/react-router/dist/development/dom-export.js
index 59c9a78..af2dbfb 100644
--- a/node_modules/react-router/dist/development/dom-export.js
+++ b/node_modules/react-router/dist/development/dom-export.js
@@ -4471,6 +4471,8 @@ function singleFetchUrl(reqUrl) {
   ) : reqUrl;
   if (url.pathname === "/") {
     url.pathname = "_root.data";
+  } else if (url.pathname === window.__reactRouterContext.basename) {
+    url.pathname = window.__reactRouterContext.basename.concat("_root.data");
   } else {
     url.pathname = `${url.pathname.replace(/\/$/, "")}.data`;
   }
diff --git a/node_modules/react-router/dist/development/index.js b/node_modules/react-router/dist/development/index.js
index 46834a0..42c686c 100644
--- a/node_modules/react-router/dist/development/index.js
+++ b/node_modules/react-router/dist/development/index.js
@@ -5956,6 +5956,8 @@ function singleFetchUrl(reqUrl) {
   ) : reqUrl;
   if (url.pathname === "/") {
     url.pathname = "_root.data";
+  } else if (url.pathname === window.__reactRouterContext.basename) {
+    url.pathname = window.__reactRouterContext.basename.concat("_root.data");
   } else {
     url.pathname = `${url.pathname.replace(/\/$/, "")}.data`;
   }
@@ -9154,7 +9156,12 @@ var createRequestHandler = (build, mode) => {
     let response;
     if (url.pathname.endsWith(".data")) {
       let handlerUrl = new URL(request.url);
-      handlerUrl.pathname = handlerUrl.pathname.replace(/\.data$/, "").replace(/^\/_root$/, "/");
+      handlerUrl.pathname = handlerUrl.pathname.replace(/\.data$/, "").replace(
+        _build.basename ?
+          /\/_root$/ :
+          /^\/_root$/,
+        "/"
+      );
       let singleFetchMatches = matchServerRoutes(
         routes,
         handlerUrl.pathname,
diff --git a/node_modules/react-router/dist/production/chunk-OKQ6KMOJ.mjs b/node_modules/react-router/dist/production/chunk-OKQ6KMOJ.mjs
index 3d1b632..535fe32 100644
--- a/node_modules/react-router/dist/production/chunk-OKQ6KMOJ.mjs
+++ b/node_modules/react-router/dist/production/chunk-OKQ6KMOJ.mjs
@@ -5811,6 +5811,8 @@ function singleFetchUrl(reqUrl) {
   ) : reqUrl;
   if (url.pathname === "/") {
     url.pathname = "_root.data";
+  } else if (url.pathname === window.__reactRouterContext.basename) {
+    url.pathname = window.__reactRouterContext.basename.concat("_root.data");
   } else {
     url.pathname = `${url.pathname.replace(/\/$/, "")}.data`;
   }
@@ -9009,7 +9011,12 @@ var createRequestHandler = (build, mode) => {
     let response;
     if (url.pathname.endsWith(".data")) {
       let handlerUrl = new URL(request.url);
-      handlerUrl.pathname = handlerUrl.pathname.replace(/\.data$/, "").replace(/^\/_root$/, "/");
+      handlerUrl.pathname = handlerUrl.pathname.replace(/\.data$/, "").replace(
+        _build.basename ?
+          /\/_root$/ :
+          /^\/_root$/,
+        "/"
+      );
       let singleFetchMatches = matchServerRoutes(
         routes,
         handlerUrl.pathname,
diff --git a/node_modules/react-router/dist/production/dom-export.js b/node_modules/react-router/dist/production/dom-export.js
index 8d83202..cac7673 100644
--- a/node_modules/react-router/dist/production/dom-export.js
+++ b/node_modules/react-router/dist/production/dom-export.js
@@ -4471,6 +4471,8 @@ function singleFetchUrl(reqUrl) {
   ) : reqUrl;
   if (url.pathname === "/") {
     url.pathname = "_root.data";
+  } else if (url.pathname === window.__reactRouterContext.basename) {
+    url.pathname = window.__reactRouterContext.basename.concat("_root.data");
   } else {
     url.pathname = `${url.pathname.replace(/\/$/, "")}.data`;
   }
diff --git a/node_modules/react-router/dist/production/index.js b/node_modules/react-router/dist/production/index.js
index d5cace8..1f1138a 100644
--- a/node_modules/react-router/dist/production/index.js
+++ b/node_modules/react-router/dist/production/index.js
@@ -5956,6 +5956,8 @@ function singleFetchUrl(reqUrl) {
   ) : reqUrl;
   if (url.pathname === "/") {
     url.pathname = "_root.data";
+  } else if (url.pathname === window.__reactRouterContext.basename) {
+    url.pathname = window.__reactRouterContext.basename.concat("_root.data");
   } else {
     url.pathname = `${url.pathname.replace(/\/$/, "")}.data`;
   }
@@ -9154,7 +9156,12 @@ var createRequestHandler = (build, mode) => {
     let response;
     if (url.pathname.endsWith(".data")) {
       let handlerUrl = new URL(request.url);
-      handlerUrl.pathname = handlerUrl.pathname.replace(/\.data$/, "").replace(/^\/_root$/, "/");
+      handlerUrl.pathname = handlerUrl.pathname.replace(/\.data$/, "").replace(
+        _build.basename ?
+          /\/_root$/ :
+          /^\/_root$/,
+        "/"
+      );
       let singleFetchMatches = matchServerRoutes(
         routes,
         handlerUrl.pathname,

@cmatlock-comp
Copy link

I am still experiencing this issue, any updates on a patch for this?

@cmatlock-comp
Copy link

cmatlock-comp commented Jan 29, 2025

For React Router v7, I had to create this patch (based on @mpqmpqm's) for it to work consistently in local and production environments.

Can verify this works, used patch-package https://www.npmjs.com/package/patch-package. Thank You!!! @Aleuck

@brophdawg11
Copy link
Contributor

This should be resolved by #12898 and will be included in the next release

@brophdawg11 brophdawg11 added the awaiting release This issue has been fixed and will be released soon label Feb 19, 2025
@timdorr timdorr marked this as a duplicate of #13096 Feb 23, 2025
@manzaloros
Copy link

@brophdawg11 , thanks for the update.

On 7.2.0 I'm still getting the 404 error when following a link to the root route. I have a basename configured in the react-router.config.ts.

@s-cochrane
Copy link

@manzaloros, from what I can tell this didn't make the 7.2 cut. This was merged into dev right after 7.2 was cut for a release. I have my eye on this one as well, but I think we'll be waiting for 7.3.

Copy link
Contributor

github-actions bot commented Mar 6, 2025

🤖 Hello there,

We just published version 7.3.0 which involves this issue. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

@manzaloros
Copy link

@s-cochrane , thanks! After upgrading to 7.3.0 it looks like the issue is fixed!

Just curious: I made a quick RR7 app from scratch before 7.3.0 and didn't notice this issue occurring. Why did it occur only on my app that I upgraded from Remix?

@s-cochrane
Copy link

@manzaloros - I'm not sure about the specifics of your setup, but we observed the issues when we put RR behind a reverse-proxy. In our case we're replacing a legacy application with a new RR app, and we can't move the legacy app off of the base URL. Because the RR app was being routed to via the reverse proxy it made the break very obvious for us. The app would work if we opened the app directly, but as soon as we put it behind the reverse proxy the issues would surface. Instead of routing static assets and data requests on top of the basename path, it would incorrectly add those paths to the basename. This link above explains the issue we were seeing exactly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.