Use a value set on the state itself rather than an isReady flag

[clayne11]

When redux initializes it passes a couple of actions to each reducer. This initialization was setting the "isReady" flag. If we have an initial state coming from outside of the reducer, however, then on the actual @@INIT pass the isReady flag would be true and the state be defined even though it isn't actually initialized.

By using a flag on the state itself we can ensure that the state is properly initialized.

Fixes #16

[coveralls]

Coverage remained the same at 84.615% when pulling e1d7c92e7e1793d29da2d50c113707eccfd323cc on clayne11:ssr into e443f0900ceaede36691d799efc6dfd12b048c86 on mattkrick:master.

[mattkrick]

awesome, thanks for the PR! To make sure there are no regressions (and so I can more fully understand the problem) could you change the new test to be something that would fail under the current version? That way, changing how we achieve the goal shouldn't affect the desired result.

[clayne11]

No problem - I added a test that fails on the master branch but passes on the branch with my fix.

I had to add redux as a devDependency to show the issue.

[coveralls]

Coverage remained the same at 84.615% when pulling f4cb2f471dfa58e83c5b3ec92ded6031167f33dd on clayne11:ssr into e443f0900ceaede36691d799efc6dfd12b048c86 on mattkrick:master.

[coveralls]

Coverage remained the same at 84.615% when pulling f4cb2f471dfa58e83c5b3ec92ded6031167f33dd on clayne11:ssr into e443f0900ceaede36691d799efc6dfd12b048c86 on mattkrick:master.

[coveralls]

Coverage increased (+7.7%) to 92.308% when pulling 6fef62bd33f9ec7085a24bb162ebf869fea82099 on clayne11:ssr into e443f0900ceaede36691d799efc6dfd12b048c86 on mattkrick:master.

[coveralls]

Coverage increased (+7.7%) to 92.308% when pulling 6fef62bd33f9ec7085a24bb162ebf869fea82099 on clayne11:ssr into e443f0900ceaede36691d799efc6dfd12b048c86 on mattkrick:master.

[mattkrick]

Ohhhh now I see what's up, thank you! I'm still not in love with the implementation, because then we're keeping a 1-way flag in our state the whole time. This should be private state because it's a part of the implementation & has no useful info for the user. As is, this is required because we'd create a state, then override that state with the preloadedState, then override that preloadedState with an optimistic version of itself. Instead, what if you just did this?

const dataToLoad = {counter: 0};
const preloadedState = Map({
  beforeState: undefined,
  history: List(),
  current: dataToLoad
});
const store = createStore(enhancedReducer, preloadedState);

I wouldn't be opposed to a helper function that did this for you, so we could write: const store = createStore(enhancedReducer, preloadState({count: 0}));

[clayne11]

The issue for me is that I'm only using optimistic on a sub-reducer, not on the root reducer. It's not trivial to preload a specific nested reducer with without hardcoding a relationship between the instantiation of the store and specifically which reducer is optimistic.

I suppose I could preload the state the way you're suggesting, though. Neither solution seems ideal to me.

[mattkrick]

Agreed, it's not great. However, if I have to make a decision to dirty up package logic or dirty up application logic, I'll dirty that application every time. Since your state tree is deterministic, it wouldn't be hard to write something like this:

const dehydratedState = {
  foo: 0,
  bar: {
    baz: 0
  }
};

const optimisticParts = ['foo', 'bar.baz'];

const optimistify = (dehydratedState, optimisticParts) => {
  optimisticParts.forEach(subState => {
    const depths = subState.split('.');
    depths.reduce((left, right, idx) => {
      if (idx < depths.length -1) {
        return left[right];
      } else {
        left[right] = preloadState(left[right]);
        return left[right];
      }
    }, dehydratedState)
  })
}

By declaratively scoping your optimistic parts, you make your code really easy to grok for future you, since the enhancer itself will probably get applied in your async router so you never really get the full picture.

[clayne11]

Fair enough. I'll change the PR to be for a helper function to preload state.

[clayne11]

Added a new PR https://github.com/mattkrick/redux-optimistic-ui/pull/18