fix: Wrap route with location arg in location context

[johnpangalos]

In order for the useLocation hook to work with the use of the modal pattern, routes with the locationArg prop are wrapped in a LocationContext. This is done conditionally in order to ensure performance in the default case doesn't change.

closes #8470

[changeset-bot]

🦋 Changeset detected

Latest commit: 29f273b

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

This PR includes changesets to release 4 packages

Name	Type
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

[johnpangalos]

Not sure how changesets are meant to work, they aren't mentioned in contributing document but the changeset bot has kindly reminded me about them.

[johnpangalos]

I also bumped the file sizes half a kB as it seems that the code I added is now breaking that limit. Is there any guidance as what should be done when reaching that limit?

[timdorr]

For the changesets, just follow the links in the comment above. This change should result in a patch version bump (it fixes a bug), so I would add one.

As for the size thing, that's fine to bump up just a little. It's mainly a protection against adding something stupidly huge in a PR without anyone noticing.

[brophdawg11]

Thanks for the PR @johnpangalos! I left a comment over on #8470 but I'll ping Michael and Ryan on this as well to get their take

[flying-sheep]

@mjackson, @ryanflorence, could you please take a look? I think a lot of people are eagerly waiting for this fix!

[brophdawg11]

Thanks @johnpangalos! Made a few small updates, but this looking good and I'll merge shortly

[johnpangalos]

Sounds good, glad to see this merged! Thanks @brophdawg11

[gnapse]

Hi, there's something about this change that I wanted to ask about: having this bifurcation in the code, rendering two different component trees depending on whether there's a location prop provided or not, has a troublesome implication: the entire tree inside the routes is re-built. Not merely re-rendered, but components are discarded, and new ones are mounted.

To illustrate, check at this short screen recording where I show how the component tree re-rendered in full, because the tree change after having introduced a Location.Provider in between the parent Routes and the child RenderedRoute:

CleanShot.2024-08-07.at.14.19.29.mp4

Note how before the rendering commit took place, the tree starts with Routes -> RenderedRoute, and after the commit took place, the structure is now Routes -> Location.Provider -> RenderedRoute. This causes RenderedRoute and everything below it to not merely update, but to be unmounted, and re-created entirely. This in turn causes elements to be re-created, which can cause state in them to be lost.

This is understandable due to how React reconciliation works. But it is undesirable, due to the potential to lose inner component state in this entire sub-tree.

Proposed solution

What if we always wrap the return value of useRoutes inside a Location.Provider?