mui / material-ui

Material UI: Comprehensive React component library that implements Google's Material Design. Free forever.
https://mui.com/material-ui/
MIT License
94.03k stars 32.3k forks source link

[Button][material-ui] Button href TypeScript error #39287

Open kmoyse-apizee opened 1 year ago

kmoyse-apizee commented 1 year ago

Duplicates

Latest version

Steps to reproduce 🕹

Hi, I found issue that really looks like mine : https://github.com/mui/material-ui/issues/22699 Steps:

  1. Developing in typescript
  2. Use <Button with href
  3. add target="_blank" property to it
  4. I get error ; TS2769: No overload matches this call. Overload 1 of 3, '(props: { href: string; } & ButtonOwnProps & Omit<ButtonBaseOwnProps, "classes"> & CommonProps & Omit<Omit<DetailedHTMLProps<AnchorHTMLAttributes, HTMLAnchorElement>, "ref"> & { ...; }, "color" | ... 24 more ... | "startIcon">): Element', gave the following error. Type 'string | undefined' is not assignable to type 'string'. Type 'undefined' is not assignable to type 'string'.

Trying to set strictNullChecks: true in tsconfig.json does not solve the problem

Current behavior 😯

typescript transpile error

Expected behavior 🤔

No response

Context 🔦

No response

Your environment 🌎

System: OS: Linux 5.19 Ubuntu 22.04.3 LTS 22.04.3 LTS (Jammy Jellyfish) Binaries: Node: 16.18.0 - /usr/local/bin/node Yarn: 1.22.19 - ~/.yarn/bin/yarn npm: 9.6.4 - /usr/local/bin/npm Browsers: Chrome: 117.0.5938.132 npmPackages: @emotion/react: ^11.11.1 => 11.11.1 @emotion/styled: ^11.11.0 => 11.11.0 @mui/base: 5.0.0-beta.17 @mui/core-downloads-tracker: 5.14.11 @mui/icons-material: ^5.14.11 => 5.14.11 @mui/material: ^5.14.11 => 5.14.11 @mui/private-theming: 5.14.11 @mui/styled-engine: 5.14.11 @mui/system: 5.14.11 @mui/types: 7.2.4 @mui/utils: 5.14.11 @types/react: ^18.2.12 => 18.2.12 react: ^18.2.0 => 18.2.0 react-dom: ^18.2.0 => 18.2.0 typescript: ^4.9.5 => 4.9.5

aryan0078 commented 1 year ago

assign this task to me

mj12albert commented 1 year ago

@kmoyse-apizee I can't reproduce this: https://codesandbox.io/s/https-github-com-mui-material-ui-issues-39287-whkjn3

Could you provide a repro? You can fork my sandbox or this template: https://mui.com/r/issue-template-latest

legendary0911 commented 1 year ago

can you assign it to me ?

mj12albert commented 1 year ago

@legendary0911 Are you able to reproduce the issue?

kmoyse-apizee commented 1 year ago

Hi,

Thanks for your answer and the reproduction.

I forked it to reproduce my problem : https://codesandbox.io/s/https-github-com-mui-material-ui-issues-39287-forked-hc4nwk If I copy paste the Demo.tsx code into a component under my vs-code I reproduce the issue. It does not on codesandbox...

It seems to be more related to the 'href' property rather than the 'target' itself.

import React, { useState } from "react";
import Button from "@mui/material/Button";

export default function TextButtons() {
  const [href, setHref] = useState<string>();
  return (
    <Button href={href} target="_blank" rel="noopener">
      Link
    </Button>
  );
}

The error : No overload matches this call. Overload 1 of 3, '(props: { href: string; } & ButtonOwnProps & Omit<ButtonBaseOwnProps, "classes"> & CommonProps & Omit<Omit<DetailedHTMLProps<AnchorHTMLAttributes, HTMLAnchorElement>, "ref"> & { ...; }, "href" | ... 24 more ... | "variant">): Element', gave the following error. Type 'string | undefined' is not assignable to type 'string'. Type 'undefined' is not assignable to type 'string'.

Seems happens because my href constant coulld be undefined...

I don't know why the issue shows on vs-code and not on codesandbox ? my tsconfig has the same configuration...

mj12albert commented 1 year ago

@kmoyse-apizee The initial state in your snippet is undefined, you can fix it by initializing with an empty string:

-const [href, setHref] = useState<string>();
+const [href, setHref] = useState<string>('');
kmoyse-apizee commented 1 year ago

Yes but in my application logic the variable in the href can actually be undefined. To fix the problem I had to do :

<Button href={href ?? "#"} target="_blank" rel="noopener" disabled={!href}>Link</Button>

Which is fine because if not defined the button becomes disabled.

I still don't get why same code works on codesandbox and cannot be compiled by typescript in my environment.

imevanc commented 1 year ago

Yes but in my application logic the variable in the href can actually be undefined. To fix the problem I had to do :

<Button href={href ?? "#"} target="_blank" rel="noopener" disabled={!href}>Link</Button>

Which is fine because if not defined the buttons becomes disabled.

I still don't get why same code works on condesandbox and cannot be compiled by typescript in my environment.

@kmoyse-apizee - I do not think that this code will be maintainable in the long time run. I'd advise you to use to create a conditional wrapper for this component.

I'm struggling to reproduce your issue locally and the sandbox is not working for me for some reason.

DanielJSottile commented 1 year ago

While not a solution, and I think this should still be addressed, I have a workaround for you @kmoyse-apizee. If you conditionally render the property, it should avoid typescript issues. Below is an example from my own project I'm working on, where I'm creating my own Button component and rendering out MUI's button below:

<MUIButton
    {...(href ? { component: 'a', href } : {})}
    sx={{ ...customSx }}
    variant={variant}
    startIcon={ItemIcon}
    onClick={onClick}
    disabled={disabled}
    type={type}
    fullWidth={fullWidth}
    {...(target ? { target } : {})}
  >

As you can see, I'm also doing the same thing for the href, with some additional renderings for the component. I ran into the same issue you did and I was just looking for a way to avoid TS errors.