[react-router] Improve withRouter return type#21329
Conversation
|
types/react-router/index.d.ts to authors (@sergey-buturlakin @mrk21 @vasek17 @ngbrown @awendland @KostyaEsmukov @johnnyreilly @LKay @DovydasNavickas @tkrotoff @huy-nguyen @Grmiade @DaIgeb @egorshulga). Could you review this PR? |
|
Looking forward on this! |
RyanCavanaugh
added
Popular package
|
@Grmiade Please fix the failures indicated in the Travis CI log. |
|
@Grmiade Please fix the failures indicated in the Travis CI log. |
|
Anxiously awaiting this change. Thanks for pushing this along! |
Grmiade
force-pushed
the
feature/grmiade/react-router-dom
branch
from
812e1ab to
83ba2b8
Compare
Grmiade
force-pushed
the
feature/grmiade/react-router-dom
branch
from
83ba2b8 to
fb95a9b
Compare
Grmiade
force-pushed
the
feature/grmiade/react-router-dom
branch
from
a43aab4 to
2689bb2
Compare
|
I had to remove |
|
Any ETA for getting this merged ? |
|
@johnnyreilly Newbie question here. How much time it take to get the this change in |
|
It's an automated process @bilby91 - should appear in the next 24 hours or so. ( Occasionally it fouls up but AFAIK that's rare) |
|
Good! Will wait then! Thanks for the info BTW. |
|
|
||
| export function matchPath<P>(pathname: string, props: RouteProps): match<P> | null; | ||
| export function withRouter<P>(component: React.ComponentType<RouteComponentProps<any> & P>): React.ComponentClass<P>; | ||
| export function withRouter<P extends RouteComponentProps<any>>(component: React.ComponentType<P>): React.ComponentClass<Omit<P, keyof RouteComponentProps<any>>>; |
There was a problem hiding this comment.
I have ran into an issue with this change and would be interested to get your thoughts @Grmiade: #24070
Separately, do you know why the old type definition didn't work? It seems like it should. If you provide the generic explicitly, it works as expected. Perhaps it's a bug with TypeScript's generic inference? I filed a TypeScript bug here: microsoft/TypeScript#21964
There was a problem hiding this comment.
We wanted to fix this issue #17181
And make the function withRouter smarter. No need to provide explicit type. It can be inferred.
The typings becomes more safe. But indeed, you found a bug with the Omit definition.
There was a problem hiding this comment.
We were also hitting #17181. Thanks for fixing it!
I'm still curious why the old definition didn't work. I would have expected the return type to omit the router props, because the generic excludes the router props. Is that what you would expected? However, it seems TypeScript is not inferring the generic to exclude the router props. I filed a TS bug for that here—please chime in! microsoft/TypeScript#21964.
There was a problem hiding this comment.
interface TOwnProps extends RouteComponentProps<any> {
username: string;
}
const ComponentFunction = (props: TOwnProps) => (
<h2>Welcome {props.username} on {props.location.pathname}</h2>
);
const WithRouterComponentFunction = withRouter(ComponentFunction);
// withRouter<TOwnProps>(component: React.ComponentType<RouteComponentProps<any> & TOwnProps>): React.ComponentClass<TOwnProps>#17181 issue appears when TOwnProps already includes RouteComponentProps because the returned component includes RouteComponentProps too.
Indeed Typescript is not inferring the generic to exclude the router props. Bug or miss-feature, I don't know :/
|
The problem with DefinitelyTyped definitions and PRs like this is missing examples. I actually tried to use |
|
@1999 You can find so many examples in the withRouter test file. You will see the generic type is not needed, it should be smart and inferred in so many case ;) |
9 participants
Related to #17181
This PR allow to improve the withRouter type return by omitting RouteComponentProps.
No need to specify props type of the returned component. It's inferenced now =)
Decorator type has same problem :/
But I didn't find a solution for the moment. Any idea?