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

Update descendant routes warning message #8202

Merged
merged 1 commit into from Dec 9, 2021

Conversation

msutkowski
Copy link
Contributor

This slightly modifies the error message for silly people like me that might see this, which would also be an error:

image

This would show if you were in the middle of a v5->6 migration and had this:

<Routes>
  <Route to="/" element={<SomeSectionWithLotsOfNestedRoutes />} />
</Routes>

As an aside, it might make sense to do toMatchInlineSnapshot checks of the consoleWarn in the test to cover both of these cases.

@msutkowski msutkowski changed the base branch from main to dev November 4, 2021 06:34
@mjackson
Copy link
Member

mjackson commented Nov 4, 2021

👍 I’d definitely support adding some inline snapshot tests of the two different cases. Would you like to add that to this PR @msutkowski ?

@msutkowski
Copy link
Contributor Author

+1 I’d definitely support adding some inline snapshot tests of the two different cases. Would you like to add that to this PR @msutkowski ?

Will do! I'll tack that on shortly.

expect(consoleWarn.mock.calls[0][0])
.toMatch(`You rendered descendant <Routes> (or called \`useRoutes()\`) at "/" (under <Route path="/">) but the parent route path has no trailing "*". This means if you navigate deeper, the parent won't match anymore and therefore the child routes will never render.

Please change the parent <Route path="/"> to <Route path="/*">.`);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's just tell them to use <Route path="*"> instead of <Route path="/*">.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated!

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

Successfully merging this pull request may close these issues.

None yet

3 participants