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

[feature request] make vuex-class more type safe #2

Open
Evertt opened this issue Jan 20, 2017 · 26 comments · May be fixed by #16
Open

[feature request] make vuex-class more type safe #2

Evertt opened this issue Jan 20, 2017 · 26 comments · May be fixed by #16

Comments

@Evertt
Copy link

Evertt commented Jan 20, 2017

I just read a blog post which explained that TypeScript 2.1 has a new feature which makes it possible to type-check the keys of objects. Here's the blog post.

So basically I'm imagining that it should also be possible to do this with vuex, since the store object is just a regular object, right?

So my feature request means that if I have a store like this:

const store = new Vuex.Store({
  state: {
    count: 0
  },
  mutations: {
    increment (state) {
      state.count++
    }
  }
})

And I have a component like this:

import Vue from 'vue'
import Component from 'vue-class-component'
import {
  State,
  Getter,
  Action,
  Mutation,
  namespace
} from 'vuex-class'

const ModuleGetter = namespace('path/to/module', Getter)

@Component
export class MyComp extends Vue {
  @State('count') count
  @Mutation('incremet') increment
}

That the type-annotation would make typescript understand that the count variable of MyComp is a number, since the state variable is a number. And also that typescript would throw an error during compilation, because it can see that increment was misspelled.

Do you think you could try to implement that? Even just having one of those two features would already be awesome.

@ktsn
Copy link
Owner

ktsn commented Jan 20, 2017

I guess this can't be implemented, unfortunately, because decorators can't affect/detect the property's type 😞

@Evertt
Copy link
Author

Evertt commented Jan 20, 2017

Well you might be right about affect, but I'm pretty sure it can detect. Because I've seen another library implement such a thing:

This library allows you to decorate your component's properties without having to explicitly state which type they are. Here's a snippet from the example code:

@vts.component({components: {ChildComponent}})
export default class Example extends Vue {
    // props with initializer -> sets default value and type
    @vts.prop() aStringPropWithValue = 'abc'
    @vts.prop() aNumberPropWithValue = 123
}

So that means that library is able to detect the property's type that typescript is inferring it to be, right?

@ktsn
Copy link
Owner

ktsn commented Jan 20, 2017

This example code is doing just infer the property types from the default value. The decorator still does not detect it's type.

You can see the PropertyDecorator type does not have the information of property's type. https://github.com/Microsoft/TypeScript/blob/master/lib/lib.es6.d.ts#L1326

@Evertt
Copy link
Author

Evertt commented Jan 20, 2017 via email

@Evertt
Copy link
Author

Evertt commented Jan 20, 2017

This guy found a cool solution using functions combined with decorators. :-)

@Evertt
Copy link
Author

Evertt commented Jan 21, 2017

I'm still a bit confused. Like okay, I'm starting to accept that you can't influence a property's type using a property decorator. However, you do know the "key" to which the property is referring. As in, you know that when someone writes @State('count') count or @State count that they are referring to the count state in the store. So doesn't that mean you should at least be able to use the keyof T feature of typescript to check for spelling mistakes? Simply by doing something like this?

function prop<T, K extends keyof T>(obj: T, key: K) {
    return obj[key];
}

const doesntMatter = prop(store, propertyName);

Not that you are going to use that constant.. the point of that line of code is simply to make TypeScript throw a compilation-error if the property name contains a spelling error.

To be honest, I don't really know what I'm talking about and I'm just making guesses here. If I'm saying stupid shit and it annoys you, just let me know. :-)

@ktsn
Copy link
Owner

ktsn commented Jan 21, 2017

I see, we can check spelling error but maybe we need to some changes for API. I also noticed we need to update Vuex's declaration file because it currently does not have the type of getter/action/mutation.

I'm considering to implement them but I'm currently too busy by the other jobs. Maybe need 1 week or more 😢

@Evertt
Copy link
Author

Evertt commented Jan 21, 2017

I don't mind waiting a week. I'm already really happy that you're willing to look into this. :-)

@Evertt
Copy link
Author

Evertt commented Jan 21, 2017

I just learned something new while implementing the parentheses-less thingy on that other repo (your explanation was a great help by the way).

So yes, a property decorator can't detect the type of the property it is decorating, because it's only getting the target object and the property-name. However, that just so happens to be enough information to then do Reflect.getMetadata('design:type', target, key) to get the property type. Doesn't that mean we can also check the type? I'm just not sure if that then counts as runtime or compile-time...

