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

feat: improve helper types for more type safety #1121

Closed
wants to merge 25 commits into from
Closed
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
0768c80
feat: improve helper types to utilize Vue 2.5 types
ktsn Nov 6, 2017
25775c5
feat(types): [WIP] more strict typed store assets
ktsn Jan 4, 2018
21ee399
Merge branch 'dev' into feat-improve-typing
ktsn Jan 4, 2018
eee1f3c
fix(types): relax map state function type
ktsn Jan 4, 2018
f94cf70
test(types): update namespaced helper type test
ktsn Jan 4, 2018
a5c4e26
feat(types): allow to specify assets types on mapXXX helpers
ktsn Jan 4, 2018
58d28a5
chore(types): add comments for helpers and utilities types
ktsn Jan 4, 2018
0858c6d
fix(types): revert renaming Payload to avoid breaking change
ktsn Jan 9, 2018
1e27c5e
feat(helpers): return root helpers if no namespace is provided to cre…
ktsn Jan 9, 2018
c2068f3
feat(types): add `DefineModule` utility type
ktsn Jan 9, 2018
7abf34f
fix(types): allow to omit payload on mapped methods if it is untyped
ktsn Jan 9, 2018
9564b80
docs(helpers): improve `createNamespacedHelpers` description
ktsn Jan 11, 2018
1aa407f
fix(types): expose DefineGetters/Mutations/Actions type
ktsn Jan 11, 2018
cfb6042
chore: include utils.d.ts for `files` field
ktsn Jan 11, 2018
00360b5
fix(types): make dispatch/commit more type safe in module actions if …
ktsn Jan 17, 2018
9b89ae7
fix(types): remove default type parameters from StrictDispatch/Commit
ktsn Jan 17, 2018
09475d5
refactor: use undefined type to indicate empty payload instead of null
ktsn Jan 17, 2018
8e0c60b
fix(types): fix incorrect type annotation
ktsn Jan 17, 2018
b14662c
fix(types): fix ActionContext type
ktsn Jan 17, 2018
b24d744
fix(types): remove incorrect overload
ktsn Jan 18, 2018
8b6a6f9
refactor(types): just use Dispatch/CommitOptions instead of BaseDispa…
ktsn Jan 18, 2018
9b183f0
docs: add typescript docs
ktsn Jan 22, 2018
715eaad
docs: fix typo in typescript docs
ktsn Jan 22, 2018
fc1f29b
docs: add a note about conditional types
ktsn Jan 25, 2018
c3626f7
Merge branch 'dev' into feat-improve-typing
ktsn Feb 1, 2018
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion docs/en/api.md
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,8 @@ const store = new Vuex.Store({ ...options })

