search

marmelab / react-admin

A frontend Framework for single-page applications on top of REST/GraphQL APIs, using TypeScript, React and Material Design
http://marmelab.com/react-admin
MIT License
24.89k stars 5.24k forks source link

undefined is not a valid value for choices prop of SelectInput #8308

Closed thdk closed 1 year ago

thdk commented 1 year ago

When you pass undefined as value to the SelectInput component you'll get an error like cannot read property map of undefined. So either the SelectInput component should be fixed to deal with choices being undefined or the choices prop should not be optional.

What you were expecting: I did not expect to be able to build react-admin when undefined is used as value for the choices prop of the SelectInput component. However, since I can build I would have expected an empty dropdown control to be shown.

What happened instead: See stacktrace posted below.

Steps to reproduce:

<SelectInput choices={undefined} />}

Related code: Below code should not compile because data may be undefined on first render resulting in js errors when trying to access it's properties.

const { data } = useGetList<Merchant>('merchants');
  return (
      <Create {...props}>
          <SimpleForm>
                      {<SelectInput source="category" choices={data} />}
          </SimpleForm>
      </Create>javascript

Other information:

This can be fixed by removing the questionmark here: https://github.com/marmelab/react-admin/blob/6503cb3c6aa50f5048edf7bdcb5ee22fc3ce6c10/packages/ra-core/src/form/useChoices.tsx#L16

Environment

React will try to recreate this component tree from scratch using the error boundary you provided, ErrorBoundary2.

fzaninotto commented 1 year ago

This snippet strikes me:

  const { data } = useGetList<Merchant>('merchants');
  return (
      <Create {...props}>
          <SimpleForm>
                      {<SelectInput source="category" choices={data} />}
          </SimpleForm>
      </Create>
  );

You shouldn't need to do that, and instead you should use <ReferenceInput>.

That's also the reason why choices are not required: when inside a ReferenceInput, the choices can be undefined, and then the SelectInput grabs them from the ChoicesContext filled by ReferenceInput.

So we won't remove that error - but we probably need to make it more explicit.

thdk commented 1 year ago

Oops! Thanks for point that out, just picked up on react-admin since a long time.

So I quickly went to create a PR to make this more explicit, however, it turns out it used to be explicit in the past. PR #7185 removed the warning when choices is undefined. Other input controls like CheckboxGroupInput still have the warning in place. PR incoming :)