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] Revert apiRef to be MutableRefObject for React versions < 19 #16279

Merged
merged 4 commits into from
Jan 24, 2025

Conversation

arminmeh
Copy link
Contributor

@arminmeh arminmeh commented Jan 21, 2025

Closes #16135

Because I would like to cherry pick this to v7 I have left these two items for a follow-up PR that will be for v8 only

Results of these changes:
React 18
R18

React 19
R19

CC: @flaviendelangle @alexfauquette @LukasTy
We can extend this also to other packages

@arminmeh arminmeh added bug 🐛 Something doesn't work component: data grid This is the name of the generic UI component, not the React module! typescript needs cherry-pick The PR should be cherry-picked to master after merge v7.x labels Jan 21, 2025
@arminmeh arminmeh requested a review from a team January 21, 2025 08:10
@mui-bot
Copy link

mui-bot commented Jan 21, 2025

Deploy preview: https://deploy-preview-16279--material-ui-x.netlify.app/

Generated by 🚫 dangerJS against d914b33

Copy link
Member

@LukasTy LukasTy left a comment

Choose a reason for hiding this comment

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

Great work. 👍
Sorry for not testing thoroughly enough and thus not flagging this problem when working on the migration PR. 🙈

@arminmeh
Copy link
Contributor Author

Great work. 👍 Sorry for not testing thoroughly enough and thus not flagging this problem when working on the migration PR. 🙈

Well, you were not alone in that 😅

@arminmeh
Copy link
Contributor Author

arminmeh commented Jan 22, 2025

@cherniavskii just to confirm and give a context for other as well.
We want to have the following outcome

for type T v7 v8
R18 MutableRefObject<T> MutableRefObject<T | null | undefined>
R19 RefObject<T> RefObject<T | null>

Or R19 for v7 should also have null?

Update:

Types matrix

for type T v7 v8
R18 MutableRefObject<T> MutableRefObject<T | null>
R19 RefObject<T> RefObject<T | null>

@cherniavskii
Copy link
Member

@arminmeh For v8, either null or undefined are fine. Having both doesn't serve any additional purpose.

@arminmeh arminmeh force-pushed the mutable-ref-object-r18 branch from 04da28e to d914b33 Compare January 23, 2025 10:48
Copy link
Member

@cherniavskii cherniavskii left a comment

Choose a reason for hiding this comment

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

👍🏻

@arminmeh arminmeh merged commit 308c089 into mui:master Jan 24, 2025
18 checks passed
Copy link

Cherry-pick PRs will be created targeting branches: v7.x

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work component: data grid This is the name of the generic UI component, not the React module! needs cherry-pick The PR should be cherry-picked to master after merge typescript v7.x
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[data grid] Possible bug in useGridApiRef after upgrade to v7.23.6
5 participants