Skip to content

Adjust store creation to resolve issue with redux-first-router #166

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

Merged
merged 1 commit into from
Sep 25, 2017
Merged

Adjust store creation to resolve issue with redux-first-router #166

merged 1 commit into from
Sep 25, 2017

Conversation

rshackleton
Copy link
Contributor

There is an existing issue when using ng-redux with redux-first-router (RFR).

When first loading the page RFR dispatches an action for the matched route. Due to how ng-redux creates the store this action is not passed to the registered middleware. I tested creating the store separately and passing this into ng-redux using the method outlined at the end of #19 but this then caused the digest loop to not be updated with the new state (due to the digestMiddleware not being added to the same middleware chain).

These changes to store creation mean the store is created as per the redux documentation and the middleware is initialised at the same time as the other store enhancers, thus preventing the action being swallowed.

This is very much an edge case and this was a quick fix for us so it may not meet your guidelines for PRs.

@AntJanus
Copy link
Collaborator

@rshackleton Thanks for the work on this! I haven't run into the same issue you're talking about but can you explain to me how this fix works?

I'm having a hard time understanding how passing in a store that already has middleware built in, and then adding to that middleware chain.

I'll merge this in right after, I just want to make sure I understand it. Again, thanks for working on this 👍

@rshackleton
Copy link
Contributor Author

It seemed to be related to when the middleware and enhancers were registered and in what order they were composed.

Calling applyMiddleware(...)(finalCreateStore)(_reducer) meant the middleware weren't added before the redux-first-router's enhancer had dispatched the first action, whereas calling createStore(...) with the middleware and enhancers composed works.

It is worth me clarifying that I'm not providing an existing store anymore, I'm using ng-redux as per the documentation and this PR fixes the original issue I had. Providing an existing store had a separate issue related to the digest middleware.

@AntJanus
Copy link
Collaborator

👍 sounds good! Merging in.

@AntJanus AntJanus merged commit 645eae2 into angular-redux:master Sep 25, 2017
@maxlapides
Copy link

@rshackleton Wow thanks so much for this fix. Just spent a few hours digging deep into RFR and ng-redux trying to figure out what was going wrong. I ended up looking at the ng-redux code on Github, scratching my head cuz it looked totally correct, only to discover that that's because you just fixed the issue and the code I'm testing is the latest on npm! Anyway, thanks again, this is exactly the fix I needed at exactly the right time. And if it's helpful for anyone else, here's a workaround until this fix is pushed to npm:

$ngRedux.createStoreWith(
  combinedReducers,
  [],
  [
    rfrEnhancer,
    applyMiddleware(
      rfrMiddleware,
      myMiddleware1,
      myMiddleware2
    )
  ]
)

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