fix _root.data fetch when basename is set, closes #12295

[Aleuck]

This should fix #12295

[changeset-bot]

🦋 Changeset detected

Latest commit: d7f7e91

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

This PR includes changesets to release 11 packages

Name	Type
react-router	Patch
@react-router/architect	Patch
@react-router/cloudflare	Patch
@react-router/dev	Patch
react-router-dom	Patch
@react-router/express	Patch
@react-router/node	Patch
@react-router/serve	Patch
@react-router/fs-routes	Patch
@react-router/remix-routes-option-adapter	Patch
create-react-router	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

[remix-cla-bot]

Thank you for signing the Contributor License Agreement. Let's get this merged! 🥳

[timdorr]

Can you add a test of some sort so we can prevent a regression?

[Aleuck]

Can you add a test of some sort so we can prevent a regression?

Added a couple of tests, @timdorr.
I changed the base to dev, not sure if it is the most appropriate.

[Aleuck]

I am not quite sure if it is possible access the basename when singleFetchUrl is being called in a SSR context. This might not be a complete fix.

The integration tests failed because I forgot to test the availability of window. I just fixed that.

[s-cochrane]

@timdorr, any updates here? We've applied @Aleuck's patch in the meantime with good success, but we would love to see this fix merged upstream. This is going to be a big issue for anyone running a full-stack RR app behind a reverse proxy and not on the base path of the domain. Our use-case is we're using a new RR app to replace an aging legacy app via the strangler-fig pattern.

[timdorr]

I made a request to @brophdawg11 to get a review. Hopefully we can land this for 7.2.0. 🤞

[Aleuck]

I would like to find a way to introduce tests for the createRequestHandler, and also integration tests. But the mocks and fixtures generators are not ready to handle basename and it does not seem to be too simple to make it work.

[brophdawg11]

Ugh, sorry I missed this mention. Between paternity leave late last year and then the holiday break my github notifications are still a mess so I'm not catching these types of mentions. @brookslybrand just called my attention to this and it won't make it into 7.2 but I will look at it right after and we can get into dev for the next release.

[github-actions]

🤖 Hello there,

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

Thanks!

[skrhlm]

howdy! great stuff addressing this, however if i understand correctly this only applies to the case when you're running this using ssr? this fix would not work on a fully prerendered site, being deployed statically?

thanks!