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

[DataGrid] Fix conflict with onResize added to React.HTMLAttributes #6797

Merged
merged 1 commit into from
Nov 14, 2022

Conversation

vizv
Copy link
Contributor

@vizv vizv commented Nov 9, 2022

I got the following error when build my TypeScript project with @mui/x-data-grid@5.17.10:

node_modules/@mui/x-data-grid/components/GridAutoSizer.d.ts:6:18 - error TS2430: Interface 'AutoSizerProps' incorrectly extends interface 'Omit<HTMLAttributes<HTMLDivElement>, "children">'.
  Types of property 'onResize' are incompatible.
    Type '((size: AutoSizerSize) => void) | undefined' is not assignable to type 'ReactEventHandler<HTMLDivElement> | undefined'.
      Type '(size: AutoSizerSize) => void' is not assignable to type 'ReactEventHandler<HTMLDivElement>'.
        Types of parameters 'size' and 'event' are incompatible.
          Type 'SyntheticEvent<HTMLDivElement, Event>' is missing the following properties from type 'AutoSizerSize': height, width

6 export interface AutoSizerProps extends Omit<React.HTMLAttributes<HTMLDivElement>, 'children'> {
                   ~~~~~~~~~~~~~~


Found 1 error in node_modules/@mui/x-data-grid/components/GridAutoSizer.d.ts:6

After investigation, DefinitelyTyped/DefinitelyTyped#63076 adds onResize event to video elements for @types/react@18.0.25 due to facebook/react#21973, which causes the issue.

To fix this error, I omitted property onResize when extending AutoSizerProps from HTMLDivElement attributes.

Note this PR might also need to backport to master branch for v5

@mui-bot
Copy link

mui-bot commented Nov 9, 2022

Messages
📖 Netlify deploy preview: https://deploy-preview-6797--material-ui-x.netlify.app/

These are the results for the performance tests:

Test case Unit Min Max Median Mean σ
Filter 100k rows ms 605.9 898.1 661.7 720.88 107.09
Sort 100k rows ms 585.3 1,002.6 790.7 796.34 132.192
Select 100k rows ms 170 290 255.4 241.24 42.256
Deselect 100k rows ms 133.7 248.3 148.1 176.82 46.22

Generated by 🚫 dangerJS against 2c72e42

@vizv vizv force-pushed the fix-onresize-prop-conflict branch 2 times, most recently from 21343b4 to 676761b Compare November 10, 2022 04:38
@vizv vizv changed the base branch from master to next November 10, 2022 04:38
@zannager zannager added the component: data grid This is the name of the generic UI component, not the React module! label Nov 10, 2022
@m4theushw m4theushw changed the title [GridAutoSizer] Fix onResize property type conflict [DataGrid] Fix conflict with onResize added to React.HTMLAttributes Nov 10, 2022
Copy link
Member

@m4theushw m4theushw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution! It will close #6807.

Could you also bump @types/react to force the error? You only need to bring the changes from #6742.

@vizv
Copy link
Contributor Author

vizv commented Nov 11, 2022

Thanks for the contribution! It will close #6807.

Could you also bump @types/react to force the error? You only need to bring the changes from #6742.

Done

@m4theushw m4theushw merged commit 07d29b4 into mui:next Nov 14, 2022
@m4theushw m4theushw mentioned this pull request Nov 14, 2022
1 task
@vizv vizv deleted the fix-onresize-prop-conflict branch November 15, 2022 01:52
m4theushw pushed a commit to m4theushw/mui-x that referenced this pull request Nov 16, 2022
m4theushw pushed a commit to m4theushw/mui-x that referenced this pull request Nov 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: data grid This is the name of the generic UI component, not the React module! typescript
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants