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: withRouter fails with ComponentType starting in 3.6.2 #38271

Closed
4 tasks done
OliverJAsh opened this issue Sep 10, 2019 · 10 comments · Fixed by #38326
Closed
4 tasks done

react-router: withRouter fails with ComponentType starting in 3.6.2 #38271

OliverJAsh opened this issue Sep 10, 2019 · 10 comments · Fixed by #38326
Assignees

Comments

@OliverJAsh
Copy link
Contributor

This worked in 3.5.2, but fails in 3.6.2.

strict mode is disabled.

Is this a bug in the typings or TypeScript?

I added failing tests for this here: f036856.

import * as React from 'react';

// Copied from https://github.com/DefinitelyTyped/DefinitelyTyped/blob/5db94ca7fd3570dc23588b404d7bb669a7886a8a/types/react-router/index.d.ts#L146
declare function withRouter<P, C extends React.ComponentType<P>>(
    component: C & React.ComponentType<P>,
): unknown;

declare const Component: React.ComponentType<{}>;
/*
Argument of type 'ComponentType<{}>' is not assignable to parameter of type 'ComponentClass<{}, any> | (ComponentClass<{}, any> & FunctionComponent<{}>)'.
  Type 'FunctionComponent<{}>' is not assignable to type 'ComponentClass<{}, any> | (ComponentClass<{}, any> & FunctionComponent<{}>)'.
    Type 'FunctionComponent<{}>' is not assignable to type 'ComponentClass<{}, any> & FunctionComponent<{}>'.
      Type 'FunctionComponent<{}>' is not assignable to type 'ComponentClass<{}, any>'.
        Type 'FunctionComponent<{}>' provides no match for the signature 'new (props: {}, context?: any): Component<{}, any, any>'.
*/
withRouter(Component);

If you know how to fix the issue, make a pull request instead.

If you do not mention the authors the issue will be ignored.

@eps1lon
Copy link
Collaborator

eps1lon commented Sep 10, 2019

I won't have time to dig into this. I think we should just remove any usage of ComponentType and use plain functions with explicit props with regard to ref and children. ComponentType was always a bit problematic because of the union. If it didn't create errors it produced very large unions with other hocs. Library consumers shouldn't touch statics anyway so loosing those shouldn't be an issue.

@basslagter
Copy link

Got the same issue. Already reported on the react-router repo (remix-run/react-router#6906) but got redirected to here.

orta pushed a commit that referenced this issue Sep 17, 2019
…arting in 3.6.2) (#38326)

* Make withRouter compatible with ComponentType

Fixes #38271.

* Add new baselines

Co-authored-by: Oliver Joseph Ash <oliverjash@gmail.com>
@bkoltai
Copy link

bkoltai commented Sep 17, 2019

I'm still seeing issues even after this fix. Error message is something like

 Argument of type '({ myProp, history }: RouteComponentProps<{}, StaticContext, any> & { myProp: boolean; }) => Element' is not assignable to parameter of type 'ComponentType<RouteComponentProps<{}, StaticContext, any>>'.
  Type '({ myProp, history }: RouteComponentProps<{}, StaticContext, any> & { myProp: boolean; }) => Element' is not assignable to type 'FunctionComponent<RouteComponentProps<{}, StaticContext, any>>'.
    Types of parameters '__0' and 'props' are incompatible.
      Type 'PropsWithChildren<RouteComponentProps<{}, StaticContext, any>>' is not assignable to type 'RouteComponentProps<{}, StaticContext, any> & { myProp: boolean; }'.
        Property 'myProp' is missing in type 'RouteComponentProps<{}, StaticContext, any> & { children?: ReactNode; }' but required in type '{ myProp: boolean; }'.

with the component defined as

import { RouteComponentProps, withRouter } from "react-router-dom";

interface Props extends RouteComponentProps {
  myProp: boolean;
}

const Component = ({ myProp, match }: Props) => null

export default withRouter(Component);

I'm using

typescript: 3.6.3
@types/react-router: 5.0.4

@artola
Copy link
Contributor

artola commented Sep 17, 2019

@karol-majewski I am on TS 3.6.3 and the latest version @types/react-router: 5.0.4 is breaking, while returning to former is OK.

export function withRouter<P extends RouteComponentProps<any>, C extends React.ComponentType<P>>(
  component: C & React.ComponentType<P>,
): React.ComponentClass<Omit<P, keyof RouteComponentProps<any>> & WithRouterProps<C>> & WithRouterStatics<C>;

@eps1lon
Copy link
Collaborator

eps1lon commented Sep 18, 2019

I encourage everyone to open new issues. If we fix your particular issue you have no way of being notified if you only comment in a closed thread. The typescript playground now allows configuring the ts version + config and is able to download type definitions. Including a link in a new issue helps maintainers a lot. Thank you 🙏

It seems like this only throws with strictFunctionTypes and I have a solution in mind. We'll see if it works out.

@OliverJAsh
Copy link
Contributor Author

OliverJAsh commented Sep 18, 2019

@sandersn Can we re-open this since the fix was reverted?

@OliverJAsh
Copy link
Contributor Author

IIRC, @eps1lon discovered that this is due to a TypeScript bug: microsoft/TypeScript#33490

Fix will come in: microsoft/TypeScript#34607.

@ahejlsberg ahejlsberg self-assigned this Oct 21, 2019
jfsiii pushed a commit to jfsiii/kibana that referenced this issue Oct 30, 2019
Issues are described in DefinitelyTyped/DefinitelyTyped#38271

Root cause is stated to be an issue with TS microsoft/TypeScript#34607 which has since been fixed in master.

Commiting now to prevent issues, but would also like to revert this and try again when TS fix ships (TS 3.7?)
@karol-majewski
Copy link
Contributor

Works fine with the nightly version. Playground Link

jfsiii pushed a commit to jfsiii/kibana that referenced this issue Oct 31, 2019
IIRC, I added this while tracking down the issues described in DefinitelyTyped/DefinitelyTyped#38271

It could have been another error, but we're not _supposed_ to have to add this dependency. I added it trying to suppress errors. Seeing if I can remove it and have the same results we currently have.
@cristiantorresf19191919

typescript is a waste of time, I have spent hours trying to solve this stupid error and nothins, so if typescript is made to make life easier it is definately not doing the job, full of errors and bugs

@ahejlsberg
Copy link
Collaborator

This issue was fixed in TS 3.7. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
9 participants