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 option --exactOptionalPropertyTypes is not supported #28745

Open
kimljac opened this issue Oct 1, 2021 · 5 comments
Open

TypeScript option --exactOptionalPropertyTypes is not supported #28745

kimljac opened this issue Oct 1, 2021 · 5 comments
Assignees
Labels
new feature New feature or request typescript

Comments

@kimljac
Copy link

kimljac commented Oct 1, 2021

Current Behavior 😯

Trying to compile (TypeScript) any example from the section about the Autocomplete component fails with type errors like:

billede

It is obviously not absolutely necessary to be able to use that option, but it would be nice if it worked. Otherwise it should probably be well documented, that you can not use --exactOptionalPropertyTypes with MUI.

Your Environment 🌎

@mui/envinfo

System:
OS: Windows 10 10.0.19043
Binaries:
Node: 16.5.0 - C:\Program Files\nodejs\node.EXE
Yarn: Not Found
npm: 7.19.1 - C:\Program Files\nodejs\npm.CMD
Browsers:
Chrome: Not Found
Edge: Spartan (44.19041.1023.0), Chromium (94.0.992.31)
npmPackages:
@emotion/react: ^11.4.1 => 11.4.1
@emotion/styled: ^11.3.0 => 11.3.0
@mui/core: 5.0.0-alpha.49
@mui/icons-material: ^5.0.1 => 5.0.1
@mui/material: ^5.0.2 => 5.0.2
@mui/private-theming: 5.0.1
@mui/styled-engine: 5.0.1
@mui/styles: ^5.0.1 => 5.0.1
@mui/system: 5.0.2
@mui/types: 7.0.0
@mui/utils: 5.0.1
@types/react: ^17.0.26 => 17.0.26
react: ^17.0.2 => 17.0.2
react-dom: ^17.0.2 => 17.0.2
typescript: ^4.4.3 => 4.4.3

tsconfig.json

{
"compilerOptions": {
"target": "es5",
"lib": ["dom", "dom.iterable", "esnext"],
"allowJs": true,
"skipLibCheck": true,
"esModuleInterop": true,
"allowSyntheticDefaultImports": true,
"strict": true,
"forceConsistentCasingInFileNames": true,
"noFallthroughCasesInSwitch": true,
"module": "esnext",
"moduleResolution": "node",
"resolveJsonModule": true,
"isolatedModules": true,
"noEmit": true,
"jsx": "react-jsx",
"exactOptionalPropertyTypes": true,
"noImplicitOverride": true,
"useUnknownInCatchVariables": true
},
"include": ["src"]
}

@kimljac kimljac added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Oct 1, 2021
@eps1lon eps1lon added new feature New feature or request typescript and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Oct 1, 2021
@nmggithub
Copy link

The problem here is two-fold:

  1. The params object does not define variant, which is required.
  2. The package defines the className?: string while React defined it as className?: string | undefined

The second one is where the main issue is. The first is trivially solved by adding a variant param manually.

@akomm
Copy link
Contributor

akomm commented Jul 12, 2022

I think the issue with exactOptionalPropertyTypes isn't that easy here.

See this TextField variant property

The default variant "outlined" is encoded as optional property. First it might appears rational, but its not quite. If you want to create a custom component that passes through some of the TextFieldProps and evtl has some other defaults (inkl. for variant), you have to do weird "remapping" to undefined even without exactOptionalPropertyTypes enabled. With exactOptionalPropertyTypes enabled, you can't spread props, you need to filter it out because you can't pass explicitly "undefined".

If a library wants to maintain its "functionality", but be compatible with exactOptionalPropertyTypes, it should be explicit:

// instead of
type X1 = {
  variant?: "B" // default = "A"
}
// it should be either
type X2 = {
  variant?: "B" | undefined
}
// or
type X3 = {
  variant?: "A" | "B"
}

Option X3 has though the downside of having 3 values for actually 2 variants. The X2 seems more clean.

@aaronadamsCA
Copy link
Contributor

A different failing use case is PaginationItem being incompatible with PaginationRenderItemParams:

import { Pagination, PaginationItem } from "@mui/material";
import * as React from "react";

export const Test: React.FC = () => (
  <Pagination renderItem={(params) => <PaginationItem {...params} />} />
);
No overload matches this call.
  Overload 1 of 2, '(props: { component: ElementType<any>; } & { classes?: Partial<PaginationItemClasses>; color?: "primary" | "secondary" | "standard"; components?: { ...; }; ... 7 more ...; variant?: "text" | "outlined"; } & CommonProps & Omit<...>): Element', gave the following error.
    Property 'component' is missing in type '{ color: "primary" | "secondary" | "standard" | undefined; shape: "circular" | "rounded" | undefined; size: "small" | "medium" | "large" | undefined; variant: "text" | "outlined" | undefined; ... 4 more ...; disabled: boolean; }' but required in type '{ component: ElementType<any>; }'.
  Overload 2 of 2, '(props: DefaultComponentProps<PaginationItemTypeMap<{}, "div">>): Element', gave the following error.
    Type '{ color: "primary" | "secondary" | "standard" | undefined; shape: "circular" | "rounded" | undefined; size: "small" | "medium" | "large" | undefined; variant: "text" | "outlined" | undefined; ... 4 more ...; disabled: boolean; }' is not assignable to type '{ classes?: Partial<PaginationItemClasses>; color?: "primary" | "secondary" | "standard"; components?: { first?: ElementType<any>; last?: ElementType<...>; next?: ElementType<...>; previous?: ElementType<...>; }; ... 7 more ...; variant?: "text" | "outlined"; }' with 'exactOptionalPropertyTypes: true'. Consider adding 'undefined' to the types of the target's properties.
      Types of property 'color' are incompatible.
        Type '"primary" | "secondary" | "standard" | undefined' is not assignable to type '"primary" | "secondary" | "standard"'.
          Type 'undefined' is not assignable to type '"primary" | "secondary" | "standard"'.

IMO this case is quite different (and more easily solved), but I was asked to move the description here from #33700, so I did.

@alexwork1611
Copy link

Any progress on this?

@ZeeshanTamboli ZeeshanTamboli marked this as a duplicate of #45106 Jan 25, 2025
@ZeeshanTamboli
Copy link
Member

This issue is open for anyone with ideas on how to support this option.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new feature New feature or request typescript
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants