Material-ui: TypeScript errors upgrading MUI from 4.9.5 to 4.9.9

[NMinhNguyen]

• [x] The issue is present in the latest release.
• [x] I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

I have been able to trace the following errors to some changes made recently to use TS features such as Omit which aren't available in 3.2.4 (as far as I understand, that's the minimum TS version that MUI supports) or trailing commas:

• https://github.com/mui-org/material-ui/pull/20264 (Omit)
• https://github.com/mui-org/material-ui/pull/20211 (the trailing comma issue)

Click to expand

node_modules/@material-ui/core/Backdrop/Backdrop.d.ts:8:52 - error TS2304: Cannot find name 'Omit'.

8     React.HTMLAttributes<HTMLDivElement> & Partial<Omit<FadeProps, 'children'>>,
                                                     ~~~~

node_modules/@material-ui/core/Box/Box.d.ts:29:22 - error TS1009: Trailing comma not allowed.

29     typeof typography,
                        ~

node_modules/@material-ui/core/Fade/Fade.d.ts:4:36 - error TS2304: Cannot find name 'Omit'.

4 export interface FadeProps extends Omit<TransitionProps, 'children'> {
                                     ~~~~

node_modules/@material-ui/core/TextareaAutosize/TextareaAutosize.d.ts:4:11 - error TS2304: Cannot find name 'Omit'.

4   extends Omit<React.TextareaHTMLAttributes<HTMLTextAreaElement>, 'children' | 'rows'> {
            ~~~~

node_modules/@material-ui/core/styles/shadows.d.ts:26:9 - error TS1009: Trailing comma not allowed.

26   string,
           ~

Expected Behavior 🤔

Steps to Reproduce 🕹

Steps:

1. Install @material-ui/[email protected]
2. Run your project through tsc using [email protected]
3. Observe the errors above

Context 🔦

We set a minimum supported version of TypeScript to 3.2.4 in our design system to match Material-UI but seems like the latest MUI version uses newer TypeScript features. This isn't usually a big issue to React projects, but we also support Angular teams via Custom Elements and usually bumping TypeScript requires them to bump their Angular versions too (usually across major version boundaries) which can be a bit painful. For example, Angular 7.2 supports TS 3.2.4, Angular 8.0.0 adds support for 3.4 dropping support for older versions, Angular 9.0.0 requires 3.6/3.7

Your Environment 🌎

| Tech | Version |
| ----------- | ------- |
| Material-UI | v4.9.9 |
| TypeScript | v3.2.4 |

[eps1lon]

Yeah this is problematic. Prettier's trailingCommas: all also applies to TypeScript and I think some trailing commas are only valid in later TS versions.

Maybe trailingCommas: es5 for .d.ts files works.

[NMinhNguyen]

Maybe trailingCommas: es5 for .d.ts files works.

I'm happy to work on a PR for this one. What shall we do about Omit? I don't suppose there's a way to lint against such types? :/

[eps1lon]

This was by accident. We have an internal Omit that works without the built-in one.

[NMinhNguyen]

Oh nice! But still might be good to be able to lint against using the built-in one. I'll raise a PR shortly for trailing commas and using the internal Omit

[eps1lon]

Yeah this is what we wanted to avoid but I'd argue that this is more of an issue with a PR that was too big not the tooling.

I would rather move forward here though (upcoming @ts-expect-error and tslint -> eslint). Usage numbers for earlier typescript version is pretty low so I don't think it's justified to build tooling for 3.2. But this is for me personally. If wants to build it I'm all ears.

[oliviertassinari]

@eps1lon I wanted to ask about this a couple of days ago, this is an opportunity. What do you think about this diff?

diff --git a/docs/src/pages/getting-started/supported-platforms/supported-platforms.md b/docs/src/pages/getting-started/supported-platforms/supported-platforms.md
index befc4ca39..e49aeba94 100644
--- a/docs/src/pages/getting-started/supported-platforms/supported-platforms.md
+++ b/docs/src/pages/getting-started/supported-platforms/supported-platforms.md
@@ -36,3 +36,9 @@ It's a must-do for static pages, but it needs to be put in balance with not doin

 Material-UI supports the most recent versions of React, starting with ^16.8.0 (the one with the hooks).
 Have a look at the older [versions](https://material-ui.com/versions/) for backward compatibility.
+
+## TypeScript
+
+Material-UI supports the most recent versions of TypeScript.
+However, the codebase doesn't follow semver for types.
+You can expect breaking changes between two minor versions.
+As of now, we support TypeScript ^3.8.0.
+Have a look at the older [versions](https://material-ui.com/versions/) for backward compatibility.
diff --git a/packages/material-ui/package.json b/packages/material-ui/package.json
index e682bb751..0b43f57b5 100644
--- a/packages/material-ui/package.json
+++ b/packages/material-ui/package.json
@@ -39,9 +39,13 @@
   "peerDependencies": {
     "@types/react": "^16.8.6",
     "react": "^16.8.0",
-    "react-dom": "^16.8.0"
+    "react-dom": "^16.8.0",
+    "typescript": "^3.8.0"
   },
   "peerDependenciesMeta": {
+    "typescript": {
+      "optional": true
+    },
     "@types/react": {
       "optional": true
     }

[NMinhNguyen]

I'm happy to add that as long as we say 3.2.4 :stuck_out_tongue: (or I can just pass on this breaking change to our Angular users, I don't really care much haha)

[oliviertassinari]

@NMinhNguyen So they are a couple of topics:

1. Should we add an optional peer dependency to warn when using a wrong version? Doesn't it even work?
2. Should we follow semver for our types?
3. What should be the minimum TypeScript supported version?

[NMinhNguyen]

• Should we add an optional peer dependency to warn when using a wrong version? Doesn't it even work?

Not sure how well it works as I haven't used optional peer dependencies, but I'm for it

• Should we follow semver for our types?

I think (but not 100% sure as I'm not a TS user myself) other packages do tend to follow semver for types, but I'm happy to be corrected!

• What should be the minimum TypeScript supported version?

Preferably the version that MUI v4 originally said it supported (3.2.4), but I guess that depends on the point above.

[NMinhNguyen]

@eps1lon actually I just noticed another error that I initially omitted from the PR description because I thought it was valid, but upon further inspection I see that children became required in TS, but actually became optional in JS: https://github.com/mui-org/material-ui/pull/20298/files#diff-c4e405e74787b5750e61d8305589f825

Should the TS be updated to match JS or the other way around?

[NMinhNguyen]

@eps1lon did you see the comment above?

[eps1lon]

@eps1lon did you see the comment above?

I think React.ReactNode includes optional

[NMinhNguyen]

packages/dialog/examples/BasicDialog.tsx:44:8 - error TS2741: Property 'children' is missing in type '{ open: false; }' but required in type 'DialogProps'.

44       <Dialog open={false} />
          ~~~~~~

  node_modules/@material-ui/core/Dialog/Dialog.d.ts:20:3
    20   children: React.ReactNode;
         ~~~~~~~~
    'children' is declared here.

if I specify children, then the error goes away. I think it's more about the left hand side (i.e. children) than it is about React.ReactNode (although this does include undefined and null) but I could be wrong.

Edit

Updated the error above by just using <Dialog open={false} /> to reduce noise from our own Dialog wrapper