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.

Sign up for GitHub

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

Jump to bottom

fix(react): Update types to match react router 6.4 updates #5992

Merged
merged 3 commits into from Oct 20, 2022
Merged

fix(react): Update types to match react router 6.4 updates #5992

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
041f6aa
Update Types to Match React Router Updates
philipatkinson Oct 7, 2022
82fa137
add reasoning about why this change is happening
AbhiPrasad Oct 19, 2022
6fe8bdb
Adjust typing
AbhiPrasad Oct 19, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
35 changes: 28 additions & 7 deletions packages/react/src/reactrouterv6.tsx
View file
Open in desktop
Expand Up @@ -8,14 +8,27 @@ import React from 'react';

import { Action, Location } from './types';

interface RouteObject {
interface NonIndexRouteObject {
caseSensitive?: boolean;
children?: RouteObject[];
element?: React.ReactNode;
index?: boolean;
element?: React.ReactNode | null;
index?: false;
path?: string;
}

interface IndexRouteObject {
caseSensitive?: boolean;
children?: undefined;
element?: React.ReactNode | null;
index?: true;
path?: string;
}

// This type was originally just `type RouteObject = IndexRouteObject`, but this was changed
// in https://github.com/remix-run/react-router/pull/9366, which was released with `6.4.2`
// See https://github.com/remix-run/react-router/issues/9427 for a discussion on this.
type RouteObject = IndexRouteObject | NonIndexRouteObject;
Copy link
Member

@mydea mydea Oct 20, 2022 •
edited

Choose a reason for hiding this comment

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

L: I guess to reduce duplication this could also be something like:

interface BaseRouteObject {
  caseSensitive?: boolean;
  element?: React.ReactNode | null;
  path?: string;
}

type IndexRouteObject = BaseRouteObject & { index: true, children?: undefined };
type NonIndexRouteObject = BaseRouteObject & { index: false, children?: RouteObject[]; };
type RouteObject = IndexRouteObject | NonIndexRouteObject;

But not sure if it is really worth it, or just adds unnecessary complexity 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

I just kept the types from what was done in #5915, you're change is cleaner, but I'll just keep it like this for now. We can update this afterwards if we need to.


type Params<Key extends string = string> = {
readonly [key in Key]: string | undefined;
};
Expand Down Expand Up @@ -45,8 +58,16 @@ interface RouteMatch<ParamKey extends string = string> {
type UseEffect = (cb: () => void, deps: unknown[]) => void;
type UseLocation = () => Location;
type UseNavigationType = () => Action;
type CreateRoutesFromChildren = (children: JSX.Element[]) => RouteObject[];
type MatchRoutes = (routes: RouteObject[], location: Location) => RouteMatch[] | null;

// For both of these types, use `any` instead of `RouteObject[]` or `RouteMatch[]`.
// Have to do this so we maintain backwards compatability between
// react-router > 6.0.0 and >= 6.4.2.
// eslint-disable-next-line @typescript-eslint/no-explicit-any
type RouteObjectArrayAlias = any;
// eslint-disable-next-line @typescript-eslint/no-explicit-any
type RouteMatchAlias = any;
type CreateRoutesFromChildren = (children: JSX.Element[]) => RouteObjectArrayAlias;
type MatchRoutes = (routes: RouteObjectArrayAlias, location: Location) => RouteMatchAlias[] | null;

let activeTransaction: Transaction | undefined;

Expand Down Expand Up @@ -106,7 +127,7 @@ function getNormalizedName(
return [location.pathname, 'url'];
}

const branches = matchRoutes(routes, location);
const branches = matchRoutes(routes, location) as unknown as RouteMatch[];

let pathBuilder = '';
if (branches) {
Expand Down Expand Up @@ -209,7 +230,7 @@ export function withSentryReactRouterV6Routing<P extends Record<string, any>, R
_useEffect(() => {
// Performance concern:
// This is repeated when <Routes /> is rendered.
routes = _createRoutesFromChildren(props.children);
routes = _createRoutesFromChildren(props.children) as RouteObject[];
isBaseLocation = true;

updatePageloadTransaction(location, routes);
Expand Down