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.

Sign up for GitHub

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

Jump to bottom

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

Open
nebaughman opened this issue Oct 19, 2021 · 5 comments · May be fixed by #27
Open

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

nebaughman opened this issue Oct 19, 2021 · 5 comments · May be fixed by #27

Comments

@nebaughman
Copy link

nebaughman commented Oct 19, 2021 •
edited
Loading 

"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!
@nebaughman
Copy link
Author

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.

@davestewart
Copy link
Owner

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?

@nebaughman
Copy link
Author

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!

@davestewart
Copy link
Owner

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

@thecodewarrior thecodewarrior linked a pull request Oct 30, 2021 that will close this issue
Open
@nebaughman
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Implement direct injection of vue into the constructed instance (Vue 2) DFStudio/vue-class-store
2 participants
@davestewart @nebaughman