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

Passing in arguments to store from async actions is painful to unpack #191

Open
JAStanton opened this issue May 14, 2015 · 15 comments
Open

Comments

@JAStanton
Copy link

Here is my psuedo code example of how I have to unpack my arguments passed to the store method callbacks for async actions:
https://gist.github.com/JAStanton/a89844d150ef5b0f66b9

I feel like I'm doing something wrong, why do I have to dive into actionArgs for the arguments specifically in handleFetching / Begin callback?

(This is psuedo code)

@acdlite
Copy link
Owner

acdlite commented May 14, 2015

Hi! Regarding rejecting multiple values, that's how the promise API is designed. You can use destructuring in the params list to make this less of a burden.

Regarding passing action args to the begin handler, that was a poor API design on my part which will be fixed in 4.0 by passing the entire payload (including action args) to every handler as the final parameter (again, you can use destructuring to make values easier to access).

I'm on my phone now but I'll post a code example later. In the meantime you can check out the 4.0 branch's changelog, and try out 4.0-alpha1 on npm.

@acdlite acdlite closed this as completed May 14, 2015
@JAStanton
Copy link
Author

Wow thanks for the fantastic response time :) I appreciate your input on this. Do you know if people are using 4.0-alpha1? Would you advise against using it?

@acdlite
Copy link
Owner

acdlite commented May 14, 2015

I haven't encountered any bugs personally — it's still in alpha because there may be API changes. Also React 0.14 will have a new API for side-loading data which we will update to.

On Thu, May 14, 2015 at 1:04 PM, Jonathan Stanton
[email protected] wrote:

Wow thanks for the fantastic response time :) I appreciate your input on this. Do you know if people are using 4.0-alpha1? Would you advise against it?

Reply to this email directly or view it on GitHub:
#191 (comment)

@JAStanton
Copy link
Author

👍 you're amazing thanks :)

@nevir
Copy link

nevir commented May 14, 2015

Turns out we're actually on 4.0.0-alpha2!

It looks like you're accidentially publishing your alpha versions with the latest tag (npm publish is dumb that way) - your alpha releases are being tagged for general use :p

You might want to set latest back to a stable release:

npm tag [email protected] latest

And when publishing an alpha build:

npm publish --tag 4.0-alpha

(as long as you have some tag, it won't default to latest)

@JAStanton
Copy link
Author

Yeah so given the fact that we are using 4.0.0-alpha2 I am still having this issue. :(

@acdlite
Copy link
Owner

acdlite commented May 14, 2015

Gah I'll fix that ASAP.

So what's the issue you're still having? You don't like having to go deep into the payload to access the action args? I'm open to suggestions for how you'd like to make that better. Destructuring removes the pain of this for me, personally.

@acdlite acdlite reopened this May 14, 2015
@JAStanton
Copy link
Author

Destructuring is an option sure. promise reject we can leave because it's just goofy. but It would be nice if the arguments passed into the action were passed into the begin handler like so:

// my action method takes 3 args:
doAction(a, b, c) {}
// my store handle method takes those same 3 args:
handleDoActionBegin(a, b, c){}

@JAStanton
Copy link
Author

instead of:

// my action method takes 3 args:
doAction(a, b, c) {}
// my store handle method takes an argument with the arguments listen in actionArgs
handleDoActionBegin(args) {
 [a,b,c] = args.actionArgs;
}

@acdlite
Copy link
Owner

acdlite commented May 14, 2015

We used to do it that way but we also need to accommodate additional info such as the dispatch id (to make optimistic updates easier — the begin and success/error handlers receive the same dispatch id per promise) and possibly other data in the future. So for consistency's sake this is the solution we came up with.

@acdlite
Copy link
Owner

acdlite commented May 14, 2015

Did you know you could do this?

handleAction({ actionArgs: [ a, b, c] }) {
  // use a, b, c as expected
}

@acdlite
Copy link
Owner

acdlite commented May 14, 2015

Btw that was really hard to type on my phone :D

@JAStanton
Copy link
Author

wow, that's.... wow. also kudos or typing that in on your phone ;)

also I'm super curious about optimistic updates using dispatch id, do you have any examples of someone doing that? We're working on our own solution right now which is to have the view generate local Id's and when the view gets updates it will check the array of things for the object with the local Id and do stuff based on state we inject in that object.

@JAStanton
Copy link
Author

Also in the begin handler arguments example can we have your metadata argument be the first argument and all other arguments passed from action args passed in after that?

handleActionBegin(metaData, a, b, c)
// this would also be fine I guess? 
handleActionBegin(a, b, c, metadata)

I actually don't care if it's before or after, I would just really like to have it so I don't have to unpack the arguments myself.

@jordaaash
Copy link

@acdlite:

Regarding passing action args to the begin handler, that was a poor API design on my part which will be fixed in 4.0 by passing the entire payload (including action args) to every handler as the final parameter (again, you can use destructuring to make values easier to access).

Is this the same as the handleActionBegin(a, b, c, metadata) API proposed by @JAStanton?

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

No branches or pull requests

6 participants