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

@ts-ignore is hard to use with calls to overloads #33795

Closed
turadg opened this issue Oct 3, 2019 · 9 comments
Closed

@ts-ignore is hard to use with calls to overloads #33795

turadg opened this issue Oct 3, 2019 · 9 comments
Labels
Bug A bug in TypeScript Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented.
Milestone

Comments

@turadg
Copy link

turadg commented Oct 3, 2019

TypeScript Version: 3.7.0-beta, 3.6.3
(Works in 3.5.1)

Search Terms: ts-ignore, JSX, React, props

Code
(See links on the version numbers above for TS Playground)

import React from 'react';

enum SomeEnum {
    FIRST,
    SECOND,
};

class SomeComp extends React.Component<{ foo: SomeEnum, bar: number }> {
}

function OtherComp() {
    return (
        <SomeComp
            // @ts-ignore
            foo="FIRST"
            // @ts-ignore
            bar="aString"
        />
    )
}

Expected behavior: no errors

Actual behavior: Error on foo and on bar. Deleting either prop makes for no errors.

Playground Link:

Related Issues: I first thought it was #33256 but @sandersn advised to file a separate issue

@threehams
Copy link

This is only a problem with class components, interestingly. Function components are fine.
3.7.0-beta

@sandersn sandersn added Bug A bug in TypeScript Needs Investigation This issue needs a team member to investigate its status. labels Oct 4, 2019
@sandersn
Copy link
Member

sandersn commented Oct 4, 2019

Briefly, the reason that this fails is that reporting of errors on overloads doesn't want to stack tonnes of errors on individual parameters. Instead it puts a span on the entire call <SomeComp .... You can work around this problem by putting the ts-ignore above <SomeComp ....

Because overload error reporting can report one error per overload per parameter, the non-overload behaviour of sticking errors on individual parameters would put multiple errors on each parameter. Instead it gives a multiline report-style error, but its span is the entire call.

React.FC doesn't have overloads, so it will only ever have one error per parameter, so it keeps error on each parameter. (React.Component's constructor has two overloads, one of which is deprecated =(

        constructor(props: Readonly<P>);
        constructor(props: P, context?: any);

To fix this, we'd need to come up with a better way to correlate and display per-parameter errors with overloads. I'm not sure what that would look like.

Thanks all for the playground links. React repros are 💯 easier now that the playground has react.

@sandersn sandersn added Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented. and removed Needs Investigation This issue needs a team member to investigate its status. labels Oct 4, 2019
@sandersn sandersn added this to the Backlog milestone Oct 4, 2019
@sandersn sandersn changed the title @ts-ignore only works on one prop in a JSX prop list @ts-ignore is hard to use with calls to overloads Oct 4, 2019
@turadg
Copy link
Author

turadg commented Oct 4, 2019

You can work around this problem by putting the ts-ignore above <SomeComp ....

That does work in the snippets above. How would you do it for this?

        <div>
            <SomeComp
                foo="FIRST"
                bar="aString"
                />
        </div>

This stops the errors but puts a string into the JSX expression:

        <div>
            // @ts-ignore
            <SomeComp
                foo="FIRST"
                bar="aString"
                />
        </div>

Flow supports {/* */} suppressions, but it seems TypeScript does not.

        <div>
            {/* @ts-ignore */}
            <SomeComp
                foo="FIRST"
                bar="aString"
                />
        </div>

@sandersn
Copy link
Member

sandersn commented Oct 4, 2019

@turadg How would {/* */} solve the problem of inserting a string? The string insertion happens because of the extra line inside the <div>, right?

@lamontadams
Copy link

lamontadams commented Oct 4, 2019

@sandersn I just want to make sure you know this works as-expected in 3.5 and it feels like a regression in 3.6 - almost like something changed so that ts-ignore is applied at the statement level now instead of the line level now.

Putting ts-ignore on the whole component works around the issue for me, but it has the undesirable side-effect of removing type-checking on the whole component (because ts-ignore applies at the component level rather than at the single prop I want ignored). That's really undesirable for components that have lots of props.

@turadg
Copy link
Author

turadg commented Oct 4, 2019

How would {/* */} solve the problem of inserting a string? The string insertion happens because of the extra line inside the <div>, right?

@sandersn In JSX within <> expressions, {} escapes back into JS.

You can see the ts-ignore string appear in // style and be absent in {/* */} style

this works as-expected in 3.5 and it feels like a regression in 3.6

+1, this is a regression in 3.6 and persisting in 3.7

@threehams
Copy link

threehams commented Oct 4, 2019

It's nothing specific to JSX, but rather a change to errors on overloads in general. It's affecting React because of the use of overloading in @types/react (interface Component + class Component) overload in DefinitelyTyped here. Here's an example of an overloaded function and the difference between 3.5.1 and 3.7.0:
3.5.1
3.7.0-beta

Might be a good question for the maintainers of the React types; I don't really understand why the interface overload is there.

@vamshi9666
Copy link

This worked for me ,


           {
              // @ts-ignore
              <p>{chef && user[dataKey]}</p>
            }

@turadg
Copy link
Author

turadg commented Apr 13, 2021

Agree that in 4.2.2 the problem in #31147 is gone. (What @vamshi9666 said now works.)

Closing this issue since that is a fine work-around for the problem reported.

@turadg turadg closed this as completed Apr 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented.
Projects
None yet
Development

No branches or pull requests

5 participants