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

digestMiddleware is swallowing the result of the next action #92

Closed
sinn1 opened this issue Jul 1, 2016 · 8 comments
Closed

digestMiddleware is swallowing the result of the next action #92

sinn1 opened this issue Jul 1, 2016 · 8 comments

Comments

@sinn1
Copy link

sinn1 commented Jul 1, 2016

In our particular use case, we use a clientMiddleware to handle API requests which returns a promise. Based on the result of this promise, we may or may not dispatch further actions:

// snippet from clientMiddleware.js return promise(ApiClient).then( (result) => next({...rest, result, type: SUCCESS, receivedAt: Date.now()}), (error) => next({...rest, error, type: FAILURE}) ).catch((error) => { console.error('MIDDLEWARE ERROR:', error); next({...rest, error, type: FAILURE}); });

// usage this.fetchSomethingFromServer().then((success) => this.fetchSomethingElse());

digestMiddleware.js calls:

$rootScope.$evalAsync(next(action);

but doesn't return the result of next(action), so the promise is not returned and can't be chained.

@pgavazzi
Copy link

pgavazzi commented Aug 18, 2016

I have the same issue, this is supposed to work! Have you got this to work by simply return $rootScope.$evalAsync(next(action);?

@c-dante
Copy link
Contributor

c-dante commented Jan 5, 2017

I think the correct fix would be:

const res = next(action);
$rootScope.$evalAsync(res);
return res;

At least with that things like thunk chains and other middleware can work with the dispatch chain.

@KasaiMagi
Copy link

Maybe I'm missing something obvious and/or this isnt the right place for this question,but why is next(action) passed into $evalAsync in the first place?

It should eval to something other middleware would return, but without extra middleware this will be an action object. Calling $evalAsync on a basic action object will do nothing different than calling $evalAsync without an argument, right?

Doesnt that mean this digestMiddleware is useful just to cause an async digest regardless of the param? Is this only helpful if we ever want to make custom middleware that returns an angular expression?

@c-dante
Copy link
Contributor

c-dante commented Feb 2, 2017

It really doesn't matter what you pass into the $evalAsync -- what's happening is this is taking advantage of how Angular 1.x deals with an $evalAsync, which is to trigger a digest if one is NOT currently happening (if a digest is happening, then redux can't be reduxing).

The real issue here is that the middleware is not returning the result of the middleware chain (the return of next(action)). Because of this, the library cannot position well in certain middleware chains.

@KasaiMagi
Copy link

KasaiMagi commented Feb 2, 2017

@c-dante right! And I'd love your PR to be merged too. But I want to make sure the middleware shouldnt instead invoke next(action) within the expression passed to $evalAsync - thereby executing the middleware chain according to when $evalAsync says it should. For example:
$rootScope.$evalAsync(() => next(action));

If this was required then your proposed return wouldnt be possible. As it stands now the middleware chain will continue regardless of what $evalAsync does, right?

As it works now, this middleware tells redux to continue and at some point in the future an angular digest is triggered - it doesnt care if redux processes the action now or later on the async queue.

@c-dante
Copy link
Contributor

c-dante commented Feb 2, 2017

So there are a 2 problems with this:

  1. If angular hangs, your entire store is frozen

  2. Redux would need to bake in async to the middleware enhancement -- you still need to return something from this middleware function. Even if you run the chain once angular's digest propagates, you're back to being stuck consuming the result of the middleware chain.

In that example, the middleware still returns undefined and consumes the chain.

Really, just triggering the digest is the important part. Getting the correct state in angular happens automatically by using the $ngRedux.connect

@TobyColeman
Copy link

Any idea when a fix for this might be released?

@AntJanus
Copy link
Collaborator

AntJanus commented Jan 8, 2018

@TobyColeman it's been merged in and released.

@AntJanus AntJanus closed this as completed Jan 8, 2018
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

No branches or pull requests

6 participants