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 warning when passing partial props to useListContext and other view context hooks #5802

Merged
merged 8 commits into from Feb 13, 2021

Conversation

Luwangel
Copy link
Contributor

@Luwangel Luwangel commented Jan 20, 2021

React-admin wraps lists, create, edit and show views inside contexts. The main goal is to avoid passing props from the top component to every child one by one.

For example an Edit View is composed of multiple layout elements:

  • A form
  • An action toolbar
  • A title
    ...

And these layout elements are composed of atomic elements (and some of them are iterator displaying smaller elements):

  • Datagrid (iterator)
  • ArrayField (iterator)
  • TextInput
  • Select

Thanks to the modularity of react-admin, you can write your own atomic elements.

Nevertheless, from a developer perspective, it's difficult to know which props are implicitly injected from the top because there are too many levels of components. And the reason why we added the contexts.

For example, imagine you want to create a custom iterator (a customized list of elements). Thanks to the contexts, you just need to connect the useListContext hook and directly get the props you need like the loaded state, the data, the total number of elements.


When we developed the contexts, we deprecated the props mechanism with the intention to remove it in the next major version (v4). During this time, we encouraged the users to change their custom code by displaying a warning:

"Show components must be used inside a <ShowContext.Provider>. Relying on props rather than context to get Show data and callbacks is deprecated and won't be supported in the next major version of react-admin."


But, as a result, some smaller components became unusable without surrounding them by the appropriate context. It's the case of the <Datagrid> which now calls the useListContext.

And it's not a pleasant developer experience because it means adding a <ListContext> around your custom iterator. That's why we decided to rely on both props and context.

Starting from this PR, you can override the value of the useListContext, the useEditContext, the useCreateContext and the useShowContext` hooks.

It's as simple as passing arguments to a function, and the props will take precedence over the values of the context.

const context = useListContext(props);

If there is no context, it works too like this <Datagrid> (see the screenshot attached):

<Datagrid
   // You should pass the following props to make it work without a context
   basePath=""
   currentSort={currentSort}
   data={data}
   ids={ids}
   selectedIds={selectedIds}
   loaded={loaded}
   total={total}
>
   <TextField source="id" sortable={false} />
   <TextField source="title" sortable={false} />
</Datagrid>

Todo

  • Refact the useListContext, the useEditContext, the useCreateContext and the useShowContext hooks to use props
  • Add an example in the simple example
  • Write documentation

Screenshot

It's now possible to use a Datagrid without putting it inside a ListContext:

Sélection_004

@Luwangel Luwangel added the WIP Work In Progress label Jan 20, 2021
Base automatically changed from next to master January 30, 2021 07:44
@fzaninotto fzaninotto changed the title Use props over the context values in use context hooks Fix warning when passing partial props to useListContext and other view context hooks Feb 13, 2021
@fzaninotto fzaninotto merged commit 93c94b7 into master Feb 13, 2021
@fzaninotto fzaninotto deleted the use-props-over-context branch February 13, 2021 17:08
@fzaninotto fzaninotto modified the milestones: 3.12.3, 3.12.4 Feb 13, 2021
@stensrud
Copy link
Contributor

stensrud commented Feb 26, 2021

@Luwangel FYI, this PR broke our production system today. Seems selectedIds always generates a new object on every render, and our code const { selectedIds } = useListContext(); useEffect(()=>{...force rerender...}, [selectedIds]) was fired in an infinite loop.

We were on react-admin version 3.8.3 which depends on ra-core version ^3.8.3 which resolved to 3.12.5.

@fzaninotto
Copy link
Member

@stensrud Would you mind opening a new issue for your problem, following the issue template (and providing a way to reproduce the problem)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WIP Work In Progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants