fix(react): Update types to match react router 6.4 updates#5992
fix(react): Update types to match react router 6.4 updates#5992AbhiPrasad merged 3 commits intomastergetsentry/sentry-javascript:masterfrom abhi-update-react-router-typesgetsentry/sentry-javascript:abhi-update-react-router-typesCopy head branch name to clipboard
Conversation
Fixes a type mismatch caused by change remix-run/react-router#9366 in react-router code.
Lms24
left a comment
There was a problem hiding this comment.
LGTM, out of curiosity, you said that it's not very easy to maintain backwards compatiblity if we didn't use any but a stronger type. What would we need to do to get that stronger type?
Not saying it needs to be done (consider this a logaf extra-l) but since we try to avoid any as much as possible, I was just curious.
| // 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; |
There was a problem hiding this comment.
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 🤔
There was a problem hiding this comment.
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.
The problem here is that with TS 3.8.3, TypeScript isn't smart enough to discriminate the union |
Supersedes #5915'
Fixes #5959
Fixes a type mismatch caused by change remix-run/react-router#9366 in react-router code. Have to use
anybecause of typing changes in the above PR, since we can't be backwards compatible with both versions very easily.