RetailMeNotSandbox / redux-mount-store

Redux store enhancer that makes it possible to mount sub-stores
MIT License
2 stars 3 forks source link

Changes to own state of mounted stores should invalidate cache of ancestors #10

Closed lingram-rmn closed 8 years ago

lingram-rmn commented 8 years ago

When a descendant of a mounted store updates its own state, we don't invalidate the cache of its ancestor mounted stores. This results in stale state appearing in the ancestor's own state.

Repro

'use strict';

const redux = require('redux');
const reduxMountStore = require('./');
const assert = require('assert');
const immutable = require('object-path-immutable');

const store = redux.createStore(
    state => state,
    {},
    reduxMountStore
);

const hostStore = store.mount('host')(
    state => state,
    {}
);

const childStore = hostStore.mount('child')(
    (state, action) => {
        if (action.type === 'MUTATE_CHILD') {
            state = immutable.set(state, 'childOwn', 'new child');
        }

        return state;
    },
    {
        childOwn: 'child'
    }
);

childStore.dispatch({
    type: 'MUTATE_CHILD'
});

assert.deepEqual(store.getState(), {
    host: {
        child: {
            childOwn: 'new child'
        }
    }
});

assert.deepEqual(hostStore.getState(), {
    child: {
        childOwn: 'new child'
    }
});

This script can be run from the root of the repo to reproduce the bug. The second assertion will fail. Note that the first assertion is fine, because the changes are included in the new state we return from the root reducer.

lawnsea commented 8 years ago

FYI @lzilioli

lawnsea commented 8 years ago

I see two possible solutions:

  1. Keep track of which paths have updated their own state and recompute the cached state of their ancestors
  2. Do a depth-first traversal of the paths (instead of the current breadth-first traversal) and update the cached state after recursing into descendants

The latter is probably the cleaner solution, but requires changes to getViewedState.