Add createReducer() with init-state-as-a-lazy-getter overload

[Jazzmanpw]

In my project, I have a piece of state that may be preloaded from the local storage:

import { loadState } from './storage';

export default createReducer(loadState(), builder => ...);

The issue comes when I try to test this reducer because I need to isolate modules (I use Jest, but it may be relevant for other testing libraries, I believe). Without RTK it would be possible to write

function reducer(state = loadState(), builder) { ... }

and the loadState() would be called every time the reducer is called with undefined as the first argument, but using RTK is much more convenient overall.

So is it possible to add createState<S>(initState: () => S, ...) overload so that a lazy getter could be passed?

[markerikson]

Hmm. A few thoughts:

• Seems like a mostly reasonable request in general
• I could see this requiring some complexity with the types and overloads
• That would also have to carry through to createSlice
• On the other hand: testing reducers is usually just const actual = reducer(testState, action), in which case it doesn't matter what the initial state was. What's the issue with testing in regards to the initial state here?

[Jazzmanpw]

Since the variables declared in the module can't be changed after it was imported, I can't just mock loadState() with two different values and call the reducer with undefined test state after each mock. So I need to write some complex code like this:

/* ./reducer.js */
import { loadState } from './storage';
export default createReducer(loadState() || '', builder => ...);

/* ./reducer.test.js */
import reducer from './reducer'
jest.mock('./storage', () => ({ loadState: jest.fn() });

// isolated imports with mocked value in storage
const preloadedState = 'some state';
let isolatedReducerWithLoadedState;
jest.isolateModules(() => {
  require('./storage').loadState.mockReturnValue(preloadedState);
  isolatedReducerWithTruthyInitState = require('./reducer').default;
});

// plain imports assuming there's no value stored
const reducer = require('./reducer').default;

describe('initState', () => {
  test('no preloaded state', () => {
    expect(reducer(undefined, {})).toBe('');
  });

  test('preloaded state', () => {
    expect(isolatedReducerWithLoadedState(undefined, {}).toBe(preloadedState);
  });
});

Partially it is a problem of jest, but they resolved the issue, and I missed it somehow, so the code will become a bit simpler. On the other hand, it will be even cleaner if I don't need to use isolateModules() at all.

Maybe this problem has a much better solution, but this is what I use now

[markerikson]

Hmm. My question is, why is it even necessary to test that the reducer returns the initial state? That's basically testing our library code implementation, rather than your app logic.

[Jazzmanpw]

In fact, the initial state in my app is not that simple and depends on configuration, etc. So it's more about testing if the code combines data from the storage right.

[markerikson]

Okay. Going back to the actual proposed change: how does passing a function actually help in this case? It's going to be the same function all the time, even if it's being called later. Won't you have the same re-import / mocking issue regardless?

[Jazzmanpw]

I see it like this:

/* ./reducer.js */
import { loadState, loadAnotherState } from './storage';

function getInitState() {
  const state = loadState();
  const anotherState = loadAnotherState();
  const combined = /* some complex combination */
  return combined;
}

export default createReducer(getInitState, builder => ...);

/* ./reducer.test.js */
import { loadState, loadAnotherState } from './storage';
import { reducer } from './reducer';

describe('initState', () => {
  test('no preloaded state', () => {
    expect(reducer(undefined, {})).toBe('');
  });

  test('preloaded state', () => {
    loadState.mockReturnValue('value');
    loadAnotherState.mockReturnValue('another value');

    expect(isolatedReducerWithLoadedState(undefined, {}).toBe(properlyCombinedState);

    loadState.mockReset();
    loadAnotherState.mockReset();
  });
});

And that's it. no isolation, no module reinitialization. The initial state will be recomputed every time I call the reducer.

Since the combination logic is not exported, I can't test it directly. But I want to check if it works as expected.

[phryneas]

You are jumping a lot of hooks to write a unit test for your combination logic. Why not just add an export keyword there? Maybe mark it as /** @internal */ and that's that.

[Jazzmanpw]

Because it doesn't feel right. Export just for testing smells a lot for me. I'm a kinda test noob and probably don't get how things should be done, so I follow intuition and some aesthetics considerations.

[phryneas]

Testing a function by calling it implicitly through another function doesn't look much better to me 😕

[Jazzmanpw]

What will be the initial state if the local storage data happens to be this

Isn't it better?

[markerikson]

So ironically I actually did run into this particular use case myself not too long ago - a slice where we're persisting the data from localStorage and loading it in the slice file, and it did make testing kind of a pain.

I think my biggest question would be much more complex this might make the types for createSlice. Lenz, any thoughts?

[phryneas]

Type-wise it would not be a problem, but we should be very clear on what it does.

Does it execute once when undefined is passed in? Every time?

Tbh I was also pondering to expose slice.initalState so people can write it inline and not have to export it, for use like useReducer(slice.reducer, slice.initialState) - but those would probably kinda conflict then.

[markerikson]

Suggestion from Lenz in chat:

• allow initialState to also be () => State
• add slice.getInitialState() which either just returns the initial state or executes the lazy initializer
  That should be very little type change and that second change would be a good quality of life improvement for useReducer users.

useReducer(slice.reducer, undefined, slice.getInitialState)

Nice side effect: we can tunnel initialState through produce once, so it returns a frozen object. If people define initialState as a constant, there is always a risk they manipulate that accidentally

[markerikson]

Available as of https://github.com/reduxjs/redux-toolkit/releases/tag/v1.7.0-beta.0 .