Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TypeScript definition for StoreEnhancer prevents state type extension #1648

Closed
fdecampredon opened this issue Apr 20, 2016 · 8 comments · Fixed by #2773
Closed

TypeScript definition for StoreEnhancer prevents state type extension #1648

fdecampredon opened this issue Apr 20, 2016 · 8 comments · Fixed by #2773

Comments

@fdecampredon
Copy link

With the current type definition of StoreEnhancer:

export type StoreEnhancer<S> = (next: StoreEnhancerStoreCreator<S>) => StoreEnhancerStoreCreator<S>;
export type GenericStoreEnhancer = <S>(next: StoreEnhancerStoreCreator<S>) => StoreEnhancerStoreCreator<S>;
export type StoreEnhancerStoreCreator<S> = (reducer: Reducer<S>, initialState: S) => Store<S>;

We cannot define an enhancer that change the type of the sate :

declare function reducerWithFoo<S>(reducer: Reducer<S>): Reducer<S & { foo: string }>;

function test<S>(next: StoreEnhancerStoreCreator<S>): StoreEnhancerStoreCreator<S & { foo: string }> {
  return (reducer: Reducer<S>, initialSate: S & { foo: string }) => next(reducerWithFoo(reducer), initialSate);
}

Here we try to create an enhancer that add a foo field to the sate, but typescript errors saying (in short) that S is not compatible with S & { foo: string }.
This is due to the fact that the next function passed as arguments is not generic and so cannot capture the type from arguments.

Perhaps the solution would be to add a generic version of the StoreEnhancerStoreCreator type, and to change the signature of StoreEnhancer to use that type for the next argument.

definitions :

export type StoreEnhancer<S> = (next: GenericStoreEnhancerCreator) => StoreEnhancerStoreCreator<S>;
export type GenericStoreEnhancer = <S>(next: GenericStoreEnhancerCreator) => StoreEnhancerStoreCreator<S>;
export type StoreEnhancerStoreCreator<S> = (reducer: Reducer<S>, initialState: S) => Store<S>;
export type GenericStoreEnhancerCreator = <S>(reducer: Reducer<S>, initialState: S) => Store<S>;

example:

declare function reducerWithFoo<S>(reducer: Reducer<S>): Reducer<S & { foo: string }>;

function test<S>(next: GenericStoreEnhancerCreator): StoreEnhancerStoreCreator<S & { foo: string }> {
  return (reducer: Reducer<S>, initialSate: S & { foo: string }) => next(reducerWithFoo(reducer), initialSate);
} // no error
@gaearon
Copy link
Contributor

gaearon commented May 2, 2016

cc @aikoven @Igorbek @use-strict

@Igorbek
Copy link
Contributor

Igorbek commented May 4, 2016

We could probably add some signatures that would support state type alteration. I'm on vacation in trip now and if noone has done it before I return, I'll make a change then.

@aikoven
Copy link
Collaborator

aikoven commented May 9, 2016

@fdecampredon Unfortunately I don't have much time right now, can you please make a PR?

@aikoven
Copy link
Collaborator

aikoven commented May 17, 2016

@fdecampredon I took a closer look and got a bit confused.

Suppose we have a reducer enhancer, as you said:

function wrapReducer<S>(reducer: Reducer<S>): Reducer<S & {foo: string}>;

I assume that what we want to achieve is to get createStore function that accepts parameters with S and returns Store<S & {foo: string}>:

function createStore<S>(reducer: Reducer<S>, initialState: S): Store<S & {foo: string}>;

This already differs from what you wrote, note initialState type. It seems wrong to me to allow Store Creators that accept different state types for reducer and initialState.

So I assume you meant something like this:

function wrapState<S>(state: S): S & {foo: string};

function enhancer<S>(next: StoreEnhancerStoreCreator<S & {foo: string}>) {
  return (reducer: Reducer<S>, initialState: S) => 
    next(wrapReducer(reducer), wrapState(initialState));
}

The return type of enhancer will match the type of createStore function above.

Building new types

Let's try to rewrite our typings to allow such Store Creators:

type StoreEnhancerStoreCreator<S, R> = 
  (reducer: Reducer<S>, initialState: S) => Store<R>;

Pull this new type to the StoreCreator interface:

