championswimmer / vuex-module-decorators

TypeScript/ES7 Decorators to create Vuex modules declaratively
https://championswimmer.in/vuex-module-decorators/
MIT License
1.8k stars 170 forks source link

Getters don't work in subclass #287

Open zaro opened 4 years ago

zaro commented 4 years ago

Given the following base class:

export abstract class ModuleBase<Model> extends VuexModule {
  // Declare state
  items: {
    [key: string]: Model;
  } = {};

  get allItems(): Model[] {
    return Object.values(this.items);
  }

}

and subclass

@Module({name: 'itemList', namespaced: true})
export class ItemListModule extends ModuleBase<Item> {
}

when using ItemListModule the allItems getter is undefined. If I move the getter in the subclass everything works as expected.

This seems to be due to the fact that gettters are created by using Object.getOwnPropertyNames( at src/module/index.ts line 42), which doesn't take into account the parent classes.

this makes the modules defined with vuex-module-decorators way less reusable, as for example I have several base actions/mutations/getters over number of stores, and this make the code much less DRY.

cordery commented 4 years ago

See this workaround #152 , specifically the first block of code.

For example I have something like the following (probably inefficient, its fairly old code at this point when I was still figuring this out):

/* stores/BaseStore.ts */
export interface BaseModel {
  readonly id: number | string
  [key: string]: any
}
const getById = <Model extends BaseModel>(
  id: number | string,
  collection: Model[]
): Model => {
  ...
  return obj
}

export abstract class BaseStore<Model extends BaseModel> extends VuexModule {
  public all: Model[] = []

  /**
   * Regular getters do not inherit, so use this work around from
   * https://github.com/championswimmer/vuex-module-decorators/issues/152
   */
  public static getters?: GetterTree<any, any> = {
    /**
     * Get an object by its id.
     * @param state
     */
    getById: (state) => {
      return <Model extends BaseModel>(id: number | string) => {
        try {
          return <Model>getById(id, state.all)
        } catch (e) {
          throw new Error(`Object id ${id} not found.`)
        }
      }
    }
  }

  /* Repeated for Jetbrains IDE To pick it up properly, and in hopes that the work around won't be necessary in the future. */
  public getById(id: number | string): Model {
    return <any>getById(id, this.all)
  }
}

Then to extend it:


/* stores/users.ts */
import {BaseModel, BaseStore} from '~/stores/BaseStore.ts'
export interface User extends BaseModel { 
  name:string
  email:string
}

@Module({ stateFactory: true, name: "users", namespaced: true })
export default class Users extends BaseStore<User> {

  /* 
  * Custom actions, mutations, getters, etc here.   
  */
}

Create the initializer, all modules can be initialized here

/* stores/store-accessor.ts */
import { Store } from "vuex"
import { getModule } from "vuex-module-decorators"
import users from "~/store/users"

let usersStore: users

function initializeStores(store: Store<any>): void {
  usersStore = getModule(users, store)
}

export {
  initializeStores,
  usersStore
}

And finally in your store index.ts

/* stores/index.ts */
import { Store } from "vuex"
import { initialiseStores } from "~/helpers/store/store-accessor"

const initializer = (store: Store<any>) => initialiseStores(store)
export * from "~/helpers/store/store-accessor"

export const state = (): {
  exampleRootStateItem: string
} => ({
   (): {
  exampleRootStateItem: "just an example of root state",
})
export const mutations = {
  SET_EXAMPLE_ROOT_STATE_ITEM(state, value: string) {
    state.exampleRootStateItem = value
  },
}

export const plugins = [
  initializer,
]

So in your app code you could do the following and your intellisense and typescript should be happy and returning the correct types.

import { usersStore }  from "~/store"
import { User } from "~/store/users.ts"
usersStore.all /* accesses the all state variable in the users module we defined, and should be of type User[] */
const exampleUser = usersStore.getById(5) /* returns User id 5 (if it were in the all state variable).  exampleUser should be of type User if all went well.  */
zaro commented 4 years ago

@cordery This works, thank you. But it's sub-optimal at best, and also a bit annoying that getters is static, so doesn't play nice with generics and the need for repetition, so that code completion works.

I still think think this is a bug, as the current behaviour violates basic rules of inheritance. Also I think it won't be very hard to fix properly by simply using Object.keys (with some additional filtering if needed) instead of Object.getOwnPropertyNames.

hmuronaka commented 4 years ago

@zaro I had a same problem, so I fixed and sending pullreqest. But yet no response...

I hope this problem will be fixed. https://github.com/championswimmer/vuex-module-decorators/pull/284