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

Setting an object through MutateAction throws an exception #382

Open ZeroThe2nd opened 2 years ago

ZeroThe2nd commented 2 years ago

Using the following module code:

@Module({
  name: 'ExampleModule',
  dynamic: true,
  store
})
export class ExampleModule extends VuexModule {
  valString: string | null = null
  valNumber: number | null = null
  valObject: object | null = null
  valArray: string[] = []

  @MutationAction
  async getObject(): Promise<Partial<ExampleModule>> {
    const response = await fetch('/load/object')
    return { valObject } = await response.json()
  }

  @MutationAction
  async applyState(
    state: Partial<ExampleModule>
  ): Promise<Partial<ExampleModule>> {
    return state
  }
}

Then using it in a Vue module, and calling getObject successfully sets the valObject from the API call's response.

However calling applyState({ valObject: { test: 'yes' } }) causes the ERR_MUTATE_PARAMS_NOT_IN_PAYLOAD exception to be thrown.

Adding a console.log('Apply state', state) before the return does work properly, and returns the object in vue's Observer shell. However I still receive the same exception message. The exception itself also isn't very clear what param(s) it does trip over, so it's a bit difficult to track down outside of stepping through the code manually.

ragnese commented 2 years ago

I think that the issue is that the Mutations' return value must ONLY have a STRICT SUBSET of the keys in the module's state. Unfortunately, this isn't actually something that can be expressed in TypeScript's type system. TypeScript interfaces and types only describes a "minimum" requirement for a value to satisfy.

To fix that problem, your applyState implementation should create a new object from the input by explicitly taking only the fields it needs:

  @MutationAction
  async applyState(
    { valString, valNumber, valObject, valArray }: Partial<ExampleModule>
  ): Promise<Partial<ExampleModule>> {
    return {
      valString,
      valNumber,
      valObject,
      valArray,
    }
  }
}

However, I would also suggest that you maybe narrow your MutationActions' return types. Technically Partial<ExampleModule> also includes the methods (and getters, mutations, actions, etc), but the return values for Mutations and MutationActions are only supposed to be Partials of the state (just the true fields of your class).

ZeroThe2nd commented 2 years ago

Fair, however when I pass an object that matches the spec, which the TypeScript type does force off in type checking at least, it still throws this same error. As stated in my initial description, even calling the method with a parameter that matches the store spec 1:1 it drops out with an error. Though that should not happen, as the mutation is properly valid.

ragnese commented 2 years ago

@ZeroThe2nd

Can you show the exact calling code? Are you accessing the store through the component plugin thing with vm.$store or are you using the getModule function provided by this library? Because I've found that the getModule function is incorrectly implemented and doesn't work correctly except for accessing the state directly. Are you passing a literal object to the action, or is it possible that the object you're passing has extra fields attached to it (being a superset of the interface)?

I don't really know how TS decorators/annotations work, nor exactly how this library implements these annotations, but I wonder if what might be happening is that the action is being transformed at compile time in such a way that it's including the fields of the Partial type in the action payload, even if they're undefined at runtime.

Honestly... I've stopped using this library. It is too much magic and it doesn't really work as expected- partly because TypeScript's typing can't actually be used to type the actions and mutations correctly (as I pointed out above), but also because the library itself has a few inconsistencies and bugs (like the getModule function). It was less painful to just suck it up and write my store the "default" way. I'm fairly close to giving up on the class component library for similar reasons...