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

ResourceContextProvider isn't documented, and doesn't drive the <List> component #5943

Closed
andrewlorenz opened this issue Feb 22, 2021 · 8 comments · Fixed by #6017
Closed
Labels

Comments

@andrewlorenz
Copy link

What you were expecting:
To find ResourceContextProvider documented
When I wrap a <List> with <ResourceContextProvider> I expected it to provide the exact same context to a <List> as the <Resource> component does to its "list" component, but it clearly doesn't inject any props to its child component.
It may be that this context component just doesn't do what I think it does / assume it does based on the odd snippet it appears in. But I seemingly can't get a <List> to work outside of a <Resource> component which is very restrictive.

What happened instead:
It only occurs when its being shown in a particular use case, but never explained beyond that.
I expected my <List> (which is a standalone, not defined as one of the list options for a <Resource>), to work. But it gets none of the props from its <ResourceContextProvider> wrapper.

Steps to reproduce:
just do:

<ResourceContextProvider value={existing_resource_name}>
  <List ...>  (using the same list component defined under this resource <Resource name={existing_resource_name} list={..} )
</ResourceContextProvider>

Environment

  • React-admin version: 3.12.4
  • Last version that did not exhibit the issue (if applicable): n/a
  • React version: 17.0.1
  • Browser: Chrome
  • Stack trace (in case of a JS error):
@djhi
Copy link
Collaborator

djhi commented Feb 23, 2021

Thanks for reporting this. Please provide a sample application showing the issue by forking the following CodeSandbox (https://codesandbox.io/s/github/marmelab/react-admin/tree/master/examples/simple).

@jeremydrichardson
Copy link

jeremydrichardson commented Mar 5, 2021

Here is a simple example I came up with

https://codesandbox.io/s/resourcecontextprovider-kkzji

Go to the url /users/3/show

Look in the console and you will see the error:

Failed prop type: The prop 'hasCreate' is marked as required in 'List', but its value is 'undefined'.

In my own code I'm also getting errors that the props hasEdit, hasList and hasShow are missing as well. Not sure why that's not happening in CodeSandbox example.

@fzaninotto
Copy link
Member

I'm not sure I understand the problem in the react-admin codebase. You're using <List> outside of the documented use cases and you're having trouble with it. That's your problem I think.

Now one could argue that the proptype warning you're seeing shouldn't be logged by react-admin, because hasCreate isn't actually required. We can easily fix that "bug".

Also, you're trying to create a List for another resource than the current one. We already provide ReferenceManyField for displaying related lists. Could you explain your use case?

@jeremydrichardson
Copy link

Well, I guess I might not be understanding the use case for ResourceContextProvider. My implementation is based off the article in the February 2021 Update blog titled "Use Everywhere You Want".

The example given looks like:

const TagsEdit = (props) => (
    <>
        <Edit {...props}>
            // ...
        </Edit>
        <ResourceContextProvider resource="posts">
            <ListBase basePath="/posts" filter={{ tags: [id]}}>
                <Datagrid>
                    <TextField source="title" />
                </Datagrid>
            </ListBase>
        </ResourceContextProvider>
    </>
)

Based on that and the description in the blog, it sounds like your should be able to use anywhere with any resource regardless of what the "current" resource is as long as your wrap it in ResourceContextProvider with a prop naming the resource.

It seems to work with ListBase fine with no errors, but List throws the console errors of props missing.

Use Case 1

We have the resource projects and tasks. Tasks are children of projects. When you go an individual project resource, we want to list the related tasks below in a .

I'll look at ReferenceManyField to see if that might work in this instance.

Use Case 2

What about a dashboard page where it is a custom route and we are displaying a number of different resources (top 5 tasks and projects). We were thinking ResourceContextProvider would be perfect in that instance. A lot less code and simpler to implement than using useQueryWithStore for each resource.

@andrewlorenz
Copy link
Author

andrewlorenz commented Mar 8, 2021

hey @jeremydrichardson I'm not sure whether I was aware of that specific blog post as I look at so many every day (!) but you are absolutely right that this article lays out exactly the use case I am trying to implement. In my specific case I need to provide a non-admin end user with particular view of the data in a list form based on a resource (their "orders" in the system), and so I just need what is effectively a stand-alone page that can render a filtered DataGrid of records from the orders resource. So I was specifically following the snippet from the react-admin documentation directly here: https://marmelab.com/react-admin/List.html - though this is addressing the option to have multiple lists on a single page but from a different resource (posts inside tags). And yes, exactly as you say, the List component shown there doesn't work, whereas it would appear from your test that ListBase does - so I am going to try using that now.

In the meantime this was my hack, which works fine actually:

      <List
        title="Order Inbox"
        filter={{ ord_status: { $in : [ 'E','Q', 'S' ] }}}
        filters={<OrderFilter />}
        actions={<ExportButton exporter={orderExporter} />}
        pagination={<CustomAdmin.Pagination />}
        empty={<EmptyOrders />}
        resource="orders"
        basePath="/orders"
        hasCreate={false}
        hasEdit={false}
        hasList={false}
        hasShow={false}
      >
        <Datagrid>
          etc..

It makes me think the bug here may be that whilst ResourceContextProvider should work "anywhere", it's actually only working inside an existing Resource context, which happens to inject the necessary props to the inner List such that it works. Which may not be a bug of course, but the documentation isn't clear. thanks

@jeremydrichardson
Copy link

jeremydrichardson commented Mar 8, 2021

Right. I noted that also that I could stop the errors by adding the necessary props as true or false. But that is where I was trying to understand where those props normally come from.

ResourceContextProvider component code

From what I can tell, the ResourceContextProvider is just wrapping its children with the ResourceContext.Provider (makes sense...). https://github.com/marmelab/react-admin/blob/master/packages/ra-core/src/core/ResourceContextProvider.tsx

Then it is passing in the name of the resource into the ResourceContext.

What's not clear is how this process differs from a page resource as it appears they are using the same components. Only conclusion I can come to is that the hasCreate, hasEdit, hasList, and hasShow don't come from the resource context but from the page level. Haven't quite tracked down how this works though.

Resource component code

I see that those props (hasCreate, hasEdit, hasList, hasShow) are being set with a dispatch. I'm not super familiar with redux but it seems like those props are actually stored in the resource record in redux. I still don't understand how those props are being added to the List component though.

@fzaninotto
Copy link
Member

Thanks for the context and the explanation. There is indeed a bug about required props warnings.

@crates
Copy link

crates commented Sep 20, 2021

More documentation on this, and things like SaveContext and EditBase and stuff like that, is super necessary. This issue shouldn't have been closed. There are still mountains of new features that remain undocumented on the main docs.

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

Successfully merging a pull request may close this issue.

5 participants