feat: improve helper types for more type safety

[ktsn]

This typings update allows to use typed getters/actions/mutations out of the box if they are used in the following manner.

1. Declare each store assets types as interfaces.

// State
export interface CounterState {
  count: number
}

// Getters
// key: getter name
// value: return type of getter
export interface CounterGetters {
  power: number
}

// Mutations
// key: mutation name
// value: payload type of mutation
export interface CounterMutations {
  increment: { amount: number }
}

// Actions
// key: action name
// value: payload type of action
export interface CounterActions {
  incrementAsync: { amount: number, delay: number }
}

2. annotate a namespaced module with DefineModule utility type.

The annotated assets must fulfill specified names and return type (getters) / payload type (actions, mutations). Also the assets types will be inferred.

The type in the following example should be fully inferred:

import { DefineModule } from 'vuex'

const counter: DefineModule<CounterState, CounterGetters, CounterMutations, CounterActions> = {
  namespaced: true,

  state: {
    count: 0
  },

  getters: {
    power: state => state.count * state.count
  },

  mutations: {
    increment (state, payload) {
      state.count += payload.amount
    }
  },

  actions: {
    incrementAsync ({ commit }, payload) {
      setTimeout(() => {
        commit('increment', { amount: payload.amount })
      }, payload.delay)
    }
  }
}

3. create typed namespaced helpers with createNamespacedHelpers.

Then, we can acquire typed mapXXX helpers for the defined namespaced module.

export const counterHelpers = createNamespacedHelpers<CounterState, CounterGetters, CounterMutations, CounterActions>('counter')

4. use the namespaced helpers in a component.

import { counterHelpers } from '@/store/modules/counter'

export default Vue.extend({
  computed: counterHelpers.mapState({
    value: 'count'
  }),

  methods: counterHelpers.mapMutations({
    inc: 'increment'
  }),

  created () {
    // These are correctly typed!
    this.inc({ amount: 1 })
    console.log(this.value)
  }
})

Caveats

Store is still not typed

I think it is probably impossible to infer the entire store type correctly since we cannot concat getter/actions/mutations names with namespace on type level. So this PR focuses how we do not use $store directly but use typed helpers instead.

It does not infer the types completely if passing functions to mapXXX helpers.

For example:

counterHelpers.mapState({
  foo: state => state.count
})

counterHelpers.mapMutations({
  bar (commit) {
    commit('increment', { amount: 1 })
  }
})

We can write the above code with inferred state and commit types but the component will have a type of foo: any and bar: (...args: any[]) => any.

It can be easily rewrite with a combination of object form mapXXX helpers and normal methods, so I think it would not be a problem.

Using root assets

The default mapXXX helpers still accepts any asset names and returns record of any type. To manually annotate them, we can use createNamespacedHelpers too. If we don't specify a namespace as an argument, it returns the root mapXXX helpers so that we annotate with root module assets types as same as namespaced modules.

const rootHelpers = createNamespacedHelpers<RootState, RootGetters, RootMutations, RootActions>()

fix #532
fix #564
fix #1119

[HerringtonDarkholme]

Technically this is a breaking change. I don't know how frequent Payload is used, but this might be a compatible alternative:

type Payload<P = { [k: string]: {} }, K extends keyof P = keyof P> =
    { type: K } & P[K]

http://www.typescriptlang.org/play/#src=type%20Payload%3CP%20%3D%20%7B%20%5Bk%3A%20string%5D%3A%20%7B%7D%20%7D%2C%20K%20extends%20keyof%20P%20%3D%20keyof%20P%3E%20%3D%0A%20%20%20%20%7B%20type%3A%20K%20%7D%20%26%20P%5BK%5D%0A%0Adeclare%20var%20p%3A%20Payload%0A

[ktsn]

I ended up reverting the rename of Payload 0858c6d

[HerringtonDarkholme]

Yes, changing interface to type would be a breaking change. My mistake 😷

