Skip to content
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

useRoutes should be able to return null when passing locationArg #9485

Merged
merged 3 commits into from
Oct 25, 2022
Merged

useRoutes should be able to return null when passing locationArg #9485

merged 3 commits into from
Oct 25, 2022

Conversation

danielberndt
Copy link
Contributor

The docs for useRoutes claim the following:

The return value of useRoutes is either a valid React element you can use to render the route tree, or null if nothing matched.

This is not true in case a custom locationArg is passed. In that situation useRoutes will always return a <LocationContext.Provider> with a potentially empty child.

This PR ensures that a <LocationContext.Provider> is only returned if renderedMatches is not empty.

@changeset-bot
Copy link

changeset-bot bot commented Oct 21, 2022

🦋 Changeset detected

Latest commit: 3efe421

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

This PR includes changesets to release 5 packages
Name Type
react-router-dom Patch
@remix-run/router Patch
react-router 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

@remix-cla-bot
Copy link
Contributor

remix-cla-bot bot commented Oct 21, 2022

Hi @danielberndt,

Welcome, and thank you for contributing to React Router!

Before we consider your pull request, we ask that you sign our Contributor License Agreement (CLA). We require this only once.

You may review the CLA and sign it by adding your name to contributors.yml.

Once the CLA is signed, the CLA Signed label will be added to the pull request.

If you have already signed the CLA and received this response in error, or if you have any questions, please contact us at [email protected].

Thanks!

- The Remix team

@remix-cla-bot
Copy link
Contributor

remix-cla-bot bot commented Oct 21, 2022

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

@timdorr
Copy link
Member

timdorr commented Oct 22, 2022

Do you mind adding a test as well?

@danielberndt
Copy link
Contributor Author

Alright, just added tests to ensure useRoutes returns null. One with a custom location prop, and one without

@timdorr timdorr requested a review from brophdawg11 October 24, 2022 18:21
@brophdawg11 brophdawg11 changed the base branch from main to dev October 24, 2022 20:41
@brophdawg11 brophdawg11 changed the base branch from dev to main October 24, 2022 20:41
@brophdawg11
Copy link
Contributor

@danielberndt this is looking good! Do you mind rebasing this and pointing to dev since it has code changes in it?

@brophdawg11 brophdawg11 self-assigned this Oct 24, 2022
@danielberndt danielberndt changed the base branch from main to dev October 24, 2022 21:14
@danielberndt danielberndt changed the base branch from dev to main October 24, 2022 21:15
@danielberndt danielberndt changed the base branch from main to dev October 24, 2022 21:32
@danielberndt
Copy link
Contributor Author

danielberndt commented Oct 24, 2022

Alright @brophdawg11, done!

@brophdawg11 brophdawg11 merged commit 096edeb into remix-run:dev Oct 25, 2022
@danielberndt danielberndt deleted the patch-1 branch October 25, 2022 14:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants