Skip to content
This repository has been archived by the owner on Jan 27, 2021. It is now read-only.

Fixed issue when root subspace was composed with redux dev tools #47

Merged
merged 8 commits into from
Sep 6, 2017

Conversation

mpeyper
Copy link
Contributor

@mpeyper mpeyper commented Sep 3, 2017

There is a pretty severe bug with the current alpha version when composing applyMiddleware with the redux-devtools-extension that causes an infinite loop when getState was called.

I'm not sure why it only occured when using the extension, and not when using the dev tools directly, like in the real world example, but this change works for both, and when not using dev tools at all.

The change looks more complicated than it really is. Basically, if the subspace is the root store, don't map the state, or namespace the actions (it was defaulting to subspace((state) => state, undefined) anyway).

To achieve this I also had to fix a minor issue when subspaces not created from applyMiddleware thought they were the root store if applyMiddleware was not used at all. Most of the noise in this review is caused by fixing that, rather than the actual bug.

@mpeyper mpeyper added the bug label Sep 3, 2017
@@ -16,9 +17,9 @@ const verifyState = (state) => {
return state
}

const subspaceEnhancer = (mapState, namespace) => applySubspaceMiddleware((store) => ({
const subspaceEnhancer = (mapState, namespace) => applySubspaceMiddleware(applyToChildren((store) => ({
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm still not clear on why this was required.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, I don't know why it occurs when the dev-tools-extension is used, but the store.rootStore.getState() call on the next line ends up back in this function and infinitely loops calling getState on the root store. This just enforces the a root subspace will never use this function.

@jpeyper jpeyper merged commit b2b06d2 into ioof-holdings:master Sep 6, 2017
@mpeyper mpeyper mentioned this pull request Dec 30, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants