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: add type annotation for the context of actions and mutations #1322

Merged
merged 2 commits into from
Jul 18, 2018

Conversation

sue71
Copy link
Contributor

@sue71 sue71 commented Jun 27, 2018

The handlers for actions and mutations are bound to the store, but there is no type annotation for this behavior.

This PR will enable us to use this as Store in the handlers.

@1000ch
Copy link

1000ch commented Jul 13, 2018

ping @ktsn

types/index.d.ts Outdated
export interface ActionObject<S, R> {
root?: boolean;
handler: ActionHandler<S, R>;
}

export type Getter<S, R> = (state: S, getters: any, rootState: R, rootGetters: any) => any;
export type Action<S, R> = ActionHandler<S, R> | ActionObject<S, R>;
export type Mutation<S> = (state: S, payload: any) => any;
export type Mutation<S> = (this: Store<S>, state: S, payload: any) => any;
Copy link
Member

Choose a reason for hiding this comment

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

What is the use case of this in mutations?
I think we should not use that in mutations while it might be useful in actions.

@sue71
Copy link
Contributor Author

sue71 commented Jul 18, 2018

@ktsn

What is the use case of this in mutations? I think we should not use that in mutations while it might be useful in actions.

I think so too. I just followed the implementation.

@ktsn
Copy link
Member

ktsn commented Jul 18, 2018

@sue71 If there are no use case for mutations, can we remove this type for mutations?
Originally, this in both the actions and mutations are something like private api to solve an issue on Nuxt. I don't think they are exposed on the user land. But I think it's fine for actions since it's valid use case and well-known for the users now.

@sue71
Copy link
Contributor Author

sue71 commented Jul 18, 2018

@ktsn make sense. I'll do so.

Copy link
Member

@ktsn ktsn left a comment

Choose a reason for hiding this comment

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

LGTM

@ktsn ktsn added the types Related to typings only label Jul 18, 2018
@ktsn ktsn merged commit d1b5c66 into vuejs:dev Jul 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
types Related to typings only
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants