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

fix: warn when choices prop is undefined #8309

Merged
merged 4 commits into from
Oct 27, 2022

Conversation

thdk
Copy link
Contributor

@thdk thdk commented Oct 26, 2022

fixes #8308

@@ -161,6 +162,16 @@ export const SelectInput = (props: SelectInputProps) => {
source: sourceProp,
});

warning(
source === undefined,
Copy link
Member

Choose a reason for hiding this comment

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

This is wrong, as choices will always be undefined here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Both source and allChoices variables are set by useChoicesContext just above this line. allChoices is set asynchrounously (return value of useList hook) so there we have to check for isLoading !== true before we throw. However, the source variable is set synchronously either by prop or from ChoicesContext and should be defined here.

@@ -161,6 +162,16 @@ export const SelectInput = (props: SelectInputProps) => {
source: sourceProp,
});

warning(
Copy link
Member

Choose a reason for hiding this comment

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

The component should throw if it cannot render, not warn

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree, I'll change this also for the components from where I found this snippet.

@fzaninotto fzaninotto merged commit 6d86046 into marmelab:master Oct 27, 2022
@fzaninotto
Copy link
Member

Thanks!

@fzaninotto fzaninotto added this to the 4.5.0 milestone Oct 27, 2022
@thdk thdk deleted the choices-undefined-warning branch October 27, 2022 13:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

undefined is not a valid value for choices prop of SelectInput
2 participants