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

[TypeScript / Next.js Example] Link component not compatible with latest alpha #22450

Closed
piehouserat opened this issue Sep 2, 2020 · 18 comments
Labels
docs Improvements or additions to the documentation good first issue Great for first contributions. Enable to learn the contribution process. typescript

Comments

@piehouserat
Copy link

Using TS 4.x, Material UI latest alpha and next.js latest I am getting the following error in the Link.tsx component

if (naked) {
    return (
      <NextComposed
        className={className}
        ref={innerRef}
        href={href}
        {...other}
      />
    )
  }

the error comes when assigning the innerRef to the ref prop of the NextComposed component

Type '((instance: HTMLSpanElement | null) => void) | RefObject<HTMLSpanElement> | (((instance: HTMLAnchorElement | null) => void) & ((instance: any) => void)) | ... 4 more ... | undefined' is not assignable to type '((instance: HTMLAnchorElement | null) => void) | RefObject<HTMLAnchorElement> | null | undefined'.
  Type 'RefObject<HTMLSpanElement>' is not assignable to type '((instance: HTMLAnchorElement | null) => void) | RefObject<HTMLAnchorElement> | null | undefined'.
    Type 'RefObject<HTMLSpanElement>' is not assignable to type 'RefObject<HTMLAnchorElement>'.
      Type 'HTMLSpanElement' is missing the following properties from type 'HTMLAnchorElement': charset, coords, download, hreflang, and 21 more.

I've only been working with TS for a few weeks now so not able to figure out the correct typings required here.

Any help would be appreciated

@piehouserat piehouserat added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Sep 2, 2020
@oliviertassinari
Copy link
Member

@piehouserat Do you have reproduction we can look at? (For instance, we need the exact versions of the dependencies)

@deldrid1
Copy link

deldrid1 commented Sep 2, 2020

+1 based on the feedback to #22452.

Repro is here: https://codesandbox.io/s/infallible-forest-65g4d?file=/src/Link.tsx

Opening a terminal window and running npx tsc --noEmit will spit out the error.

sandbox@sse-sandbox-65g4d:/sandbox$ npx tsc --noEmit
src/Link.tsx:56:48 - error TS2322: Type '((instance: HTMLSpanElement | null) => void) | RefObject<HTMLSpanElement> | (((instance: HTMLAnchorElement | null) => void) & ((instance: any) => void)) | ... 4 more ... | undefined' is not assignable to type '((instance: HTMLAnchorElement | null) => void) | RefObject<HTMLAnchorElement> | null | undefined'.
  Type 'RefObject<HTMLSpanElement>' is not assignable to type '((instance: HTMLAnchorElement | null) => void) | RefObject<HTMLAnchorElement> | null | undefined'.
    Type 'RefObject<HTMLSpanElement>' is not assignable to type 'RefObject<HTMLAnchorElement>'.
      Type 'HTMLSpanElement' is missing the following properties from type 'HTMLAnchorElement': charset, coords, download, hreflang, and 21 more.

56     return <NextComposed className={className} ref={innerRef} href={href} {...other} />;
                                                  ~~~

  node_modules/@types/react/index.d.ts:140:9
    140         ref?: Ref<T>;
                ~~~
    The expected type comes from property 'ref' which is declared here on type 'IntrinsicAttributes & Pick<AnchorHTMLAttributes<HTMLAnchorElement>, "dir" | "slot" | "style" | "title" | ... 257 more ... | "referrerPolicy"> & LinkProps & RefAttributes<...>'


Found 1 error.

@deldrid1
Copy link

deldrid1 commented Sep 2, 2020

FYI - looks like this issue exists all the way back to "@material-ui/core": "5.0.0-alpha.1" on my local setup.

@oliviertassinari
Copy link
Member

oliviertassinari commented Sep 4, 2020

I can't observe the issue. Is it possible a release patched it?

@oliviertassinari oliviertassinari added status: waiting for author Issue with insufficient information and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Sep 4, 2020
@piehouserat
Copy link
Author

Guys I had some more time on this, bumped all packages to latest and the problem still exists. To prove that it was unrelated to my own code I cloned the latest nextjs-with-typescript example from the repo and checked out the Link component.

Out of the box, the ref prop is underlined in red on line 56 with the same error as before.

@piehouserat
Copy link
Author

@oliviertassinari can we please re-open this, the issue definitely exists and can be observed by simply looking at the example mentioned above

@oliviertassinari
Copy link
Member

@piehouserat
Copy link
Author

@oliviertassinari this is so weird, I've attached a screenshot of what I see

https://ibb.co/BTSwfRq

Is there any way this could be environment related? Only thing of note is that I am on the macOS Big Sur beta although I can't see how this would affect things

