davestewart/vue-class-store

Constructor not entirely suitable as created hook (has wrong `this`)

Opened this issue · 5 comments

"vue": "^2.6.11",
"vue-class-store": "^2.0.5",

Experimenting with vue-class-store as a replacement for Vuex and loving it so far. However...

The following example does not work as expected. Setting local properties in the constructor does work, but calling methods that reference this will have a bad reference.

@VueStore
class Store {
  counter = 0
  constructor() {
    this.counter = 256 // this works
    this.startCounting() // will not work (method will reference wrong `this`)
  }

  private startCounting() {
    setInterval(() => this.counter++, 1000) // does not work if called from constructor (wrong `this.counter`)
  }
}

Workaround (somewhat awkward):

@VueStore
class Store {

  counter = 0

  // fired immediately by Vue with correct `this`
  private "on:any_random_name" = {
    immediate: true,
    handler: this.startCounting
  }

  private startCounting() {
    setInterval(() => this.counter++, 1000)
  }
}

Decorator workaround (still awkward):

// decorator function
function Creatable(target: Function) {
  const uuid = createUUID() // for example
  target.prototype[`on:${uuid}`] = {
    immediate: true,
    handler: "created"
  }
}

@VueStore
@Creatable // must be AFTER @VueStore -- awkward!
class Store {
  counter = 0
  private created() { // pseudo-created hook
    setInterval(() => this.counter++, 1000) // has correct `this`
  }
}

Maybe this can be fixed in a more elegant way? Any advice is most welcome. Thanks for your work on this project!

Looking at your index.ts, not sure if it's possible to augment makeOptions() to wire up a created() hook for Vue to call on instantiation.

If not, here's a suggestion from a colleague:

export default function VueStore<T extends C> (constructor: T): T {
  function construct (...args: any[]) {
    const instance = new (constructor as C)(...args)
    const vue = makeVue(instance)
    if (typeof vue['created'] === 'function') vue.created()
    return vue
  }
  return (construct as unknown) as T
}

If it exists, call created() on the post-Vue-tampering-with-the-model instance.

Hey there,

Thanks for trying the lib and I'm glad you're enjoying it so much!

Good catch here.

I'm really restricted on time for OSS right now, but would like to give this some attention. I also have a bunch of utilities I want to add to the library, so hopefully it won't be too long until I get to this.

Will you OK with the workarounds in the meantime?

Hi, thanks for your reply. Still experimenting with your lib and really liking its direction. I feel that this issue #26 & issue #14 (object replacement reactivity) are stumbling blocks to adopting this more widely, because they break from the patterns of typical Vue components, and would likely introduce mistakes in use. The workarounds are awkward at best.

Fully understand your time constraints. If I learn a bit more about Vue's internals, I may be able to offer a PR (but not very soon).

Do you think the if (typeof vue['created'] === 'function') vue.created() patch is the right approach, or would it be cleaner integration to (somehow) augment makeOptions() such that Vue's own initialization calls the created() hook as part of the normal Vue lifecycle? ... if that's even the way to approach this.

Thanks for any advice!

Do you think the if (typeof vue['created'] === 'function') vue.created() patch is the right approach...

No idea right now! Would have to refamiliarize myself with the code! :P

I can confirm that PR #27 fixes this issue, as well as issue #14.