TypeScript: Store values should probably all infer as optional about zedux HOT 2 OPEN

Thanks for the fast and detailed response. This all sounds great, and thanks for giving us multiple solutions whilst we wait on a v2 implementation. Really appreciate it.

from zedux.

@cboon-jeff thanks for reporting! I hadn't considered this before, I'll have to see why this is never causing problems for us. I agree that this behavior is not sufficiently type-safe.

I think any solution here would be a breaking change. I can tell already that making all state fields undefinable would be too disruptive a change. I'm in favor of making all undefined values passed to setStateDeep a no-op.

This would be a breaking change in this scenario:

const store = injectStore<{ foo?: number }>({ foo: 1 });
expect(store.getState().foo).toBe(1)
store.setStateDeep({ foo: undefined })
expect(store.getState().foo).toBeUndefined() // passes currently, would fail after this change

Changing the behavior of setStateDeep shouldn't be too disruptive for existing users. setStateDeep only exists for convenience and already has limited functionality (namely it can't delete fields). Since it is a breaking change, it will have to wait for Zedux v2, which we're starting to roadmap out right now, no target date yet.

Workarounds

I'm fine recommending that you use setState for now. It's less convenient, but should give an appropriate TS error when trying to set a non-undefinable field to undefined.

Alternatively, if you want improved store methods now and don't mind maintaining some code for it, here's an undocumented pattern that we've started using in several places:

Extend the Store class and create your own injector that injects instances of it. Here's an example that overrides the base Store class's setStateDeep method and uses lodash's omitBy to remove undefined keys.

class CustomStore<State> extends Store<State> {
  setStateDeep(settable: Settable<RecursivePartial<State>, State>, meta?: any) {
    const newState =
      typeof settable === 'function'
        ? (settable as (state: State) => RecursivePartial<State>)(this.getState())
        : settable;

    const filteredState =
      newState && typeof newState === 'object'
        ? (_.omitBy(newState, _.isUndefined) as RecursivePartial<State>)
        : newState;

    return super.setStateDeep(filteredState, meta);
  }
}

export const injectCustomStore = <State>(initialState: State, subscribe = true) => {
  const store = injectMemo(() => new CustomStore<State>(null, initialState), []);
  const self = injectSelf();

  injectEffect(
    () => {
      if (!subscribe) return;

      const subscription = store.subscribe(() => self.invalidate());

      return () => subscription.unsubscribe();
    },
    [subscribe],
    { synchronous: true }
  );

  return store;
};

I will officially document this pattern soon. No changes are planned for this, though note that it has a known TS issue when the store's state is an array. I'll release the fix for that before documenting this pattern.

from zedux.