@matchatype
Copy link
Contributor

matchatype commented Nov 10, 2020

@oliviertassinari It does reproduce locally with v5 example as well, but the curl command refers to master branch. It should be:

curl https://codeload.github.com/mui-org/material-ui/tar.gz/next | tar -xz --strip=2  material-ui-next/examples/nextjs-with-typescript
cd nextjs-with-typescript

As a workaround you can silence it with: ref={innerRef as any}.

@oliviertassinari

This comment has been minimized.

@oliviertassinari oliviertassinari added docs Improvements or additions to the documentation good first issue Great for first contributions. Enable to learn the contribution process. and removed status: waiting for author Issue with insufficient information labels Nov 10, 2020
@matchatype
Copy link
Contributor

Hey @oliviertassinari you are right, it does fix it... feeling so dumb now 🤪
I'll be happy to push a PR for that 🙂

@vicasas
Copy link
Member

vicasas commented Jan 10, 2021

This should close? 🤔

@oliviertassinari
Copy link
Member

oliviertassinari commented Jan 10, 2021

This should close?

@vicasas I think that we can still remove the any with in the example:

diff --git a/docs/src/modules/components/Link.tsx b/docs/src/modules/components/Link.tsx
index 16c3ee49b9..91c9cb5d61 100644
--- a/docs/src/modules/components/Link.tsx
+++ b/docs/src/modules/components/Link.tsx
@@ -67,14 +67,14 @@ const Link = React.forwardRef<HTMLAnchorElement, LinkProps>(function Link(props,

   if (isExternal) {
     if (noLinkStyle) {
-      return <a className={className} href={href as string} ref={ref as any} {...other} />;
+      return <a className={className} href={href as string} {...other} ref={ref} />;
     }

     return <MuiLink className={className} href={href as string} ref={ref} {...other} />;
   }

   if (noLinkStyle) {
-    return <NextLinkComposed className={className} ref={ref as any} to={href} {...other} />;
+    return <NextLinkComposed className={className} to={href} {...other} ref={ref} />;
   }

   let linkAs = linkAsProp || (href as string);

But I don't know why, maybe @eps1lon has an explanation.

@eps1lon
Copy link
Member

eps1lon commented Jan 11, 2021

Could somebody give me a summary with reproduction what the problem here is? I'd usually wouldn't question the type-checker if I can remove an unsound cast such as as any 😉

@oliviertassinari
Copy link
Member

oliviertassinari commented Jan 11, 2021

@eps1lon The simplest way to reproduce is to remove the any:

diff --git a/docs/src/modules/components/Link.tsx b/docs/src/modules/components/Link.tsx
index 16c3ee49b9..51830fe7a7 100644
--- a/docs/src/modules/components/Link.tsx
+++ b/docs/src/modules/components/Link.tsx
@@ -67,14 +67,14 @@ const Link = React.forwardRef<HTMLAnchorElement, LinkProps>(function Link(props,

   if (isExternal) {
     if (noLinkStyle) {
-      return <a className={className} href={href as string} ref={ref as any} {...other} />;
+      return <a className={className} href={href as string} ref={ref} {...other} />;
     }

     return <MuiLink className={className} href={href as string} ref={ref} {...other} />;
   }

   if (noLinkStyle) {
-    return <NextLinkComposed className={className} ref={ref as any} to={href} {...other} />;
+    return <NextLinkComposed className={className} ref={ref} to={href} {...other} />;
   }

   let linkAs = linkAsProp || (href as string);

and run:

cd docs
yarn typescript

it will fail with:

     Type 'RefObject<HTMLSpanElement>' is not assignable to type 'RefObject<HTMLAnchorElement>'.

77     return <NextLinkComposed className={className} ref={ref} to={href} {...other} />;
                                                      ~~~

@eps1lon
Copy link
Member

eps1lon commented Jan 12, 2021

@oliviertassinari That's our internal implementation though. I don't think we need an issue for every internal instance where we cast to any.

Either way, this is a known issue with TypeScript: microsoft/TypeScript#30748

@oliviertassinari
Copy link
Member

oliviertassinari commented Jan 12, 2021

That's our internal implementation though. I don't think we need an issue for every internal instance where we cast to any.

@eps1lon True. However, we use the same one for the public Next.js TypeScript example. Same problem, same reproduction, same proposed fix:

https://github.com/mui-org/material-ui/blob/6b18675c7e6204b77f4c469e113f62ee8be39178/examples/nextjs-with-typescript/src/Link.tsx#L68

@oliviertassinari
Copy link
Member

It seems that we can solve the root issue in #24901.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Improvements or additions to the documentation good first issue Great for first contributions. Enable to learn the contribution process. typescript
Projects
None yet
Development

No branches or pull requests

6 participants