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

[react-router] ExtractRouteParams breaks with custom matchers #52914

Closed
maraisr opened this issue May 13, 2021 · 8 comments
Closed

[react-router] ExtractRouteParams breaks with custom matchers #52914

maraisr opened this issue May 13, 2021 · 8 comments

Comments

@maraisr
Copy link
Contributor

maraisr commented May 13, 2021

React-rotuer's generatePath method just proxies path-to-regexp, which supports custom matchers.

Currently the typescript fails when asking for generatePath('/abc/:xyz([a-z])?', { xyz: 'a'}) <- should be valid. But fails.

cc @mrk21 @vasek17 @ngbrown @awendland @KostyaEsmukov @johnnyreilly @LKay @DovydasNavickas @Nguyen @Grmiade @DaIgeb @egorshulga @rraina @t49tran @8enSmith @wezleytsai @eps1lon @HipsterBrown @pawfa

@beaucollins
Copy link

beaucollins commented May 27, 2021

Some patterns now may be possible with #52712 however it doesn't cover all cases. An example:

generatePath('/abc/:xyz(-[^/]{1,})?', {xyz: '-foo' });

This fails because the initial split uses / which is also a character you can put in your pattern.

: T extends `${infer _Start}:${infer ParamWithOptionalRegExp}/${infer Rest}`

Unfortunately this seems to be going down the hole of implementing RegExp pattern matching in TypeScript string literals. 😱

@maraisr
Copy link
Contributor Author

maraisr commented May 29, 2021

Perhaps we remove these type restrictions, and have like a /strict.d.ts consumers can ///reference if they'd like?

The problem with the forward slash seems mute 👀, can't it just infer like ${infer a}(${string})${infer b} and then recurse that, and then handle if b starts with a ?. The stuff in the brackets is "who cares", so infer up until it.

That'll solve this no?

@beaucollins
Copy link

That'll solve this no?

Possibly. Start with the most specific pattern for extends ${infer ...} and handle each less specific one.

The key names may also need to be cleaned up. There's also the technical possibility of having a ) in you pattern, however that probably isn't a problem because the ) will be URL escaped in practice.

@maraisr
Copy link
Contributor Author

maraisr commented Jun 1, 2021

@beaucollins thanks for jumping in this! 🥰

@beaucollins
Copy link

It's @lmichelin who landed #52712 so thanks to them!

@justingrant
Copy link
Contributor

I just upgraded my project to post-#52712 types (@types/react-router@5.1.16) and I'm now getting TS errors that may be related to #52914 (comment) above. The first segment in the path seems to be extracted correctly, but the second segment is extracted with the prop name being extracted as the whole regex (e.g. 'd(\\\\d{8})(' instead of just d).

I assume that the problem is that the final forward-slash in my URL pattern is optional ((/)?) and it's being interpreted as a separator.

Any ideas for how to work around this? cc @lmichelin @beaucollins

Here's the relevant lines of my JSX:

<Route
  path="/times/:v(day|list)/:d(\\d{8})(/)?"
  render={(routeProps: RouteComponentProps<{ d: string; v: string }>) => {

The render prop gives me this error:

No overload matches this call.
  Overload 1 of 2, '(props: (RouteProps<"/times/:v(day|list)/:d(\\\\d{8})(/)?", { v: string; } & { "d(\\\\d{8})(": string; }> & OmitNative<{}, keyof RouteProps<string, { [x: string]: string | undefined; }>>) | Readonly<...>): Route<...>', gave the following error.
    Type '(routeProps: RouteComponentProps<{    d: string;    v: string;}>) => JSX.Element' is not assignable to type '(props: RouteComponentProps<{ v: string; } & { "d(\\\\d{8})(": string; }, StaticContext, unknown>) => ReactNode'.
      Types of parameters 'routeProps' and 'props' are incompatible.
        Type 'RouteComponentProps<{ v: string; } & { "d(\\\\d{8})(": string; }, StaticContext, unknown>' is not assignable to type 'RouteComponentProps<{ d: string; v: string; }, StaticContext, unknown>'.
          Property 'd' is missing in type '{ v: string; } & { "d(\\\\d{8})(": string; }' but required in type '{ d: string; v: string; }'.
  Overload 2 of 2, '(props: RouteProps<"/times/:v(day|list)/:d(\\\\d{8})(/)?", { v: string; } & { "d(\\\\d{8})(": string; }> & OmitNative<{}, keyof RouteProps<string, { [x: string]: string | undefined; }>>, context: any): Route<...>', gave the following error.
    Type '(routeProps: RouteComponentProps<{    d: string;    v: string;}>) => JSX.Element' is not assignable to type '(props: RouteComponentProps<{ v: string; } & { "d(\\\\d{8})(": string; }, StaticContext, unknown>) => ReactNode'.ts(2769)
Pages.tsx(124, 50): 'd' is declared here.
index.d.ts(92, 5): The expected type comes from property 'render' which is declared here on type 'IntrinsicAttributes & IntrinsicClassAttributes<Route<{}, "/times/:v(day|list)/:d(\\\\d{8})(/)?">> & Readonly<RouteProps<"/times/:v(day|list)/:d(\\\\d{8})(/)?", { ...; } & { ...; }> & OmitNative<...>> & Readonly<...>'
index.d.ts(92, 5): The expected type comes from property 'render' which is declared here on type 'IntrinsicAttributes & IntrinsicClassAttributes<Route<{}, "/times/:v(day|list)/:d(\\\\d{8})(/)?">> & Readonly<RouteProps<"/times/:v(day|list)/:d(\\\\d{8})(/)?", { ...; } & { ...; }> & OmitNative<...>> & Readonly<...>'
(JSX attribute) render?: ((props: RouteComponentProps<{
    v: string;
} & {
    "d(\\\\d{8})(": string;
}, StaticContext, unknown>) => ReactNode) | undefined

@beaucollins
Copy link

Any ideas for how to work around this?

@justingrant You can cast it to something that ExtractRouteParams can handle as a bit of a hack to unblock yourself.

<Route
  path={"/times/:v(day|list)/:d(\\d{8})(/)?" as "/times/:v/:d"}
  render={(routeProps: RouteComponentProps<{ d: string; v: string }>) => {

https://www.typescriptlang.org/play?#code/JYWwDg9gTgLgBAJQKYEMDG8BmUIjgcilQ3wG4BYAKFEljgG9EIBXGJAGidaQGFdIAdkgEwACjjABnOAF842XASLoYAWhzcoZKjsoAeBCzZU4cMChgALALz0ARAHoYoJJIcAuAG4AKACYoATwAfABtgSRgASg9fbwAdON96AA4ZSO8HSIB+OzgUaUdnEFcPTxi7GRM4IgFfJChbbw02cQgpdy42PnAIIRFWqT1GXw6IqGABAHNSOE9RmHGp2QA+SLhrZYYq01MiGGYoATg9ZYAJJBCQiD0HZaqZOVuqIA

@justingrant
Copy link
Contributor

You can cast it to something that ExtractRouteParams can handle as a bit of a hack to unblock yourself.

Great hack! Works well and is reasonably self-describing. Thx.

@maraisr maraisr closed this as completed May 4, 2022
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

No branches or pull requests

3 participants