The first argument can optionally be a namespace string. [Details](modules.md#binding-helpers-with-namespace)

- **`createNamespacedHelpers(namespace: string): Object`**
- **`createNamespacedHelpers(namespace?: string): Object`**

Create namespaced component binding helpers. The returned object contains `mapState`, `mapGetters`, `mapActions` and `mapMutations` that are bound with the given namespace. [Details](modules.md#binding-helpers-with-namespace)

If the namespace is not specified, it returns the root mapXXX helpers. This behavior is convenient to annotate strict types for mapXXX helpers.
Copy link
Member

@HerringtonDarkholme HerringtonDarkholme Jan 10, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It sounds clearer than before. Thanks!

18 changes: 9 additions & 9 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -65,11 +65,11 @@
"rollup-watch": "^4.3.1",
"selenium-server": "^2.53.1",
"todomvc-app-css": "^2.1.0",
"typescript": "^2.5.3",
"typescript": "^2.6.1",
"uglify-js": "^3.1.2",
"vue": "^2.5.0",
"vue": "^2.5.13",
"vue-loader": "^13.3.0",
"vue-template-compiler": "^2.5.0",
"vue-template-compiler": "^2.5.13",
"webpack": "^3.7.1",
"webpack-dev-middleware": "^1.10.0",
"webpack-hot-middleware": "^2.19.1"
Expand Down
12 changes: 7 additions & 5 deletions src/helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -111,14 +111,16 @@ export const mapActions = normalizeNamespace((namespace, actions) => {

/**
* Rebinding namespace param for mapXXX function in special scoped, and return them by simple object
* @param {String} namespace
* If the namespace is not specified, it returns the root mapXXX helpers.
* This behavior is convenient to annotate strict types for mapXXX helpers.
* @param {String} [namespace]
* @return {Object}
*/
export const createNamespacedHelpers = (namespace) => ({
mapState: mapState.bind(null, namespace),
mapGetters: mapGetters.bind(null, namespace),
mapMutations: mapMutations.bind(null, namespace),
mapActions: mapActions.bind(null, namespace)
mapState: namespace ? mapState.bind(null, namespace) : mapState,
mapGetters: namespace ? mapGetters.bind(null, namespace) : mapGetters,
mapMutations: namespace ? mapMutations.bind(null, namespace) : mapMutations,
mapActions: namespace ? mapActions.bind(null, namespace) : mapActions
})

/**
Expand Down
52 changes: 52 additions & 0 deletions test/unit/helpers.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -517,4 +517,56 @@ describe('Helpers', () => {
vm.actionB()
expect(actionB).toHaveBeenCalled()
})

it('createNamespacedHelpers: generates root helpers', () => {
const actionA = jasmine.createSpy()
const actionB = jasmine.createSpy()
const store = new Vuex.Store({
state: { count: 0 },
getters: {
isEven: state => state.count % 2 === 0
},
mutations: {
inc: state => state.count++,
dec: state => state.count--
},
actions: {
actionA,
actionB
}
})
const {
mapState,
mapGetters,
mapMutations,
mapActions
} = createNamespacedHelpers()
const vm = new Vue({
store,
computed: {
...mapState(['count']),
...mapGetters(['isEven'])
},
methods: {
...mapMutations(['inc', 'dec']),
...mapActions(['actionA', 'actionB'])
}
})
expect(vm.count).toBe(0)
expect(vm.isEven).toBe(true)
store.state.count++
expect(vm.count).toBe(1)
expect(vm.isEven).toBe(false)
vm.inc()
expect(store.state.count).toBe(2)
expect(store.getters.isEven).toBe(true)
vm.dec()
expect(store.state.count).toBe(1)
expect(store.getters.isEven).toBe(false)
vm.actionA()
expect(actionA).toHaveBeenCalled()
expect(actionB).not.toHaveBeenCalled()
vm.actionB()
expect(actionB).toHaveBeenCalled()
})
})
127 changes: 79 additions & 48 deletions types/helpers.d.ts
Original file line number Diff line number Diff line change
@@ -1,69 +1,100 @@
import Vue from 'vue';
import { Dispatch, Commit } from './index';

type Dictionary<T> = { [key: string]: T };
type Computed = () => any;
type MutationMethod = (...args: any[]) => void;
type ActionMethod = (...args: any[]) => Promise<any>;
type CustomVue = Vue & Dictionary<any>

interface Mapper<R> {
(map: string[]): Dictionary<R>;
(map: Dictionary<string>): Dictionary<R>;
/**
* Utility types to declare helper types
*/
type Computed<R> = () => R;
type Method<R> = (...args: any[]) => R;
type MutationMethod<P> = (payload: P) => void;
type ActionMethod<P> = (payload: P) => Promise<any>;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

type CustomVue = Vue & Record<string, any>;

interface BaseType { [key: string]: any }

interface BaseStateMap<State, Getters> {
[key: string]: (this: CustomVue, state: State, getters: Getters) => any;
}

interface BaseMethodMap<F> {
[key: string]: (this: CustomVue, fn: F, ...args: any[]) => any;
}

/**
* mapGetters
*/
interface MapGetters<Getters> {
<Key extends keyof Getters>(map: Key[]): { [K in Key]: Computed<Getters[K]> };
<Map extends Record<string, keyof Getters>>(map: Map): { [K in keyof Map]: Computed<Getters[Map[K]]> };
}

interface RootMapGetters<Getters> extends MapGetters<Getters> {
<Key extends keyof Getters>(namespace: string, map: Key[]): { [K in Key]: Computed<Getters[K]> };
<Map extends Record<string, keyof Getters>>(namespace: string, map: Map): { [K in keyof Map]: Computed<Getters[Map[K]]> };
}

/**
* mapState
*/
interface MapState<State, Getters> {
<Key extends keyof State>(map: Key[]): { [K in Key]: Computed<State[K]> };
<Map extends Record<string, keyof State>>(map: Map): { [K in keyof Map]: Computed<State[Map[K]]> };
<Map extends BaseStateMap<State, Getters>>(map: Map): { [K in keyof Map]: Computed<any> };
}

interface MapperWithNamespace<R> {
(namespace: string, map: string[]): Dictionary<R>;
(namespace: string, map: Dictionary<string>): Dictionary<R>;
interface RootMapState<State, Getters> extends MapState<State, Getters> {
<Key extends keyof State>(namespace: string, map: Key[]): { [K in Key]: Computed<State[K]> };
<Map extends Record<string, keyof State>>(namespace: string, map: Map): { [K in keyof Map]: Computed<State[Map[K]]> };
<Map extends BaseStateMap<State, Getters>>(namespace: string, map: Map): { [K in keyof Map]: Computed<any> };
}

interface FunctionMapper<F, R> {
(map: Dictionary<(this: CustomVue, fn: F, ...args: any[]) => any>): Dictionary<R>;
/**
* mapMutations
*/
interface MapMutations<Mutations> {
<Key extends keyof Mutations>(map: Key[]): { [K in Key]: MutationMethod<Mutations[K]> };
<Map extends Record<string, keyof Mutations>>(map: Map): { [K in keyof Map]: MutationMethod<Mutations[Map[K]]> };
<Map extends BaseMethodMap<Commit<Mutations>>>(map: Map): { [K in keyof Map]: Method<any> };
}

interface FunctionMapperWithNamespace<F, R> {
(
namespace: string,
map: Dictionary<(this: CustomVue, fn: F, ...args: any[]) => any>
): Dictionary<R>;
interface RootMapMutations<Mutations> extends MapMutations<Mutations> {
<Key extends keyof Mutations>(namespace: string, map: Key[]): { [K in Key]: MutationMethod<Mutations[K]> };
<Map extends Record<string, keyof Mutations>>(namespace: string, map: Map): { [K in keyof Map]: MutationMethod<Mutations[Map[K]]> };
<Map extends BaseMethodMap<Commit<Mutations>>>(namespace: string, map: Map): { [K in keyof Map]: Method<any> };
}

interface MapperForState {
<S>(
map: Dictionary<(this: CustomVue, state: S, getters: any) => any>
): Dictionary<Computed>;
/**
* mapActions
*/
interface MapActions<Actions> {
<Key extends keyof Actions>(map: Key[]): { [K in Key]: ActionMethod<Actions[K]> };
<Map extends Record<string, keyof Actions>>(map: Map): { [K in keyof Map]: ActionMethod<Actions[Map[K]]> };
<Map extends BaseMethodMap<Dispatch<Actions>>>(map: Map): { [K in keyof Map]: Method<any> };
}

interface MapperForStateWithNamespace {
<S>(
namespace: string,
map: Dictionary<(this: CustomVue, state: S, getters: any) => any>
): Dictionary<Computed>;
interface RootMapActions<Actions> extends MapActions<Actions> {
<Key extends keyof Actions>(namespace: string, map: Key[]): { [K in Key]: ActionMethod<Actions[K]> };
<Map extends Record<string, keyof Actions>>(namespace: string, map: Map): { [K in keyof Map]: ActionMethod<Actions[Map[K]]> };
<Map extends BaseMethodMap<Dispatch<Actions>>>(namespace: string, map: Map): { [K in keyof Map]: Method<any> };
}

interface NamespacedMappers {
mapState: Mapper<Computed> & MapperForState;
mapMutations: Mapper<MutationMethod> & FunctionMapper<Commit, MutationMethod>;
mapGetters: Mapper<Computed>;
mapActions: Mapper<ActionMethod> & FunctionMapper<Dispatch, ActionMethod>;
/**
* namespaced helpers
*/
interface NamespacedMappers<State, Getters, Mutations, Actions> {
mapState: MapState<State, Getters>;
mapGetters: MapGetters<Getters>;
mapMutations: MapMutations<Mutations>;
mapActions: MapActions<Actions>;
}

export declare const mapState: Mapper<Computed>
& MapperWithNamespace<Computed>
& MapperForState
& MapperForStateWithNamespace;
export declare const mapState: RootMapState<BaseType, BaseType>;
Copy link
Member

@HerringtonDarkholme HerringtonDarkholme Jan 6, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome! Great balance between types and runtime behavior.


export declare const mapMutations: Mapper<MutationMethod>
& MapperWithNamespace<MutationMethod>
& FunctionMapper<Commit, MutationMethod>
& FunctionMapperWithNamespace<Commit, MutationMethod>;
export declare const mapMutations: RootMapMutations<BaseType>;

export declare const mapGetters: Mapper<Computed>
& MapperWithNamespace<Computed>;
export declare const mapGetters: RootMapGetters<BaseType>;

export declare const mapActions: Mapper<ActionMethod>
& MapperWithNamespace<ActionMethod>
& FunctionMapper<Dispatch, ActionMethod>
& FunctionMapperWithNamespace<Dispatch, ActionMethod>;
export declare const mapActions: RootMapActions<BaseType>;

export declare function createNamespacedHelpers(namespace: string): NamespacedMappers;
export declare function createNamespacedHelpers(namespace?: string): NamespacedMappers<BaseType, BaseType, BaseType, BaseType>;
export declare function createNamespacedHelpers<State, Getters, Mutations, Actions>(namespace?: string): NamespacedMappers<State, Getters, Mutations, Actions>;
Loading