[types] Bump minimum required TypeScript version to 3.5 #24795

petyosi · 2021-02-06T08:56:30Z

Breaking changes

[core] Increase the minimum version of TypeScript supported from v3.2 to v3.5.
[@material-ui/types] Rename the exported Omit type in @material-ui/types. The module is now called DistributiveOmit. The change removes the confusion with the built-in Omit helper introduced in TypeScript v3.5. The built-in Omit, while similar, is non-distributive. This leads to differences when applied to union types. See this StackOverflow answer for further details.

-import { Omit } from '@material-ui/types';
+import { DistributiveOmit } from '@material-ui/types';

As per #24752 (comment),
built-in Omit usage should be considered a bug. There's one exception to
that in packages/material-ui-lab/src/DayPicker/PickersSlideTransition.tsx.

This change also removes the Omit export from the package. The documentation examples resort to the built-in Omit. Let me know if this makes sense.

I have followed (at least) the PR section of the contributing guide.

mui-pr-bot · 2021-02-06T08:58:33Z

Details of bundle changes

Generated by 🚫 dangerJS against 7228d94

oliviertassinari

Clearing the confusion between the two helpers is great. How about we update the documentation of https://next.material-ui.com/getting-started/supported-platforms/#typescript as we rely on TypeScript 3.5 now for the built-in omit?

I think that such breaking change should also have its entry in the migration guide. Alternatively, we could do it in two different PRs (1. Bump TypeScript, 2. Omit confusion). I have no specific preferences.

oliviertassinari

For the second migration notice, I think that we could have it close to this section: https://next.material-ui.com/guides/migration-v4/#supported-browsers-and-node-versions.

docs/src/pages/getting-started/supported-platforms/supported-platforms-es.md

docs/src/pages/guides/migration-v4/migration-v4.md

oliviertassinari · 2021-02-07T18:36:36Z

packages/material-ui-lab/src/ClockPicker/ClockNumbers.tsx