@ktsn
Copy link
Owner

ktsn commented Jan 23, 2017

Reflect metadata provides a type information on runtime. So we cannot detect our mistake on compile time. In addition, it does not provide enough information. For example, it always returns Object for all following cases.

foo: null // null type
bar: undefined // undefined type
baz: string | number // union type
qux: { disalbed: boolean } // object type

I don't think it's useful for our use case 😞

@JsonSong89
Copy link

JsonSong89 commented Apr 26, 2017

@ktsn
I have a idea, like vue-class-component
Use a @VuexComponent decorate MyVuexClass
property->state
get->getter
set->mutation
method->action
and when combining the store, use ClassToModule method ,
transform a new instance(maybe can use singleton pattern) to a object like this:

export default {
    state,
    mutations,
    getters,
    actions
}

In our Vue Component , we can declare a instance(myStore) from MyVuexClass , because we had inject the store in root , we also can access state by myStore

        @StoreComponent( MyVuexClass.SingleInstance) myStore: MyVuexClass
        testClick2() {
            this.myStore.mutation1 = "123"
            this.myStore.mutation2 = 123
            this.myStore.action1({param1: "123", param2: 123})
            console.log(this.myStore.state1)
            console.log(this.myStore.getter1())
        }

I am new with TypeScript , this is just a imagine, is it possible? 😄

@ktsn ktsn linked a pull request Jan 11, 2018 that will close this issue
@NightCatSama
Copy link

NightCatSama commented Apr 13, 2018

Is there any progress?

// store/modules/product
export interface ProductState {
  name: string
  price: number
}
const state: ProductState = {
   name: '',
   price: 0
}

What can I do to know the state type in the Vue component?

  @State(state => state.product.name) productName // can be inferred to be a string

@akoidan
Copy link

akoidan commented Jun 22, 2018

Can we also consider adding compile type check instead of relying on typescript type-safe? Like linter or so. Thus we can add it to vue-loader or smth.

@championswimmer
Copy link

I have been working lately on https://www.npmjs.com/package/vuex-module-decorators
We can probably use that in conjunction to infer types? We can build class-based store modules using this, and from class we can infer datatypes of store entities

@akoidan
Copy link

akoidan commented Jul 14, 2018

@championswimmer which features make it better than vue-property-decorator? I don't want to redeclare types above each action, I'm even lazy to append import for annotation, I guess other folks are lazy as I am.

@championswimmer
Copy link

championswimmer commented Jul 14, 2018

@Deathangel908 not sure what you mean by

I don't want to redeclare types above each action

but vuex-module-decorators are for vuex modules while vue-property-decorator is for vue components

@akoidan
Copy link

akoidan commented Jul 14, 2018

@championswimmer ok, sorry my bad, I see it allows to describe vuex , looks promising.

@bhamde
Copy link

bhamde commented Dec 9, 2018

That would be great cause currently it cannot detect @action type and always complain about for noimplicitany (or no-explicit-any)

@FlorianWendelborn
Copy link

FlorianWendelborn commented Dec 28, 2018

@championswimmer @ktsn Do you think it’s possible to infer type information from vuex-module-decorators in vuex-class/vuex-property-decorator?


Edit: Solution

@namespace('my.store').Action initialize!: store['initialize']

This will lead typescript to infer type information from vuex-module-decorators within vuex-class/vuex-property-decorator. 🎉

@kamok
Copy link

kamok commented Apr 19, 2019

Hello developers. Currently my team is duplicating the type from the actions with a type declaration in a separate files.

type SomeAction ({ foo }: { foo: string }) => void;

@SomeStore.Action someAction: SomeAction;

This gives us some sort of type checking but it requires upkeep. Do we have a better solution?

I've been playing with type intersection in order to declare a type and then intersect it into the Actions declaration. But it's still a WIP.

@frankshaka
Copy link

frankshaka commented May 20, 2019

I've found a workaround with the help of vuex-module-decorators and typeof, without modifying any library.

For example, I can define my component like this:

import Component from 'vue-class-component'
import { namespace } from 'vuex-class'

// This is a store module class defined using vuex-module-decorators
import { Foo } from '~/store/modules/foo'

const foo = namespace('foo')

@Component
export default class App extends Vue {

    @foo.State bars: typeof Foo.prototype.bars  /// i.e. string[]

