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

Add options to meta #13

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

porfirioribeiro
Copy link

In adiction to pass second parameter info to action, i think it's also good to pass the options object itself in meta.

Why:

Many times in my reducers i need some options to store the payload of my action.
So i pass those variables in the second parameter and get them as action.meta.info
But most times i am already sending that option as the main options object like:

dispatch(someAction({ userId },{ userId }))

Then in reducer i use action.meta.info.userId to set that user as loading on start and set the data on complete. with this PR i can just do

dispatch(someAction({ userId }))

And in the reducer use action.meta.options.userId

@mikestead
Copy link
Owner

Thanks for the pull request. I can see what you're looking for but not sure it's really going to work in some situations.

options are only for openapi params which are not marked as required. All required params get passed through as direct arguments to the action creator. So this change won't contain all the values you pass to an endpoint.

Also an endpoint may not have any optional params, in this case the updated code gen will be invalid I believe.

@porfirioribeiro
Copy link
Author

Ohh i see, probably the schema i am using is not well done because i get all params in options.

Need to figure out other way...

But tell me, how would i dispatch actions with required params?

Like this?

dispatch(action(required1, required2, requiredN..., { optional1, optional2, optionalN... }, { info1, info2, infoN... }));

@mikestead
Copy link
Owner

Yeah that's correct. Generally there will not be lots of these and the aim is to gen functions like you would write them, making the func signature readable.

@porfirioribeiro
Copy link
Author

Ok, i understand.

The only possible solution was to pass all params to meta, but i don't know if that can bring any problem.
Because also if i want to do optimistic updates i would like to get the body i posted, without having to pass it again in the info argument.

Probably the best option is not generate redux actions at all and make them myself, calling the openApi functions . I would just like to avoid some extra repetitive work

@mikestead
Copy link
Owner

mikestead commented Jun 29, 2017

Unfortunately I can't really see an easy way to implement what you're after, other than formulating a params object with the required and optional ones to put into meta. I'm not totally sold on that, but open to more feedback.

If we were to bundle all values into an options object there would be no easy way of knowing which were required, unless using TypeScript, which is why I went with the other approach.

@porfirioribeiro
Copy link
Author

I get your point. But it's also strange to have some params in arguments and others in options.
Also, in options i can pass parameters by its name in the object, it seems more clear to me to understand where each param is and it's value.

doSomething('Porfirio' ,{lastName: 'Ribeiro'})
doSomething({
  firstName: 'Porfirio',
  lastName: 'Ribeiro'
})

Of course it's not easy to me to know which value is required, in Typescript is easier, but with JSDoc you might be able to accomplish it

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.

2 participants