@@ -75,7 +76,7 @@ export const getMinutesNumbers = ({
  value,
  isDisabled,
  getClockNumberText,
-}: Omit<GetHourNumbersOptions, 'ampm' | 'date'> & { value: number }) => {
+}: DistributiveOmit<GetHourNumbersOptions, 'ampm' | 'date'> & { value: number }) => {


We were using the built-in Omit helper previously. Why change it for the distributive one? I guess it's the same question for all the other cases. When should we use one over the other?

I went through all occurances and applied the following:

If applied to an interface, use the built-in Omit, since the distributive one has no effect;

If applied to a union type or to a generic parameter, use DistributiveOmit.

packages/material-ui/src/useAutocomplete/useAutocomplete.d.ts

packages/material-ui/src/TextareaAutosize/TextareaAutosize.d.ts

As per mui#24752 (comment), built-in Omit usage should be considered a bug. There's one exception to that in packages/material-ui-lab/src/DayPicker/PickersSlideTransition.tsx

eps1lon · 2021-02-08T15:39:36Z

Looking at it tomorrow.

oliviertassinari · 2021-02-08T17:25:31Z

I have pushed an update to mention the change of TypeScript version in the migration guide as a standalone entry.

Looking at it tomorrow.

@eps1lon Do you mean that you have approved the changes too quickly?

eps1lon · 2021-02-08T17:41:55Z

@eps1lon Do you mean that you have approved the changes too quickly?

I have not approved all changes.

If this is needed for something we can work on it in a follow-up with tests

eps1lon · 2021-02-11T11:57:58Z

Added a rationale for the bump in TS version (focusing on the link to the DT repo).
Also reverted a bunch of changes where DistributiveOmit was added. In these instance we either know that the input isn't a union or the behavior is new. If we want to change it, we should do so with a test.

eps1lon · 2021-02-11T12:00:15Z

@petyosi Thanks!

oliviertassinari · 2021-02-14T15:32:08Z

packages/material-ui-utils/src/isMuiElement.d.ts

@@ -1,4 +1,5 @@
 import * as React from 'react';
+import { DistributiveOmit } from '@material-ui/types';


@material-ui/types isn't declared as a dependency of the package. It creates this fail: mui/mui-x#1046. Solved in #24936

petyosi mentioned this pull request Feb 6, 2021

[typescript] Replace Omit type with the built-in one #24752

Closed

1 task

oliviertassinari added typescript core Infrastructure work going on behind the scenes labels Feb 7, 2021

oliviertassinari changed the title ~~[typescript] Fix Omit / DistributiveOmit ambiguity~~ [core] Fix Omit/DistributiveOmit TypeScript ambiguity Feb 7, 2021

oliviertassinari reviewed Feb 7, 2021

View reviewed changes

oliviertassinari requested changes Feb 7, 2021

View reviewed changes

oliviertassinari requested a review from eps1lon February 7, 2021 19:57

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Feb 7, 2021

petyosi added 2 commits February 8, 2021 10:22

[typescript] Fix Omit / DistributiveOmit ambiguity

c50e6e9

As per mui#24752 (comment), built-in Omit usage should be considered a bug. There's one exception to that in packages/material-ui-lab/src/DayPicker/PickersSlideTransition.tsx

[docs] Change the minimum TS version to 3.5

1567dba

petyosi force-pushed the distributive-omit branch from e0f0807 to 8a6f1fd Compare February 8, 2021 08:23

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Feb 8, 2021

[docs] Document the Omit type removal + alternative

d3e2487

petyosi force-pushed the distributive-omit branch from 8a6f1fd to d3e2487 Compare February 8, 2021 08:34

petyosi requested a review from oliviertassinari February 8, 2021 08:39

eps1lon approved these changes Feb 8, 2021

View reviewed changes

eps1lon changed the title ~~[core] Fix Omit/DistributiveOmit TypeScript ambiguity~~ [types] Fix Omit/DistributiveOmit TypeScript ambiguity Feb 8, 2021

eps1lon added breaking change package: material-ui Specific to @mui/material package: types Specific to @mui/types and removed core Infrastructure work going on behind the scenes labels Feb 8, 2021

oliviertassinari added 2 commits February 8, 2021 16:18

yarn prettier

1d5d13e

add an entry in the migration guide

6879910

oliviertassinari approved these changes Feb 8, 2021

View reviewed changes

eps1lon self-requested a review February 11, 2021 11:14

Add rationale for change of TS version

342fe40

Revert change of existing behavior

7228d94

If this is needed for something we can work on it in a follow-up with tests

eps1lon approved these changes Feb 11, 2021

View reviewed changes

eps1lon changed the title ~~[types] Fix Omit/DistributiveOmit TypeScript ambiguity~~ [types] Bump minimum required TypeScript version to 3.5 Feb 11, 2021

eps1lon merged commit 058e409 into mui:next Feb 11, 2021

petyosi deleted the distributive-omit branch February 11, 2021 12:00

github-actions bot mentioned this pull request Feb 11, 2021

Update snapshots for pr/24795 eps1lon/mui-scripts-incubator#1842

Closed

oliviertassinari reviewed Feb 14, 2021

View reviewed changes

This was referenced Apr 24, 2021

[docs] Update minimum required TypeScript version #25930

Merged

[core] Enable trailing comma in TypeScript files #25931

Merged

oliviertassinari added this to the v5-beta milestone Aug 17, 2021

oliviertassinari mentioned this pull request Aug 17, 2021

Deprecation warnings for v5 #22074

Closed

42 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[types] Bump minimum required TypeScript version to 3.5 #24795

[types] Bump minimum required TypeScript version to 3.5 #24795

petyosi commented Feb 6, 2021 •

edited by oliviertassinari

Loading

mui-pr-bot commented Feb 6, 2021 •

edited

Loading

oliviertassinari left a comment •

edited

Loading

oliviertassinari left a comment

oliviertassinari Feb 7, 2021

petyosi Feb 8, 2021

eps1lon commented Feb 8, 2021

oliviertassinari commented Feb 8, 2021

eps1lon commented Feb 8, 2021 •

edited

Loading

eps1lon commented Feb 11, 2021

eps1lon commented Feb 11, 2021

oliviertassinari Feb 14, 2021

		@@ -1,4 +1,5 @@
		import * as React from 'react';
		import { DistributiveOmit } from '@material-ui/types';

[types] Bump minimum required TypeScript version to 3.5 #24795

[types] Bump minimum required TypeScript version to 3.5 #24795

Conversation

petyosi commented Feb 6, 2021 • edited by oliviertassinari Loading

Breaking changes

mui-pr-bot commented Feb 6, 2021 • edited Loading

oliviertassinari left a comment • edited Loading

Choose a reason for hiding this comment

oliviertassinari left a comment

Choose a reason for hiding this comment

oliviertassinari Feb 7, 2021

Choose a reason for hiding this comment

petyosi Feb 8, 2021

Choose a reason for hiding this comment

eps1lon commented Feb 8, 2021

oliviertassinari commented Feb 8, 2021

eps1lon commented Feb 8, 2021 • edited Loading

eps1lon commented Feb 11, 2021

eps1lon commented Feb 11, 2021

oliviertassinari Feb 14, 2021

Choose a reason for hiding this comment

petyosi commented Feb 6, 2021 •

edited by oliviertassinari

Loading

mui-pr-bot commented Feb 6, 2021 •

edited

Loading

oliviertassinari left a comment •

edited

Loading

eps1lon commented Feb 8, 2021 •

edited

Loading