[material-ui] Change React.ReactElement<any> to React.ReactElement<unknown> #43402

sai6855 · 2024-08-22T03:48:37Z

Part of: #42380

As of now, there are 7 instances of React.ReactElement left to be migrated except for base/, joy/ folders. Remaining chnages are not straight forward changes from any to unknown, as those requires few more type changes. i'll open a new PR for left over instances

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

mui-bot · 2024-08-22T03:51:13Z

Netlify deploy preview

https://deploy-preview-43402--material-ui.netlify.app/

Bundle size report

No bundle size changes (Toolpad)
No bundle size changes

Generated by 🚫 dangerJS against d013bd8

sai6855 · 2024-08-22T05:30:54Z

packages-internal/test-utils/src/describeConformance.tsx

@@ -91,7 +91,7 @@ function throwMissingPropError(field: string): never {
 * the root component.
 */
 export function testClassName(
-  element: React.ReactElement<any>,
+  element: React.ReactElement<HTMLElement & { 'data-testid'?: string }>,


All changes of any to unknown (some were changed to HTMLElement) in this PR are straight forward, except for changes in this file.

Could you explain the criteria you used to choose the type of element in the different functions? I'm trying to understand the impact it will have on the describeConformance usage. Will elements passed to testClassName without the 'data-tested' prop throw a type error? Where would that error show?

You are right, calling testClassName without passing data-testid is throwing type error.

This is tricky to fix, data-testid is added to element as below.

material-ui/packages-internal/test-utils/src/describeConformance.tsx

Line 108 in bb620d8

React.cloneElement(element, { className, 'data-testid': testId }),

I think we have 2 options to fix

add data-testid type to argument as above, but it throws type error when consumers of this function doesn't add data-testid type

Keep element argument type as React.ReactElement<unknown> and type cast element type near it's usage as shown below.

React.cloneElement(element + as React.ReactElement<unknown & {[data-testid]:string}> , { className, 'data-testid': testId }),

By going with option 2 , we can keep types of consumers of this function to be flexible. what do you think?

What i did for now is, i've reverted all changes in this file here, what ever option we choose i'll create a seprate PR as this PR is alread too big and i don't want this descion to block this PR from merging.

Let's open another PR and discuss it there 👌🏼

I think we can create a type that is the same as ReactElement but supports data- attributes

DiegoAndai

Thanks for taking this @sai6855, I left some questions

DiegoAndai · 2024-08-28T16:55:29Z

docs/data/material/components/app-bar/ElevateAppBar.tsx

@@ -13,7 +13,7 @@ interface Props {
   * You won't need it on your project.
   */
  window?: () => Window;
-  children?: React.ReactElement<any>;
+  children?: React.ReactElement<unknown & { elevation?: number }>;


Why is & { elevation?: number } needed?

Here elevation prop is added to children, since unknown type doesn't have elevation type, i explicitly added. I'm not entirely sure if this is optimal, but please let me know if you have better suggestion.

material-ui/docs/data/material/components/app-bar/ElevateAppBar.tsx

Line 32 in 6019f2a

elevation: trigger ? 4 : 0,

I think we can have only the elevation prop:

Suggested change

children?: React.ReactElement<unknown & { elevation?: number }>;

children?: React.ReactElement<{ elevation?: number }>;

As unknown is absorbed in intersections:

https://www.typescriptlang.org/docs/handbook/release-notes/typescript-3-0.html#new-unknown-top-type:~:text=//%20In%20an%20intersection%20everything%20absorbs%20unknown

fixed here 3ff9828

docs/src/components/typography/SectionHeadline.tsx

DiegoAndai · 2024-08-28T16:59:07Z

packages-internal/test-utils/src/createRenderer.tsx

@@ -306,7 +306,7 @@ function render(
      traceSync('forceUpdate', () =>
        testingLibraryRenderResult.rerender(
          React.cloneElement(element, {
-            'data-force-update': String(Math.random()),
+            ['data-force-update' as string]: String(Math.random()),


Why is as string required?

here element type is unknown, since unknown doesn't have data-force-update type, i've explicitly marked type as as string

But if I understand correctly, this is casting the key, not the value 🤔

What's the error thrown without the cast?

Instead of casting, i've tried to fix type error properly here =>09f5298

DiegoAndai · 2024-08-28T18:28:25Z

packages/mui-system/src/cssVars/createCssVarsProvider.d.ts

@@ -82,7 +82,7 @@ export interface CreateCssVarsProviderResult<
        disableStyleSheetGeneration?: boolean;
      }
    >,
-  ) => React.ReactElement<any>;
+  ) => React.ReactElement<unknown>;


What do you think should this type be @siriwatknp? It's the return value of CssVarsProvider, we should have it's props somewhere, right?

I think it's better to change to React.JSX.Element based on

https://www.typescriptlang.org/play/?ts=4.1.5#code/JYWwDg9gTgLgBAJQKYEMDG8BmUIjgcilQ3wG4BYAKCrQgDsBneAFSSbgF44AKMHMBgC44Abzgw2MYUyjA6AczgBfAJScAfKKpwdcIjACuUOnAA8AE2AA3OAHp1VJVWfVXVU63YSmHAES+7dVIgA

packages/mui-styles/src/ServerStyleSheets/ServerStyleSheets.d.ts

DiegoAndai · 2024-08-28T18:32:11Z

packages/mui-styled-engine/src/GlobalStyles/GlobalStyles.d.ts

@@ -8,4 +8,4 @@ export interface GlobalStylesProps<Theme = {}> {

 export default function GlobalStyles<Theme = {}>(
  props: GlobalStylesProps<Theme>,
-): React.ReactElement<any>;
+): React.ReactElement<unknown>;


@siriwatknp what do you think this type should be?

I think it should be React.JSX.Element:

https://www.typescriptlang.org/play/?ts=4.1.5#code/JYWwDg9gTgLgBAJQKYEMDG8BmUIjgcilQ3wG4BYAKCrQgDsBneAFSSbgF44AKMHMBgC44Abzgw2MYUyjA6AczgBfAJScAfKKpwdcIjACuUOnAA8AE2AA3OAHp1VJVWfVXVU63YSmHAES+7dVIgA

DiegoAndai · 2024-08-28T18:35:36Z

packages-internal/test-utils/src/describeConformance.tsx

@@ -91,7 +91,7 @@ function throwMissingPropError(field: string): never {
 * the root component.
 */
 export function testClassName(
-  element: React.ReactElement<any>,
+  element: React.ReactElement<HTMLElement & { 'data-testid'?: string }>,


Could you explain the criteria you used to choose the type of element in the different functions? I'm trying to understand the impact it will have on the describeConformance usage. Will elements passed to testClassName without the 'data-tested' prop throw a type error? Where would that error show?

sai6855 · 2024-10-03T09:39:36Z

@DiegoAndai updated PR based on #43402 (comment) #43402 (comment) it is ready for final review

DiegoAndai · 2024-10-03T16:09:41Z

packages/mui-styled-engine-sc/src/GlobalStyles/GlobalStyles.spec.tsx

@@ -25,7 +25,7 @@ export interface GlobalStylesProps {
  styles: SCGlobalStylesProps<Theme>['styles'];
 }

-function GlobalStyles(props: GlobalStylesProps): React.ReactElement<any> {
+function GlobalStyles(props: GlobalStylesProps): React.ReactElement<unknown> {


Seems like we should change this one to React.JSX.Element as well.

DiegoAndai · 2024-10-03T16:09:47Z

packages/mui-styled-engine/src/GlobalStyles/GlobalStyles.spec.tsx

@@ -25,7 +25,7 @@ export interface GlobalStylesProps {
  styles: EmGlobalStylesProps<Theme>['styles'];
 }

-function GlobalStyles(props: GlobalStylesProps): React.ReactElement<any> {
+function GlobalStyles(props: GlobalStylesProps): React.ReactElement<unknown> {


Seems like we should change this one to React.JSX.Element as well.

DiegoAndai · 2024-10-03T16:10:26Z

packages/mui-styled-engine-sc/src/GlobalStyles/GlobalStyles.d.ts

@@ -8,4 +8,4 @@ export interface GlobalStylesProps<Theme extends object = {}> {

 export default function Global<Theme extends object = {}>(
  props: GlobalStylesProps<Theme>,
-): React.ReactElement<any>;
+): React.ReactElement<unknown>;


Seems like we should change this one to React.JSX.Element as well.

DiegoAndai · 2024-10-03T18:34:11Z

Thanks @sai6855!

change

f766a4c

sai6855 marked this pull request as draft August 22, 2024 03:52

sai6855 added typescript React 19 support PRs required to support React 19 labels Aug 22, 2024

sai6855 added 3 commits August 22, 2024 09:37

change

4e3a8f7

fix

b269f20

fix

37340ed

sai6855 force-pushed the any-unknown branch from f766a4c to 37340ed Compare August 22, 2024 05:09

sai6855 added 3 commits August 22, 2024 10:48

fix

2c996c7

fix

c7b4c40

fix

9168dcd

sai6855 commented Aug 22, 2024

View reviewed changes

sai6855 added 3 commits August 22, 2024 11:09

fix

60c55e5

fix

e5605f3

fix

94f691e

sai6855 requested a review from DiegoAndai August 22, 2024 06:07

sai6855 mentioned this pull request Aug 22, 2024

[material-ui] Change React.ReactElement<any,any> type from any to unknown #43358

Merged

1 task

sai6855 marked this pull request as ready for review August 22, 2024 06:08

trigger

6019f2a

DiegoAndai reviewed Aug 28, 2024

View reviewed changes

sai6855 added 5 commits September 3, 2024 16:01

revert mui-styles change

b583310

revert describeconformance type changes

7d3d548

fix sectionheadline

9f1b74d

fix eleveation

3ff9828

add data-* type to element

09f5298

sai6855 force-pushed the any-unknown branch from 4511fd4 to 09f5298 Compare September 10, 2024 07:16

Merge branch 'master' into any-unknown

c7483d7

oliviertassinari mentioned this pull request Sep 15, 2024

[core] React 19 compatibility mui/base-ui#605

Merged

Merge branch 'master' into any-unknown

4b9919d

update type to React.JSX.Element;

4f2f5c1

DiegoAndai reviewed Oct 3, 2024

View reviewed changes

update remaining files types

d013bd8

DiegoAndai approved these changes Oct 3, 2024

View reviewed changes

DiegoAndai merged commit 46869bc into mui:master Oct 3, 2024
22 checks passed

This was referenced Oct 31, 2024

[Snyk] Upgrade @mui/icons-material from 6.1.2 to 6.1.3 sensasi-delight/simulasi-grading-sawit#29

Merged

[Snyk] Upgrade @mui/material from 6.1.2 to 6.1.3 sensasi-delight/simulasi-grading-sawit#30

Merged

Souviksamanta34 mentioned this pull request Oct 31, 2024

[Snyk] Upgrade @mui/icons-material from 6.1.2 to 6.1.3 Souviksamanta34/gruham#17

Open

bansikah22 mentioned this pull request Oct 31, 2024

[Snyk] Upgrade @mui/material from 6.1.2 to 6.1.3 bansikah22/user-fullstack#11

Merged

This was referenced Oct 31, 2024

[Snyk] Upgrade @mui/material from 5.10.10 to 6.1.3 https-quantumblockchainai-atlassian-net/ossinsight#970

Open

[Snyk] Upgrade @mui/icons-material from 5.10.9 to 6.1.3 https-quantumblockchainai-atlassian-net/ossinsight#971

Open

This was referenced Nov 1, 2024

[Snyk] Upgrade @mui/icons-material from 6.1.2 to 6.1.3 SE310-Team-1/SE310_Fitness_Tracker#99

Open

[Snyk] Upgrade @mui/material from 6.1.2 to 6.1.3 SE310-Team-1/SE310_Fitness_Tracker#100

Open

josetutis mentioned this pull request Nov 2, 2024

[Snyk] Upgrade @mui/styled-engine-sc from 6.0.0-alpha.18 to 6.1.3 lazycloud0/KingsHeathClangersApp#160

Open

Hoang-Nguyen-Huy mentioned this pull request Nov 2, 2024

[Snyk] Upgrade @mui/icons-material from 6.1.2 to 6.1.3 Hoang-Nguyen-Huy/fer-lab1#19

Merged

Hoang-Nguyen-Huy mentioned this pull request Nov 9, 2024

[Snyk] Upgrade @mui/icons-material from 6.1.3 to 6.1.4 Hoang-Nguyen-Huy/fer-lab1#20

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[material-ui] Change React.ReactElement<any> to React.ReactElement<unknown> #43402

[material-ui] Change React.ReactElement<any> to React.ReactElement<unknown> #43402

sai6855 commented Aug 22, 2024 •

edited

Loading

mui-bot commented Aug 22, 2024 •

edited

Loading

sai6855 Aug 22, 2024 •

edited

Loading

DiegoAndai Aug 28, 2024

sai6855 Sep 3, 2024

DiegoAndai Sep 9, 2024

DiegoAndai left a comment

DiegoAndai Aug 28, 2024

sai6855 Sep 3, 2024

DiegoAndai Sep 9, 2024

sai6855 Sep 10, 2024

DiegoAndai Aug 28, 2024

sai6855 Sep 3, 2024

DiegoAndai Sep 9, 2024

sai6855 Sep 10, 2024

DiegoAndai Aug 28, 2024

siriwatknp Oct 3, 2024

DiegoAndai Aug 28, 2024

siriwatknp Oct 3, 2024

DiegoAndai Aug 28, 2024

sai6855 commented Oct 3, 2024

DiegoAndai Oct 3, 2024

DiegoAndai Oct 3, 2024

DiegoAndai Oct 3, 2024

DiegoAndai commented Oct 3, 2024

	children?: React.ReactElement<unknown & { elevation?: number }>;
	children?: React.ReactElement<{ elevation?: number }>;

[material-ui] Change React.ReactElement<any> to React.ReactElement<unknown> #43402

[material-ui] Change React.ReactElement<any> to React.ReactElement<unknown> #43402

Conversation

sai6855 commented Aug 22, 2024 • edited Loading

mui-bot commented Aug 22, 2024 • edited Loading

Netlify deploy preview

Bundle size report

sai6855 Aug 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DiegoAndai left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sai6855 commented Oct 3, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DiegoAndai commented Oct 3, 2024

sai6855 commented Aug 22, 2024 •

edited

Loading

mui-bot commented Aug 22, 2024 •

edited

Loading

sai6855 Aug 22, 2024 •

edited

Loading