[HerringtonDarkholme]

Thanks! This is a long change so I think I need some time to review the change. Also, we need documentation update to catch up this new awesomeness. 😄

[HerringtonDarkholme]

I'm fine with untyped action return type. In fact, users can register multiple actions so the return type can't be checked any way.

[HerringtonDarkholme]

Usually generic default is for interface / type alias. Mapper is not exported nor augmented by users, so I think these defaults aren't required.

[ktsn]

Actually, it throws error if Mutations is BaseType since M will be inferred as {} in that case. I have no idea how we fix it without default generic type 😞

But we may no longer need it if we create typed root helpers with createNamespacedHelpers in any cases.

[HerringtonDarkholme]

Let's try typed root helpers! It looks more promising since it can return more precise types.

[HerringtonDarkholme]

The proposed usage is mapState<RootState>(['foo', 'bar']), however, the return type is RootState because foo/bar isn't participating inference.

Another usage is that we export RootMapXXX and slightly change their type.

export interface RootMapState<State> {
  <Keys extends keyof State>(keys: Keys[]): {[K in Keys]: State[K]}
}

then users can write something like const myMapState: RootMapState<RootState> = mapState. Of course, it requires us to maintain more public types, and also looks a little bit bizarre to end users.

But it is more versatile in usage -- I think object style can be supported, and more precise in returning type.

[HerringtonDarkholme]

How about reusing the pattern of createNameSpacedHelpers? Adding one more helper function like createRootHelpers? Implementation wise, it just return these root helpers, but it gives better developer experience for both end users and lib maintainers.

[ktsn]

How about reusing the pattern of createNameSpacedHelpers

That sounds good idea. I think returning root helpers from createNamespacedHelpers with no argument would work. createRootHelpers would sound a bit weird for pure JS users.

[HerringtonDarkholme]

Awesome! Great balance between types and runtime behavior.

[HerringtonDarkholme]

What about a one-for-all option? like :

export type DefineStore<
  Actions,
  State,
  Getters,
  Mutations,
  ExtraActions = {},
  RootState = {},
  RootGetters = {},
  RootMutations = {},
  RootActions = {}
> = {
  // ....
}

[ktsn]

Yes, it looks better since we always combine all assets for module 😄
I will rewrite the utility types.

[ktsn]

documentation update

I will write docs for this. Thanks for pointing it out!

[Demivan]

There is a problem with returning MutationMethod here.
MutationMethod does not have ...args: any[] parameters anymore.
This makes all mutations take one argument, same goes for actions.

[ktsn]

It's not actually a problem because mutations and actions only accept one parameter. The returned methods will never process 2nd or latter parameters.

[Demivan]

This worked for me before

[Demivan]

You can pass multiple parameters (or none) to mapped actions

[ktsn]

Ah, I see. It should be an optional argument in default to follow the existing behavior.

But I think if we annotate types via createNamespacedHelper explicitly, it should not be an optional. Because it easily breaks apps if the user adds a payload for actions/mutations with no payload and the type checker cannot detect that.

If the users want to declare non-payload actions/mutations, they should explicitly pass a null or undefined value.

@HerringtonDarkholme What do you think?

[HerringtonDarkholme]

I agree that users should explicitly pass null or undefined.

[blake-newman]

I agree, i believe undefined is probably best to signal that this payload is void. null could have context to an application where undefined can't be used on the store as it breaks the reactivity principles.

[HerringtonDarkholme]

This typing still requires all methods in mutation/action are homogeneous: all methods either require one parameter at the same time or don't accept parameter at all. We cannot declare such mutations that some methods require parameter while others don't at the same time.

[ktsn]

Yes, I intend that behavior. I leave them optional if the users do not annotate types because they probably want flexible syntax like in JS. On the other hand, the methods always require an argument if they annotate types because they probably want type safety in that case.

[HerringtonDarkholme]

