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] getValueError in valueGetter if incorrect field is supplied #755

Conversation

ZeeshanTamboli
Copy link
Member

@ZeeshanTamboli ZeeshanTamboli commented Dec 18, 2020

Fixes #749

Render empty row data if column field for valueGetter method is incorrect instead of app crashing.

Not sure if throwing error in console instead would be a better idea like:

 const col = api.getColumnFromField(field);
 if(!col) {
    throw new Error(`Material-UI: The field ${field} supplied to valueGetter element is invalid.`);
 }
 if (!col.valueGetter) {
   return rowModel[field];
 }

@oliviertassinari oliviertassinari added bug 🐛 Something doesn't work component: data grid This is the name of the generic UI component, not the React module! labels Dec 18, 2020
Copy link
Member

@oliviertassinari oliviertassinari left a comment

Choose a reason for hiding this comment

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

I was wondering too about what should be the expected outcome of getValue for a column that isn't defined. I have pushed into the direction of gracefully working, for the sake of simplicity. getValue exists in the first place to call valueGetter if it exists. I guess that some developers might want to always use it. At least, the ones starting from the demos of the documentation. But we could also say the opposite, go pull data from the raw data instead. No matter what, we need to change something, we can't fail with an obscure error.

@oliviertassinari oliviertassinari force-pushed the issue-749-getValueError-in-valueGetter branch from 036b08c to 6b6ae32 Compare December 21, 2020 10:51
@oliviertassinari oliviertassinari merged commit ead6bb0 into mui:master Dec 21, 2020
@oliviertassinari
Copy link
Member

@ZeeshanTamboli Thanks for spending time on this! I hope the final solution matches what you had expected. Happy to continue the conversion at any point. In #755 (comment), we still need to test that the returned value is correct as the branching could have been implemented in a different way, for instead, before the changes, it was crashing.

@ZeeshanTamboli ZeeshanTamboli deleted the issue-749-getValueError-in-valueGetter branch December 21, 2020 18:54
@ZeeshanTamboli
Copy link
Member Author

ZeeshanTamboli commented Dec 21, 2020

@ZeeshanTamboli Thanks for spending time on this! I hope the final solution matches what you had expected. Happy to continue the conversion at any point. In #755 (comment), we still need to test that the returned value is correct as the branching could have been implemented in a different way, for instead, before the changes, it was crashing.

@oliviertassinari Thanks! Happy to help! Saw the implementation now, and it's even better to show the warning. Looking forward to contribute more hopefully. :)

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

Successfully merging this pull request may close these issues.

[DataGrid] getValue error in valueGetter
4 participants