vuejs / vue-class-component

ES / TypeScript decorator for class-style Vue components.
MIT License
5.81k stars 429 forks source link

Some unexpected situations happended at collectDataFromConstructor(vm, Component) function #440

Closed GitPopcorn closed 4 years ago

GitPopcorn commented 4 years ago

The code is breif, so I just copy it here to see it. The behaviors and reasons are all marked with ">>>>> [MARK]" title.

import Vue from 'vue'
import { VueClass } from './declarations'
import { warn } from './util'

export function collectDataFromConstructor (vm: Vue, Component: VueClass<Vue>) {
  // override _init to prevent to init as Vue instance
  const originalInit = Component.prototype._init
  Component.prototype._init = function (this: Vue) {
    // proxy to actual vm
    const keys = Object.getOwnPropertyNames(vm)
    // 2.2.0 compat (props are no longer exposed as self properties)
    // >>>>> [MARK] The data() function is usually called like "this.$options.data()", so the "vm" object tend to be "this.$options" some time,
    // which caused a excption like "[Vue warn]: Error in v-on handler: "TypeError: Cannot read property 'props' of undefined"
    // It can be resolve by call like "this.$options.data.call(this)", but it is really hard to find cause when exception ocurred.
    if (vm.$options.props) {
      for (const key in vm.$options.props) {
        if (!vm.hasOwnProperty(key)) {
          keys.push(key)
        }
      }
    }
    // >>>>> [MARK] I am not much clear why we should bind the data properties with original vm when invoke data() method.
    // When we call data(), we mostly need a pure new data object to used as just a plan object, like to show, to reset or to recognize.
    keys.forEach(key => {
      Object.defineProperty(this, key, {
        get: () => vm[key],
        set: value => { vm[key] = value },
        configurable: true
      })
    })
  }

  // should be acquired class property values
  const data = new Component()

  // restore original _init to avoid memory leak (#209)
  Component.prototype._init = originalInit

  // create plain data object
  const plainData = {}
  // >>>>> [MARK] Now we get a component instance created with custom _init method, however there is no enumerable settings when we define properties to the instance
  // So we can get only a empty array from Object.keys(data) that means nothing will be copy to the plain data.
  Object.keys(data).forEach(key => {
    if (data[key] !== undefined) {
      plainData[key] = data[key]
    }
  })

  if (process.env.NODE_ENV !== 'production') {
    if (!(Component.prototype instanceof Vue) && Object.keys(plainData).length > 0) {
      warn(
        'Component class must inherit Vue or its descendant class ' +
        'when class property is used.'
      )
    }
  }

  return plainData
}

The repository for reproduce: https://github.com/GitPopcorn/vue-class-component-issues-440

ktsn commented 4 years ago

Thank you for filing this issue. Please make sure to provide a minimal and self-contained reproduction if you are reporting a bug. You can use the following jsfiddle template or make an example github repository. https://jsfiddle.net/ktsn/nm55jnjk/

What is a minimal reproduction, and why is it required?

GitPopcorn commented 4 years ago

Thank you for filing this issue. Please make sure to provide a minimal and self-contained reproduction if you are reporting a bug. You can use the following jsfiddle template or make an example github repository. https://jsfiddle.net/ktsn/nm55jnjk/

What is a minimal reproduction, and why is it required?

Thanks for your reply. I have adding the repository in issue descriptions. Sadly, I can just reproduce the first problems I've marked, and no others happened. It may happend because of much too intricate inherit and mixin structure in my project. So that issue may be closed, no things important since the basic function for data method worked. I'm pretty sorry for the misdescription, I'm just annoyed by the work to much to try to separate the data and logic for component with suitable variable prompts in class API.

ktsn commented 4 years ago

Your reproduction looks identical with the out-of-box Vue CLI template and there seems to be no issues.

Anyways, if the first MARK is your issue, you should call data hooks with binding the component instance to this which is plugin author responsibility but not a vue-class-component issue.