TypeScript option --exactOptionalPropertyTypes is not supported

[kimljac]

Current Behavior 😯

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

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"] }

[nmggithub]

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]

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]

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]

Any progress on this?

[ZeeshanTamboli]

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

[alexwork1611]

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

Can you please look into this more? More and more people are using exactOptionalPropertyTypes.

[nspaeth]

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

@ZeeshanTamboli I'm unsure what ideas are needed. Isn't the solution to put | undefined after any property that is optional?

Is there some other challenge?

[akomm]

@nspaeth yes. In order to support both you need to add | undefined explicitly, where its current behavior anyway.

Currently all libraries, including mui, block one from using the feature in own code, because their code "break" then type-wise.

Its backwards-compatible doing so in places where it was allowed as of now anyway. Its just the work to add it all over the place and make sure not adding it in wrong place.

[siriwatknp]

Other library are experiencing this issue too.

Adding | undefined to all of the values we have sounds really wrong. How do we ensure that we don't mess it up. At least TypeScript should have a way to handle this thing.

Also, I don't think it's a new feature. I mark this as external, the fix should not come from us.

[flying-sheep]

How do we ensure that we don't mess it up

The difference between const obj = { field: undefined } and const obj = {} is that field in obj evaluates to true in one case and false in the other. Equivalently for Object.keys(obj).

So there is a difference, and it’s totally feasible that some APIs you have support both and some only one.

Therefore the only answer I have is: activate the option and write unit tests with both variants for everything and make sure they typecheck (i.e. <Tag {...{}} /> and <Tag {...{ optionalAttr: undefined }} />)

Adding | undefined to all of the values we have sounds really wrong

Any field that can be undefined should have | undefined in its type, yes.

1. interface I {
       field?: string
   }
   

   means that { field: "hi" } and {} fullfill interface I.

2. interface I {
       field: string | undefined
   }
   

   means that { field: "hi" } and { field: undefined } fullfill interface I.

3. interface I {
       field?: string | undefined
   }
   

   means that { field: "hi" }, { field: undefined } and {} fullfill interface I.

[akomm]

Writing | undefined is contrary to initial feeling one can have about this, me included, not wrong. Its factually correct. It was implied, but was actually a mistake to do so. It does make a difference at runtime, whether a property actually exist or not, even if the value might be undefined.

[zeorin]

I use the following type augmentation:

type UndefinedOnPartial<T> = {
  [K in keyof T]: T[K] | (undefined extends T[K] ? undefined : never);
};

declare module '@mui/material/OverridableComponent' {
  export interface OverridableComponent<TypeMap extends OverridableTypeMap> {
    <RootComponent extends React.ElementType>(
      props: {
        component: RootComponent;
      } & UndefinedOnPartial<OverrideProps<TypeMap, RootComponent>>
    ): React.JSX.Element | null;
    (
      props: UndefinedOnPartial<DefaultComponentProps<TypeMap>>
    ): React.JSX.Element | null;
  }
}

OverridableComponent is used to define the type of most of MUI's React Components.

[akomm]

Other library are experiencing this issue too.

Adding | undefined to all of the values we have sounds really wrong. How do we ensure that we don't mess it up. At least TypeScript should have a way to handle this thing.

Also, I don't think it's a new feature. I mark this as external, the fix should not come from us.

Adding | undefined where its accepted as a value is not wrong, its correct. Its the behavior with any other type then. Currently undefined and optional properties have own behavior that was based on an assumption that it does not matter, later realized that it does matter, but kept for backwards compatibility.

Adding it anywhere were currently optional property is used is guaranteed BC, because its what it is now implicitly.

The issue with this feature is, if one library applies it, as long as any one library you use does not you basically can't use the feature in your entire project. Adopting this change doesn't seem to be a big movement right now sadly, so I can understand when the authors think this change adds "clutter" that probably won't help the user at the end anyway, because some library does not implement it and so the user will have to turn it off anyway.

[flying-sheep]

Yeah, it’s not that complicated. Imagine some API takes a parameter obj, where it’s valid that obj[key] === undefined. Now if none of material-ui’s APIs where this applies acts differently depending on if key in obj or not, then adding | undefined everywhere makes sense.

If any API does act differently, it makes a lot of sense to document that.

[unrevised6419]

[alexwork1611]

Kind reminder...