-
-
Notifications
You must be signed in to change notification settings - Fork 705
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
feat(react-router): allow useMatch
to not throw if match was not found
#1738
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,54 @@ | ||
import { describe, expectTypeOf, test } from 'vitest' | ||
import { createRootRoute, createRoute, createRouter, useMatch } from '../src' | ||
import type { MakeRouteMatch } from '../src/Matches' | ||
|
||
const rootRoute = createRootRoute() | ||
|
||
const indexRoute = createRoute({ | ||
getParentRoute: () => rootRoute, | ||
path: '/', | ||
}) | ||
|
||
const invoicesRoute = createRoute({ | ||
getParentRoute: () => rootRoute, | ||
path: 'invoices', | ||
}) | ||
|
||
const routeTree = rootRoute.addChildren([invoicesRoute, indexRoute]) | ||
|
||
const defaultRouter = createRouter({ routeTree }) | ||
|
||
type DefaultRouter = typeof defaultRouter | ||
|
||
type TRouteMatch = MakeRouteMatch<DefaultRouter['routeTree']> | ||
|
||
describe('useMatch', () => { | ||
const from = '/invoices' | ||
test('return type is `RouteMatch` when shouldThrow = true', () => { | ||
const shouldThrow = true | ||
const match = useMatch< | ||
DefaultRouter['routeTree'], | ||
typeof from, | ||
true, | ||
TRouteMatch, | ||
TRouteMatch, | ||
typeof shouldThrow | ||
>({ from, shouldThrow }) | ||
|
||
expectTypeOf(match).toEqualTypeOf<TRouteMatch>() | ||
}) | ||
|
||
test('return type is `RouteMatch | undefined` when shouldThrow = false', () => { | ||
const shouldThrow = false | ||
const match = useMatch< | ||
DefaultRouter['routeTree'], | ||
typeof from, | ||
true, | ||
TRouteMatch, | ||
TRouteMatch, | ||
typeof shouldThrow | ||
>({ from, shouldThrow }) | ||
|
||
expectTypeOf(match).toEqualTypeOf<TRouteMatch | undefined>() | ||
}) | ||
}) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,113 @@ | ||
import { afterEach, describe, expect, it, test, vi } from 'vitest' | ||
import '@testing-library/jest-dom/vitest' | ||
import React from 'react' | ||
import { render, screen } from '@testing-library/react' | ||
import { | ||
Link, | ||
Outlet, | ||
RouterProvider, | ||
createMemoryHistory, | ||
createRootRoute, | ||
createRoute, | ||
createRouter, | ||
useMatch, | ||
} from '../src' | ||
import type { RouteComponent, RouterHistory } from '../src' | ||
|
||
describe('useMatch', () => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We also probably have a type test in a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, for sure we need the type test aswell |
||
function setup({ | ||
RootComponent, | ||
history, | ||
}: { | ||
RootComponent: RouteComponent | ||
history?: RouterHistory | ||
}) { | ||
const rootRoute = createRootRoute({ | ||
component: RootComponent, | ||
}) | ||
const indexRoute = createRoute({ | ||
getParentRoute: () => rootRoute, | ||
path: '/', | ||
component: () => ( | ||
<React.Fragment> | ||
<h1>IndexTitle</h1> | ||
<Link to="/posts">Posts</Link> | ||
</React.Fragment> | ||
), | ||
}) | ||
|
||
const postsRoute = createRoute({ | ||
getParentRoute: () => rootRoute, | ||
path: '/posts', | ||
component: () => <h1>PostsTitle</h1>, | ||
}) | ||
|
||
const router = createRouter({ | ||
routeTree: rootRoute.addChildren([indexRoute, postsRoute]), | ||
history, | ||
}) | ||
|
||
render(<RouterProvider router={router} />) | ||
} | ||
|
||
describe('when match is found', () => { | ||
test.each([true, false, undefined])( | ||
'returns the match if shouldThrow = %s', | ||
async (shouldThrow) => { | ||
function RootComponent() { | ||
const match = useMatch({ from: '/posts', shouldThrow }) | ||
expect(match).toBeDefined() | ||
expect(match!.routeId).toBe('/posts') | ||
return <Outlet /> | ||
} | ||
|
||
setup({ | ||
RootComponent, | ||
history: createMemoryHistory({ initialEntries: ['/posts'] }), | ||
}) | ||
await screen.findByText('PostsTitle') | ||
}, | ||
) | ||
}) | ||
|
||
describe('when match is not found', () => { | ||
test.each([undefined, true])( | ||
'throws if shouldThrow = %s', | ||
async (shouldThrow) => { | ||
function RootComponent() { | ||
useMatch({ from: '/posts', shouldThrow }) | ||
return <Outlet /> | ||
} | ||
setup({ RootComponent }) | ||
expect( | ||
await screen.findByText( | ||
'Invariant failed: Could not find an active match from "/posts"', | ||
), | ||
).toBeInTheDocument() | ||
}, | ||
) | ||
|
||
describe('returns undefined if shouldThrow = false', () => { | ||
test('without select function', async () => { | ||
function RootComponent() { | ||
const match = useMatch({ from: 'posts', shouldThrow: false }) | ||
expect(match).toBeUndefined() | ||
return <Outlet /> | ||
} | ||
setup({ RootComponent }) | ||
expect(await screen.findByText('IndexTitle')).toBeInTheDocument() | ||
}) | ||
test('with select function', async () => { | ||
const select = vi.fn() | ||
function RootComponent() { | ||
const match = useMatch({ from: 'posts', shouldThrow: false, select }) | ||
expect(match).toBeUndefined() | ||
return <Outlet /> | ||
} | ||
setup({ RootComponent }) | ||
expect(await screen.findByText('IndexTitle')).toBeInTheDocument() | ||
expect(select).not.toHaveBeenCalled() | ||
}) | ||
}) | ||
}) | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd rather this be
throw
to match that API surface with what we are already doing withredirect()
.