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

ref(types): add default type parameters for RouteComponentProps #85139

Merged
merged 6 commits into from
Feb 14, 2025

Conversation

TkDodo
Copy link
Contributor

@TkDodo TkDodo commented Feb 13, 2025

giving the type RouteComponentProps safe default values for Params and RouteParams avoids us having to always pass them where we don't know / don't want to restrict us to specific params, which is the case in many places. Oftentimes, the type {} was used, which is an unsafe type that we're trying to get rid of and forbid its usage with eslint

With those changes, this PR gets the usages of the empty object type down to 275 from the initial starting point of 465.


The most relevant changes are:

- export interface RouteComponentProps<P, R, ComponentProps = any, Q = any> {
+ export interface RouteComponentProps<
+   P = Record<string, string | undefined>,
+   R = Record<string, string | undefined>,
+   ComponentProps = any,
+   Q = any,
+ > {

Record<string, string | undefined> is a “safe” default value, as it means all params that components get access to from the router can be read in a potentially undefined way.

Usages where we know parameters exist because of route matching can still overwrite this default, which is what we frequently do, e.g.:

type Props = RouteComponentProps<{orgId: string}>;

In this cases, orgId can be read as type string, and it will also be the only key that can be read from params.

some other interfaces had to be widened per default from Record<string, string> to Record<string, string | undefined> so that they are compatible:

- export interface WithRouterProps<P = Record<string, string>, Q = any> {
+ export interface WithRouterProps<P = Record<string, string | undefined>, Q = any> {
- export interface RouteContextInterface<P = Record<string, string>, Q = any> {
+ export interface RouteContextInterface<P = Record<string, string | undefined>, Q = any> {

This change didn’t yield any type errors, but it will make future access more safe.

giving the type RouteComponentProps safe default values for `Params` and `RouteParams` avoids us having to always pass them where we don't know / don't want to restrict us to specific params, which is the case in many places. Oftentimes, the type `{}` was used, which is an unsafe type that we're trying to get rid of and forbid its usage with eslint
@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Feb 13, 2025
@TkDodo TkDodo marked this pull request as ready for review February 13, 2025 12:49
@TkDodo TkDodo requested review from a team as code owners February 13, 2025 12:49
@TkDodo TkDodo requested a review from evanpurkhiser February 13, 2025 15:47
@TkDodo TkDodo enabled auto-merge (squash) February 14, 2025 11:39
@TkDodo TkDodo merged commit 3f3df6d into master Feb 14, 2025
43 checks passed
@TkDodo TkDodo deleted the tkdodo/ref/route-component-props branch February 14, 2025 11:46
Copy link

sentry-io bot commented Feb 17, 2025

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

Did you find this useful? React with a 👍 or 👎

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Scope: Frontend Automatically applied to PRs that change frontend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants