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

[data grid] add validation in useGridApiOptionHandler #14463

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

k-rajat19
Copy link
Contributor

doing this will ensure that we are not registering multiple event handlers with the same event name :)

@zannager zannager added the component: data grid This is the name of the generic UI component, not the React module! label Sep 3, 2024
Copy link
Contributor

@romgrk romgrk 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 work, but I don't think we should merge this :| I don't see a need to add validation code if it's not catching an actual problem.

@k-rajat19
Copy link
Contributor Author

k-rajat19 commented Sep 3, 2024

This will help prevent any future regressions.
Currently, we are not registering multiple event handlers with the same event name anywhere in the codebase but in case we do so somewhere deep in the codebase and forget that we have already registered a handler for that event name, it will prevent that.
Also, we are exporting useGridApiOptionHandler so if users try to do the same it will prevent them too.

these are my Points of view. however, you know better than me, Feel free to close this if you want :)

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!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants