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] Add possibility of null in the return type of the useGridApiRef() hook #16353

Merged
merged 9 commits into from
Jan 27, 2025

Conversation

arminmeh
Copy link
Contributor

@arminmeh arminmeh commented Jan 27, 2025

Closes #16000

Also related to #16279
since it is adding the v8 part of the types matrix

Changelog

Breaking changes

  • Return type of the useGridApiRef() hook and the type of apiRef prop are updated to explicitly include the possibilty of null. In addition to this, useGridApiRef() returns a reference that is initialized with null instead of {}.

    Only the initial value and the type are updated. Logic that initializes the API and its availability remained the same, which means that if you could access API in a particular line of your code before, you are able to access it as well after this change.

    Depending on the context in which the API is being used, you can decide what is the best way to deal with null value. Some options are:

    • Use optional chaining
    • Use non-null assertion operator if you are sure your code is always executed when the apiRef is not null
    • Return early if apiRef is null
    • Throw an error if apiRef is null

@arminmeh arminmeh added bug 🐛 Something doesn't work breaking change component: data grid This is the name of the generic UI component, not the React module! typescript labels Jan 27, 2025
@arminmeh arminmeh requested a review from a team January 27, 2025 13:31
@mui-bot
Copy link

mui-bot commented Jan 27, 2025

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.

It's messy, but the type is more accurate now 👍🏻
Thanks!

@arminmeh arminmeh merged commit 4ebe54a into mui:master Jan 27, 2025
18 checks passed
@lauri865
Copy link
Contributor

@arminmeh, for tests though, wouldn't it be better to have an internal implementation that is not null? I'd rather have a runtime error if I forgot to bind the apiRef to the grid than have an optional chaining function call that silently never gets invoked.

@arminmeh
Copy link
Contributor Author

@lauri865 that makes sense.
I will go through all test changes later and check where the change could actually make the test false positive.

For most of the test it will not matter, since the optional chaining affects certain action(s) that if silently fails will fail the test assertion

@lauri865
Copy link
Contributor

@lauri865 that makes sense.

I will go through all test changes later and check where the change could actually make the test false positive.

For most of the test it will not matter, since the optional chaining affects certain action(s) that if silently fails will fail the test assertion

I didn't mean that the change would break any old tests, but more a thought about writing new tests going forward.

@arminmeh
Copy link
Contributor Author

I didn't mean that the change would break any old tests

Well, you made me doubt it now 😅

I didn't spend to much time on the test updates as I tried to make the CI pass as fast as possible
But as I said I plan to go once more through it and set an example for future as well (that also means changes in the current tests)

A-s-h-o-k pushed a commit to A-s-h-o-k/mui-x that referenced this pull request Feb 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change bug 🐛 Something doesn't work 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.

[data grid] Improve typing for useGridApiRef
4 participants