Hmmm, what about

This is mainly for TypeScript users to annotate root helper's type.

Mentioning TypeScript explicitly makes JS users know annotating type doesn't require much care for them.

[ktsn]

It sounds clearer than before. Thanks!

[HerringtonDarkholme]

IMHO it doesn't bother if we re-export other utility type helpers. These helper types can be convenient for users to separate getters/actions/mutations to different files.

[ktsn]

Makes sense. I've exposed them again.

[blake-newman]

As we are doing a large change for TS users, we should probably do this aswell: #994

So that $store can be argumented, thus giving more type safety.

[lzgrzebski]

any ETA when this will be released? Currently I have to patch my ts code with some explicit 'any' to make it work which isn't ideal.. :<

[uoc1691]

Great Work! Question on the usage.
Can the helper be used like
storeHelper.mapMutations(["doSomething"]).doSomething({event: new MyCustomEvent()});

Without doing

methods: counterHelpers.mapMutations({ inc: 'increment' })

[kirilvit]

Please, resolve the conflicts

[uoc1691]

Within an action "this" refers to the store instance. Can we modify DefineActions type like below?

type DefineActions<
    Actions,
    State,
    Getters,
    Mutations,
    ExtraActions = {},
    RootState = {},
    RootGetters = {},
    RootMutations = {},
    RootActions = {}
    > = {
      [K in keyof Actions]: (this: Store<RootState>,
        ctx: StrictActionContext<State, RootState, Getters, RootGetters, Mutations, RootMutations, Actions & ExtraActions, RootActions>,
        payload: Actions[K]
      ) => Promise<any> | void
    }

[ffxsam]

What's holding up this PR from being merged?

Can anyone tell me what a good workaround is, in the mean time? I'm not that fluent in TypeScript.

[hobotroid]

I too would love this PR to get merged.

[RehanSaeed]

Looks like @yyx990803 is yet to review this PR.

[zhangbobell]

It will make strong sense for this PR been merged.

[latel]

strill waiting for this pr

[Raiondesu]

Every maintainer of this repo seems to have forgotten about all type-improving pull requests...

[ffxsam]

@ktsn Any updates on this? I wrote a Vue + TS cookbook, and provide a workaround for this problem:

https://github.com/ffxsam/vue-typescript-cookbook#im-using-vuex-mapstate-or-mapgetters-and-typescript-is-saying-the-mapped-stategetters-dont-exist-on-this

But it would be nice if it worked properly out of the box.

[ffxsam]

Could someone please merge this? @yyx990803 @blake-newman @HerringtonDarkholme

[kirilvit]

Could someone please merge this? @yyx990803 @blake-newman @HerringtonDarkholme

I think this PR will not be ever merged, because TypeScript support with breaking changes is announced to be in VueJs 3 https://medium.com/the-vue-point/plans-for-the-next-iteration-of-vue-js-777ffea6fabf

[ffxsam]

@kirilvit That's a great point, I totally didn't think of that.

[firewave-remo]

But at least a statement would be fair, instead of just saying nothing...

[exherb]

any progress?

[pikax]

Made a PR ktsn#1 updating Typescript@3.3.3

[joevandyk]

Vue 3 is still a very long ways away, right? If the maintainer doesn't want to merge this, could they close it and explain why? Or suggest what changes need to be made to get this merged in?

[pikax]

I think this can't be merge because doesn't work on typescript 3.x (building errors)

[ClickerMonkey]

This is a little unrelated, but I've also create new typings for vuex. Similar to this, except instead of 4 interfaces to represent the state/getters/mutations/actions you specify one and it automatically can detect which properties are each:

https://github.com/ClickerMonkey/vuex-typescript-interface

[kiaking]

@ktsn Do you think this PR is still maintainable...? Or maybe we should close this one and start over if we really need it 😓

[ktsn]

We should go with Vuex 5's TypeScript improvement. Closing.