interface StoreCreator {
  // ...
  <S, R>(reducer: Reducer<S>, initialState: S, 
         enhancer: (createStore: StoreEnhancerStoreCreator<R, R>) =>
           StoreEnhancerStoreCreator<S, R>
  ): Store<R>;

Note that createStore argument of enhancer has equal input and output state type. This is because enhancer is applied to the default createStore() function which doesn't change state type.

Let's extract enhancer type:

type StoreCreatorEnhancer<S, R> =
  (createStore: StoreEnhancerStoreCreator<R, R>) =>
    StoreEnhancerStoreCreator<S, R>;

interface StoreCreator {
  // ...
  <S, R>(reducer: Reducer<S>, initialState: S, 
         enhancer: StoreCreatorEnhancer<S, R>): Store<R>;

Note that our new StoreCreatorEnhancer is not really usable outside of Redux typings: it assumes concrete state types while real enhancers are usually unaware of state type. It also only accepts Store Creators that don't change state type.

Generic Enhancers

The most generic version of StoreEnhancer would be:

type StoreEnhancer =
  (createStore: StoreEnhancerStoreCreator<any, any>) =>
    StoreEnhancerStoreCreator<any, any>;

i.e. no restriction on input/output types.

Middleware Enhancers

applyMiddleware() returns more strict version because it doesn't change state types:

type MiddlewareEnhancer =
  <S, R>(createStore: StoreEnhancerStoreCreator<S, R>) =>
    StoreEnhancerStoreCreator<S, R>;

Experimenting

After doing some experimentation I found out that TypeScript compiler is not smart enough to infer some types:

type State = {foo: string};

const reducer: Reducer<State>;

const storeWithThunkMiddleware: Store<State> = createStore(
  reducer,
  applyMiddleware(thunkMiddleware)
);

gives following error:

Uncaught Semantic: Type 'Store<{}>' is not assignable to type 'Store<{ foo: string; }>'.
  Type '{}' is not assignable to type '{ foo: string; }'.
    Property 'foo' is missing in type '{}'.

To fix this I had to explicitly declare type parameters for createStore:

const storeWithThunkMiddleware: Store<State> = createStore<State, State>(
  reducer,
  applyMiddleware(thunkMiddleware)
);

That's not very convenient considering that enhancers usually don't alter state type. So I added two more types for such enhancers:

type StoreEnhancerStoreCreator<S> =
  (reducer: Reducer<S>, initialState: S) => Store<S>;

type StoreCreatorEnhancer<S> =
  (createStore: StoreEnhancerStoreCreator<S>) => StoreEnhancerStoreCreator<S>;

type AlteringStoreEnhancerStoreCreator<S, R> =
  (reducer: Reducer<S>, initialState: S) => Store<R>;

type AlteringStoreCreatorEnhancer<S, R> =
  (createStore: AlteringStoreEnhancerStoreCreator<R, R>) =>
    AlteringStoreEnhancerStoreCreator<S, R>;

And two overloads for createStore with single type parameter:

interface StoreCreator {
  <S>(reducer: Reducer<S>, enhancer?: StoreCreatorEnhancer<S>): Store<S>;
  <S>(reducer: Reducer<S>, initialState: S, 
      enhancer?: StoreCreatorEnhancer<S>): Store<S>;

  <S, R>(reducer: Reducer<S>, enhancer: AlteringStoreCreatorEnhancer<S, R>): Store<R>;
  <S, R>(reducer: Reducer<S>, initialState: S,
         enhancer: AlteringStoreCreatorEnhancer<S, R>): Store<R>;
}

Summing up

I made everything work, although it seems kinda heavy. Maybe we should enforce that StoreCreator must operate on single state type but allow StoreEnhancer to change state type of StoreCreator.

@gaearon
Copy link
Contributor

gaearon commented May 17, 2016

Note that we’ll be changing how store enhancers work in a month or two: #1702

@aikoven
Copy link
Collaborator

aikoven commented May 17, 2016

Simpler version

After doing some more juggling I came up with the following proposal:

  1. Rename StoreEnhancerStoreCreator to StoreCreator:
// used by Store Enhancers
type StoreCreator<S> = (reducer: Reducer<S>, initialState: S) => Store<S>;
  1. Rename GenericStoreEnhancer/StoreEnhancer to StoreEnhancer/StrictStoreEnhancer. This is because third-party libs will mostly use the former, while the latter is only used by createStore. Also add support for altering state type:
type StoreEnhancer =
  <S, R>(createStore: StoreCreator<S>) => StoreCreator<R>;

type StrictStoreEnhancer<S, R> =
  (createStore: StoreCreator<S>) => StoreCreator<R>;
  1. Replace old StoreCreator with createStore overloads:
function createStore<S>(reducer: Reducer<S>,
                        enhancer?: StrictStoreEnhancer<S, S>): Store<S>;
function createStore<S>(reducer: Reducer<S>, initialState: S,
                        enhancer?: StrictStoreEnhancer<S, S>): Store<S>;

function createStore<S, R>(reducer: Reducer<S>,
                           enhancer: StrictStoreEnhancer<S, R>): Store<R>;
function createStore<S, R>(reducer: Reducer<S>, initialState: S,
                           enhancer: StrictStoreEnhancer<S, R>): Store<R>;

This greatly reduces the number of exported types and allows to solve initial problem:

declare function wrapReducer<S>(reducer: Reducer<S>): Reducer<S & { foo: string }>;
declare function wrapState<S>(state: S): S & { foo: string };

function enhancerChangingStateShape<S>(next: StoreCreator<S & { foo: string }>) {
    return (reducer: Reducer<S>, initialState: S) =>
      next(wrapReducer(reducer), wrapState(initialState));
}

const storeWithChangingEnhancer =
  createStore<State, State & {foo: string}>(reducer, enhancerChangingStateShape);

typeof storeWithChangingEnhancer.getState().foo  // string

@bogusfocused
Copy link

bogusfocused commented Sep 10, 2016

  let middlewares = [immutableStateInvariantMiddleware()];
  let middleware = applyMiddleware(...middlewares);

  let enhancer = compose(
    middleware,
      DevTools.instrument()
  );

gives error in compose
[ts] Argument of type 'Function' is not assignable to parameter of type 'Func3<{}, {}, {}, {}, StoreEnhancerStoreCreator<{}>>'.
Type 'Function' provides no match for the signature '(a1: {}, a2: {}, a3: {}, ...args: any[]): {}'

@aikoven
Copy link
Collaborator

aikoven commented Sep 12, 2016

@Rohitlodha What's the return type of DevTools.instrument()?

Try this:

import {GenericStoreEnhancer} from "redux";

let enhancer = compose(
  middleware,
  DevTools.instrument() as GenericStoreEnhancer
);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants