Create store bug

[lilasquared]

While I was looking at the thread safety test I came across a bug. If you add the below it will fail.

public void Should_Dispatch_Once_On_Store_Creation() {
    var sut = new Store<int>((state, action) => state + 1, 0);
    var mockObserver = new MockObserver<int>();

    sut.Subscribe(mockObserver);

    CollectionAssert.AreEqual(new[] { 1 }, mockObserver.Values);
}

It could be how it was intended to be designed but redux is implemented with an initial action that is dispatched to call all reducers once when the store is created. below is a similar test for redux and when added to the test suite it fails.

  it('dispatches once on store creation', () => {
    const store = createStore((state, action) => state + 1, 0)
    expect(store.getState()).toEqual(0)
  })

The result being that the current state is 1 and not 0 because of the initial dispatch.

interesting discussion about this in the redux issues

[lilasquared]

I should add that I only call this a bug because it differs from the current implementation. The behavior that redux.net has makes more sense than redux IMO and there is interesting discussion around it in the link.

[GuillaumeSalles]

Sorry to answer this late. I gave a hint about this in another issue :

Redux.js uses a @@redux/INIT action to create the initial state instead. I think that's pretty smart because if the reducer is composed by smaller reducers, it become the responsibility of the smaller reducers to expose a valid initial state. It easier to refactor the state shape.

What do you think ?

If you think it's a good feature to add, PR is welcome :)

[cmeeren]

Are you suggesting that we require the reducer(s) to specify the initial state? How exactly would this work in practice?

[lilasquared]

@cmeeren I think the way it works for redux is that the smaller reducers can provide their initial state if one is not provided to them. You could call

const store = createStore(rootReducer)

without passing any initial state and each of the smaller reducers would supply its own. If you read through some of the PRs on redux they talk about renaming the second parameter of createStore from initialState to preLoadedState as it should be used to hydrate the store with a state from the server.

That being said I don't know if that is as necessary for C# because of the OOP style. Each state slice can have a constructor that initializes arrays or other reference types. in js reducers look like:

// todos reducer
function todos(state = [], action {
  switch(action.type) {
    case ADD_TODO:
      return [
          ...state,
          todo(null, action)
      ]
  }
}

// todo reducer
const initialState = {
  isCompleted: false
}

function todo(state = initialState, action) {
  switch(action.type) {
    case ADD_TODO:
      return {
          ...state,
          text: action.text
      }
  }
}

which simulates the constructor of a particular object that is used for the state. Since c# has the constructors it makes sense for the objects themselves to define their initial state.

Given that, I didn't know if that initial action was needed or not - I just saw the different behavior of the end result of the initial state of the store.

[cmeeren]

I don't think reducers should specify the initial state, because that would preclude them from using the "sentinel" value, e.g. null. That is, if the reducer gets previousState = null interprets this to mean "use the default state", then that precludes using null values for that state member.

Currently (not saying this is the best solution) I define the initial state when the app starts, in the DI composition root, where I have access to dependencies like app storage etc. The state is initialized through the different state constructors:

/// <summary>Provides the initial state of the app.</summary>
public class InitialStateProvider
{
    private readonly IPreferencesRepository preferences;

    public InitialStateProvider(IPreferencesRepository preferences)
    {
        this.preferences = preferences;
    }

    /// <summary>Returns the initial state of the app.</summary>
    public RootState GetInitialState()
    {
        var state = new RootState(
            authState: new AuthState(
                token: this.preferences.Token,
                tokenIsValid: false,
                isSigningIn: false,
                previousUsername: this.preferences.PreviousSignedInUsername),
            projectState: new ProjectState(
                projects: ImmutableList<Project>.Empty,
                selectedProjectId: null,
                isRefreshingProjects: false,
                isCheckingInOutProject: false));
        return state;
    }
}

[lilasquared]

@cmeeren I think we are saying the same thing, but Imagine though that you have 5 or 6 root states that are all composed of objects with 5 or 6 other state objects. You can see how this can grow exponentially and be quite a nightmare to maintain all in one place.

[GuillaumeSalles]

It seems you understand the issue correctly but just to illustrate :

public class State
{
    public AState A { get; }
    public BState B { get; }
}

public static AState ReduceAState(AState state, object action)
{
    if(state == null)
        return InitialAState();

    [...]
}

public static BState ReduceBState(BState state, object action)
{
    if(state == null)
        return InitialBState();

    [...]
}

public static State Reduce(State state, object action)
{
    return new State(
        ReduceAState(state.A,action),
        ReduceBState(state.B,action));
}

All the philosophy is explained here.

It's not as practical as js because of the optional parameter in first position.

@cmeeren You are right that it does not allow "sentinel" values. This pattern is recommended but not mandatory. The developer can still provide an initial state and ignore the "InitAction" will not trigger anything.

[cmeeren]

Oh, I see - it's only if the actual state objects are null that the default state (for that state object, e.g. AState) is returned. I have absolutely no problem with that, because AFAIK these object should never ever be null (while the actual "data" values, e.g. the leaves of the state tree, can be null).

To clarify - if your state tree is like this:

Root
   StateA
      StateAA
         bool isSigningIn
         string Username
      StateAB
         ...
   StateB
      ...

then StateA, StateAA, StateAB, and StateB should never be null, while e.g. StateAA.Username can be null. So, returning the initial state from the higher-level state reducers (RootReducer, StateAReducer, StateBReducer) should be no problem. The StateAReducer will return a default/initial StateAA and StateAB if they are null, but the StateAAReducer can perfectly well pass through a null value for e.g. Username.

Have I understood correctly?

[GuillaumeSalles]

You understood perfectly! :)