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

Proposal: add meta to EditProps #7665

Closed
wants to merge 2 commits into from
Closed

Conversation

RWOverdijk
Copy link
Contributor

Hey!

I currently have the following changes in my own project, and I've had them for a while now. This is an example PR to share the diff.

I noticed that there was no nice way for me to pass meta through to the data provider without creating a lot of conditionals in the data provider.

An example of how I use this:

export function TopicEdit(props: any) {
  return (
    <Edit meta={{ embed: 'media' }} {...props}>
      <TopicForm />
    </Edit>
  );
}

The change is relatively minor since everything after the controller already accepts meta.

Thoughts?

@fzaninotto
Copy link
Member

This is definitely something we'll want to add. The question is: should we add a new prop for that, or use the queryOptions? Especially in controllers that do more than one query (e.g. the <Edit> component), developers may want to specify meta for both the query and the mutation.

Note that for a PR to be accepted, you should add tests and documentation.

@RWOverdijk
Copy link
Contributor Author

RWOverdijk commented May 9, 2022

@fzaninotto I thought about that, but my understanding was that queryOptions goes straight to react query (I think that's the name, don't quote me on that).

About the pr being accepted, I wasn't expecting it to be straight away (I know the high standards you have for this repo, so I wouldn't dare haha).

But a similar solution would be cool. What I liked about using meta is that it's meta so you can put in whatever you want, for example:

const meta = { findOne: { meta: 'for find one' }, update: { meta: 'for update' } };

This idea might be a bit shortsighted of me, but I think (hope) the concept is clear.

@fzaninotto
Copy link
Member

it's meta so you can put in whatever you want, for example:

const meta = { findOne: { meta: 'for find one' }, update: { meta: 'for update' } };

Not a fan, especially as it introduces a new way to target either the query or the mutation.

I agree that the queryOptions currently go straight to react-query, so the change would require some plumbing. But I can't find a better solution than to put the meta in it.

@panfiva
Copy link

panfiva commented Jun 15, 2022

Maybe meta attributes should be stored in 'Meta' context that is a parent of list, show, create, and edit contexts and the context should expose both value and value update function. This way, we will be able to update meta attributes from within each component based on data returned

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.

3 participants