fix: Wrap route with location arg in location context #9094
Conversation
🦋 Changeset detectedLatest commit: 29f273b The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
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 |
|
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. |
|
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? |
|
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. |
johnpangalos
changed the title
|
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 |
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.
|
@mjackson, @ryanflorence, could you please take a look? I think a lot of people are eagerly waiting for this fix! |
|
Thanks @johnpangalos! Made a few small updates, but this looking good and I'll merge shortly |
|
Sounds good, glad to see this merged! Thanks @brophdawg11 |
In order for the
useLocationhook to work with the use of the modal pattern, routes with thelocationArgprop are wrapped in aLocationContext. This is done conditionally in order to ensure performance in the default case doesn't change.closes #8470