vuex-class icon indicating copy to clipboard operation
vuex-class copied to clipboard

[feature request] make vuex-class more type safe

Open Evertt opened this issue 8 years ago β€’ 26 comments
trafficstars

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.

Evertt avatar Jan 20 '17 00:01 Evertt

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

ktsn avatar Jan 20 '17 01:01 ktsn

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?

Evertt avatar Jan 20 '17 01:01 Evertt

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

ktsn avatar Jan 20 '17 02:01 ktsn

Ah damn, that's too bad.

Evertt avatar Jan 20 '17 02:01 Evertt

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

Evertt avatar Jan 20 '17 17:01 Evertt

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. :-)

Evertt avatar Jan 21 '17 06:01 Evertt

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 😒

ktsn avatar Jan 21 '17 07:01 ktsn

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

Evertt avatar Jan 21 '17 15:01 Evertt

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...

Evertt avatar Jan 21 '17 23:01 Evertt

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 😞

ktsn avatar Jan 23 '17 05:01 ktsn

@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? :smile:

JsonSong89 avatar Apr 26 '17 02:04 JsonSong89

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

NightCatSama avatar Apr 13 '18 02:04 NightCatSama

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.

akoidan avatar Jun 22 '18 12:06 akoidan

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

championswimmer avatar Jul 14 '18 08:07 championswimmer

@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.

akoidan avatar Jul 14 '18 15:07 akoidan

@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

championswimmer avatar Jul 14 '18 17:07 championswimmer

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

akoidan avatar Jul 14 '18 21:07 akoidan

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

bhamde avatar Dec 09 '18 23:12 bhamde

@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. πŸŽ‰

FlorianWendelborn avatar Dec 28 '18 12:12 FlorianWendelborn

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.

kamok avatar Apr 19 '19 18:04 kamok

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?

frankshaka avatar May 20 '19 06:05 frankshaka

@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

akoidan avatar May 20 '19 08:05 akoidan

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.

frankshaka avatar May 21 '19 03:05 frankshaka

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.

masi avatar Jun 02 '19 16:06 masi

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

gdpaulmil avatar Jun 10 '19 15:06 gdpaulmil

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

merlosy avatar Mar 23 '22 21:03 merlosy