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

feat(react-router): allow useMatch to not throw if match was not found #1738

Merged
merged 1 commit into from
Jul 12, 2024

Conversation

schiller-manuel
Copy link
Contributor

in this case, matchRoute will return undefined

@schiller-manuel schiller-manuel marked this pull request as draft June 12, 2024 23:28
Copy link

nx-cloud bot commented Jun 12, 2024

☁️ Nx Cloud Report

CI is running/has finished running commands for commit ff43790. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution


✅ Successfully ran 3 targets

Sent with 💌 from NxCloud.

@chorobin
Copy link
Contributor

chorobin commented Jun 13, 2024

To me this makes sense, I think matchRoute is not the same because it doesn't provide the same stuff a useMatch and its definitely possible users might want to conditionally render based on a match or not.

I think we just need to think about if select should be called if the match is undefined

@SeanCassiere SeanCassiere changed the title feat: allow useMatch to not throw if match was not found feat(react-router): allow useMatch to not throw if match was not found Jun 13, 2024
@schiller-manuel schiller-manuel force-pushed the feature/useMatch-do-not-throw branch from 4d295a9 to fcfa1ed Compare July 11, 2024 16:25
@schiller-manuel schiller-manuel marked this pull request as ready for review July 11, 2024 16:26
Copy link

pkg-pr-new bot commented Jul 11, 2024

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

commit: ff43790

@tanstack/history

npm i https://pkg.pr.new/@tanstack/history@1738

@tanstack/react-cross-context

npm i https://pkg.pr.new/@tanstack/react-cross-context@1738

@tanstack/react-router

npm i https://pkg.pr.new/@tanstack/react-router@1738

@tanstack/react-router-with-query

npm i https://pkg.pr.new/@tanstack/react-router-with-query@1738

@tanstack/router-cli

npm i https://pkg.pr.new/@tanstack/router-cli@1738

@tanstack/router-devtools

npm i https://pkg.pr.new/@tanstack/router-devtools@1738

@tanstack/router-generator

npm i https://pkg.pr.new/@tanstack/router-generator@1738

@tanstack/router-plugin

npm i https://pkg.pr.new/@tanstack/router-plugin@1738

@tanstack/router-vite-plugin

npm i https://pkg.pr.new/@tanstack/router-vite-plugin@1738

@tanstack/start

npm i https://pkg.pr.new/@tanstack/start@1738

@tanstack/start-vite-plugin

npm i https://pkg.pr.new/@tanstack/start-vite-plugin@1738


templates

> = StrictOrFrom<TFrom, TStrict> & {
select?: (match: TRouteMatch) => TSelected
shouldThrow?: TThrow
Copy link
Member

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 with redirect().

@@ -62,3 +70,17 @@ function Component() {
// ...
}
```

### checking if a specific route is currently rendered
Copy link
Member

Choose a reason for hiding this comment

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

Capitalizing the heading.

} from '../src'
import type { RouteComponent, RouterHistory } from '../src'

describe('useMatch', () => {
Copy link
Member

Choose a reason for hiding this comment

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

We also probably have a type test in a useMatch.test-d.tsx, to check for the return type when the throw flag is set to false vs what it is when true.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, for sure we need the type test aswell

Copy link
Contributor

@chorobin chorobin left a comment

Choose a reason for hiding this comment

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

Add type tests and I think its all good

@schiller-manuel schiller-manuel force-pushed the feature/useMatch-do-not-throw branch from fcfa1ed to 7a16ada Compare July 12, 2024 17:07
in this case, `matchRoute` will return undefined
@schiller-manuel schiller-manuel force-pushed the feature/useMatch-do-not-throw branch from 7a16ada to ff43790 Compare July 12, 2024 17:19
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.

3 participants