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

Update ref to use the implicit ref type when consuming LineProps #2765

Closed
wants to merge 1 commit into from

Conversation

Sigkar
Copy link

@Sigkar Sigkar commented Jan 14, 2022

Kind of a working idea. I made these changes and cross-tested them in a separate repository as well, TypeError went away and ref could be used properly. Can't really create a proper test for this in Karma because I only saw .js files?

Error:

No overload matches this call.
  Overload 1 of 2, '(props: Props | Readonly<Props>): Line', gave the following error.
    Type '{ string?: string | number | undefined; type: CurveType; className?: string | undefined; color?: string | undefined; height?: number | undefined; id?: string | undefined; ... 486 more ...; yAxis?: (Omit<...> & { ...; }) | undefined; }' is not assignable to type 'IntrinsicClassAttributes<Line>'.
      Types of property 'ref' are incompatible.
        Type 'LegacyRef<SVGPathElement> | undefined' is not assignable to type 'LegacyRef<Line> | undefined'.
          Type '(instance: SVGPathElement | null) => void' is not assignable to type 'LegacyRef<Line> | undefined'.
            Type '(instance: SVGPathElement | null) => void' is not assignable to type '(instance: Line | null) => void'.
              Types of parameters 'instance' and 'instance' are incompatible.
                Type 'Line | null' is not assignable to type 'SVGPathElement | null'.
                  Type 'Line' is missing the following properties from type 'SVGPathElement': addEventListener, removeEventListener, pathLength, getPointAtLength, and 263 more.
  Overload 2 of 2, '(props: Props, context: any): Line', gave the following error.
    Type '{ string?: string | number | undefined; type: CurveType; className?: string | undefined; color?: string | undefined; height?: number | undefined; id?: string | undefined; ... 486 more ...; yAxis?: (Omit<...> & { ...; }) | undefined; }' is not assignable to type 'IntrinsicClassAttributes<Line>'.
      Types of property 'ref' are incompatible.
        Type 'LegacyRef<SVGPathElement> | undefined' is not assignable to type 'LegacyRef<Line> | undefined'.

Reproduction

import { LineProps } from 'recharts';

interface SomeLibraryComponent {
  line: LineProps
}

const SomeComponent: SomeLibraryComponent = ({line}) => {
  return (
    <Line // Error location of above
      {...line}
    />
  )
}

Find it kind of strange for the ref object to be the actual class.. would an any work better in this case?

@Sigkar
Copy link
Author

Sigkar commented Jan 14, 2022

Looks like this affects more than just LineProps. This needs to be updated to hit the root cause

#2709 looks like its running into the same issue.

I also noticed it is affecting the cartesian grid. Maybe can create an arged type for the ref to apply back when exporting the interfaces?

This isn't exactly right, so should I close / change to a draft, or are we close here?

@ckifer
Copy link
Member

ckifer commented Dec 14, 2022

@Sigkar can you confirm this is still an issue? Thanks :)

@ckifer ckifer added the typescript PRs or Issues surrounding Types or TS refactoring label Dec 14, 2022
@ckifer ckifer added the pending response Pending response from the issue requester label Dec 23, 2022
@ckifer
Copy link
Member

ckifer commented Oct 16, 2023

Going to close this as stale, we'll come back to these when we fix refs for the next major release

@ckifer ckifer closed this Oct 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pending response Pending response from the issue requester typescript PRs or Issues surrounding Types or TS refactoring
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants