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 1 commit
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
60 changes: 39 additions & 21 deletions types/helpers.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@ import { Dispatch, Commit } from './index';
*/
type Computed<R> = () => R;
type Method<R> = (...args: any[]) => R;
type MutationMethod<P> = (payload: P) => void;
type ActionMethod<P> = (payload: P) => Promise<any>;
type CustomVue = Vue & Record<string, any>;

interface BaseType { [key: string]: any }
Expand All @@ -20,6 +18,26 @@ interface BaseMethodMap<F> {
[key: string]: (this: CustomVue, fn: F, ...args: any[]) => any;
}

type MethodType = 'optional' | 'normal'

/**
* Return component method type for a mutation.
* You can specify `Type` to choose whether the argument is optional or not.
*/
type MutationMethod<P, Type extends MethodType> = {
optional: (payload?: P) => void;
normal: (payload: P) => void;
}[Type];

/**
* Return component method type for an action.
* You can specify `Type` to choose whether the argument is optional or not.
*/
type ActionMethod<P, Type extends MethodType> = {
optional: (payload?: P) => Promise<any>;
normal: (payload: P) => Promise<any>;
}[Type];

/**
* mapGetters
*/
Expand Down Expand Up @@ -51,50 +69,50 @@ interface RootMapState<State, Getters> extends MapState<State, Getters> {
/**
* 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]]> };
interface MapMutations<Mutations, Type extends MethodType> {
<Key extends keyof Mutations>(map: Key[]): { [K in Key]: MutationMethod<Mutations[K], Type> };

Choose a reason for hiding this comment

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

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.

Copy link
Member Author

Choose a reason for hiding this comment

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

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.

<Map extends Record<string, keyof Mutations>>(map: Map): { [K in keyof Map]: MutationMethod<Mutations[Map[K]], Type> };
<Map extends BaseMethodMap<Commit<Mutations>>>(map: Map): { [K in keyof Map]: Method<any> };
}

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]]> };
interface RootMapMutations<Mutations, Type extends MethodType> extends MapMutations<Mutations, Type> {
<Key extends keyof Mutations>(namespace: string, map: Key[]): { [K in Key]: MutationMethod<Mutations[K], Type> };
<Map extends Record<string, keyof Mutations>>(namespace: string, map: Map): { [K in keyof Map]: MutationMethod<Mutations[Map[K]], Type> };
<Map extends BaseMethodMap<Commit<Mutations>>>(namespace: string, map: Map): { [K in keyof Map]: Method<any> };
}

/**
* 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]]> };
interface MapActions<Actions, Type extends MethodType> {
<Key extends keyof Actions>(map: Key[]): { [K in Key]: ActionMethod<Actions[K], Type> };
<Map extends Record<string, keyof Actions>>(map: Map): { [K in keyof Map]: ActionMethod<Actions[Map[K]], Type> };
<Map extends BaseMethodMap<Dispatch<Actions>>>(map: Map): { [K in keyof Map]: Method<any> };
}

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]]> };
interface RootMapActions<Actions, Type extends MethodType> extends MapActions<Actions, Type> {
<Key extends keyof Actions>(namespace: string, map: Key[]): { [K in Key]: ActionMethod<Actions[K], Type> };
<Map extends Record<string, keyof Actions>>(namespace: string, map: Map): { [K in keyof Map]: ActionMethod<Actions[Map[K]], Type> };
<Map extends BaseMethodMap<Dispatch<Actions>>>(namespace: string, map: Map): { [K in keyof Map]: Method<any> };
}

/**
* namespaced helpers
*/
interface NamespacedMappers<State, Getters, Mutations, Actions> {
interface NamespacedMappers<State, Getters, Mutations, Actions, Type extends MethodType> {
mapState: MapState<State, Getters>;
mapGetters: MapGetters<Getters>;
mapMutations: MapMutations<Mutations>;
mapActions: MapActions<Actions>;
mapMutations: MapMutations<Mutations, Type>;
mapActions: MapActions<Actions, Type>;
}

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: RootMapMutations<BaseType>;
export declare const mapMutations: RootMapMutations<BaseType, 'optional'>;

export declare const mapGetters: RootMapGetters<BaseType>;

export declare const mapActions: RootMapActions<BaseType>;
export declare const mapActions: RootMapActions<BaseType, 'optional'>;

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>;
export declare function createNamespacedHelpers(namespace?: string): NamespacedMappers<BaseType, BaseType, BaseType, BaseType, 'optional'>;
export declare function createNamespacedHelpers<State, Getters, Mutations, Actions>(namespace?: string): NamespacedMappers<State, Getters, Mutations, Actions, 'normal'>;
2 changes: 2 additions & 0 deletions types/test/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ new Vue({
this.g(7)
this.h(8)
this.i(9)
this.a() // should allow 0-argument call if untyped
}
})

Expand Down Expand Up @@ -202,5 +203,6 @@ new Vue({
this.h(8)
this.i(9)
this.otherMethod()
this.a() // should allow 0-argument call if untyped
}
});