    @foo.Mutation addBar: typeof Foo.prototype.addBar   /// i.e. (bar: string) => void

    get barString() {
        return this.bars.join(', ')  /// <- we should have type check here at compile-time
    }

    onClick() {
        this.addBar(...)  /// <- we should have type check here at compile-time
    }

}

Given that the Foo module is defined like this:

import { VuexModule, Module, Mutation } from 'vuex-module-decorators'

@Module({ namespaced: true, name: 'foo' })
export class Foo extends VuexModule {

    public bars: string[] = []

    @Mutation addBar(bar: string): void {
        this.bars = this.bars.concat(bar)
    }

    @Mutation removeBar(bar: string): void {
        this.bars = this.bars.filter(b => b !== bar)
    }

}

Anyone check this out?

@akoidan
Copy link

akoidan commented May 20, 2019

@frankshaka any benefict for using vuex-module-decorators along with vuex-class? I see 2 issues:

  • what if I accidentally mistype @foo.State bars: typeof Foo.prototype.oopsbars - I won't get an exception
  • I can use vuex-module-decorators without vuex-class just like this:
import Component from 'vue-class-component'

// This is a store module class defined using vuex-module-decorators
import { Foo } from '~/store/modules/foo'

@Component
export default class App extends Vue {
    private readonly foo: getModule(Foo)
    get barString() {
        return this.foo.bars.join(', ') ; // typesafe
    }

    onClick() {
         this.foo.addBar(...)  // typesafe
    }

}

p.s. your example
return this.bars.join(', ') //works
this.addBar(...) // doesn't work

@frankshaka
Copy link

any benefict for using vuex-module-decorators along with vuex-class?

vuex-module-decorators is for defining Vuex modules, vuex-class is for binding Vuex assets with Vue instances. I see no reason why they can't work in conjunction.

I know it's hard for vuex-class to detect types (from non-typed Vuex stores), but since we have already defined types in module classes, re-using them for binding is better than never, IMHO.

  • what if I accidentally mistype @foo.State bars: typeof Foo.prototype.oopsbars - I won't get an exception

Emmm.... Mistyping is truly an issue. But since it's an issue, it may break other things as well, like onClick() { this.foo.addOopsBar(...) }. We're talking "type safety" here, so first we have to make our types correct, right?

And, since my definitions like @foo.State bars: typeof Foo.prototype.bars all follow a similar pattern, I can even make a lint tool for this to warn me about mistypings.

  • I can use vuex-module-decorators without vuex-class just like this:

I think this is off the topic. Our discussions here are to make vuex-class better, not to abandon it.

I definitely know that we can accomplish total "type safety" without using vuex-class at all. However, this library stands for its own reason.

p.s. your example
this.addBar(...) // doesn't work

I don't get what's not working. My example works as expected.

@masi
Copy link

masi commented Jun 2, 2019

any benefict for using vuex-module-decorators along with vuex-class?

vuex-module-decorators is for defining Vuex modules, vuex-class is for binding Vuex assets with Vue instances. I see no reason why they can't work in conjunction.

But what is the point? In your example you have to decorate everything just to bind it. WHy is binding better than accessing the store directly? After all without TypeScript this is exactly what you would do.

  • I can use vuex-module-decorators without vuex-class just like this:

I think this is off the topic. Our discussions here are to make vuex-class better, not to abandon it.

I definitely know that we can accomplish total "type safety" without using vuex-class at all. However, this library stands for its own reason.

Probably. But as long as I have to duplicate declarations (because of technical limitations) the idea of binding is a dead end. I have started out with vuex-class but I think I'll swap it out for vuex-module-decorators or anything else that let's me define the types just once. Sidenote: vuex-module-decorators has some other issues.

Sorry!

Yet, thanks to ktsn for writing the package and sharing the code.

@gdpaulmil
Copy link

gdpaulmil commented Jun 10, 2019

image

extreme lack of type safety, actualy none of it at all

namespace should be defined with interface T and/or string or some constant token defined in the
same file as store, that knows about module name and its interface
and the property in @ State should be a keyof T

task priority -> critical

@merlosy
Copy link

merlosy commented Mar 23, 2022

Hi!
I figured out something that works in my case. I don't need to duplicate type definitions anymore in the component, it's inferred from the store.

https://gist.github.com/merlosy/16d5251edf082d2f3cfa599ef8c1733f

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

Successfully merging a pull request may close this issue.