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.

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

Data getters are being invoked on component initialisation #7280

Closed
DeyLak opened this issue Dec 19, 2017 · 5 comments · Fixed by #7302
Closed

Data getters are being invoked on component initialisation #7280

DeyLak opened this issue Dec 19, 2017 · 5 comments · Fixed by #7302

Comments

@DeyLak
Copy link
Contributor

DeyLak commented Dec 19, 2017

Version

2.5.11

Reproduction link

https://jsfiddle.net/7gmh5wL8/1/

Steps to reproduce

  • Run JSFiddle snippet
  • Open browser console
  • You'll see a warning from property getter

What is expected?

I expect, that my data and props enumerable properties will not be invoked, without my explicit call for them.

What is actually happening?

Seems, that defineReactive function in /src/core/observer/index.js works that way, that it accepts an object property value, instead of getting it from the getter on demand. Despite the fact, that it uses getOwnPropertyDescriptor to respect user-defined getters and setters.


I faced this problem in a context of lazy-loaded backend entities. I have some redux store selectors, that returns RESTfull models. Some of the models nested properties are wrapped into getters, so I can call them, if needed and trigger a background api request for additional info. Now, all of the getters are invoked immediately and a bunch of requests is sent after initial rendering.

I can do a pull request, to fix this, but may be I do not understand some concept under this behaviour.

Sorry for bad english.

@yyx990803
Copy link
Member

I don't think there is any guarantee regarding when getters will be invoked. In this case we need to traverse and observe the returned value from getters too, so we have to invoke the getter.

Also - it's a very bad idea to rely on getters for side effects. Getters should be pure.

@DeyLak
Copy link
Contributor Author

DeyLak commented Dec 20, 2017

@yyx990803, Sorry, I didn't get that thing about guarantee. I don't suggest not to traverse these properties, but why should we get the value of the property in walk, despite the fact, that it is used only if the property has no getter?
I see, that the val parameter is necessary in other method use-cases, but for walk function, I think, it can be omitted without any harm and should be calculated only, if property has no getter.

Thank you, I'm aware of that getters thing, but I'm using it carefully for saving development time. This is not the only reason, I think this change is usefull. May be, it is not a common use-case of using getters in props and data, but for those, who does it, it can be a source of lags(for some costly getter functions), or some other unintended behaviour.

DeyLak added a commit to DeyLak/vue that referenced this issue Dec 21, 2017
The defineReactive gets the property of the object, only if it has no getter

fix vuejs#7280
@HcySunYang
Copy link
Member

@yyx990803 @DeyLak
When a property of a data object only has getter:

  const data = {}
  Object.defineProperty(data, 'getterProp', {
    enumerable: true,
    configurable: true,
    get: () => {
      return {
        a: 1
      }
    }
  })

After processing by the defineReactive function, the data.getterProp property will have both a getter and a setter.When trying to set the value of the data.getterProp property, the later defined set function will be executed and the new value will be observed.However, this observed new value will never be relied upon.
I think we should modify the set method as shown in the following code:

    set: function reactiveSetter (newVal) {
      const value = getter ? getter.call(obj) : val
      /* eslint-disable no-self-compare */
      if (
        (getter && !setter) ||
        newVal === value ||
        (newVal !== newVal && value !== value)
      ) {
        return
      }
      /* eslint-enable no-self-compare */
      if (process.env.NODE_ENV !== 'production' && customSetter) {
        customSetter()
      }
      if (setter) {
        setter.call(obj, newVal)
      } else {
        val = newVal
      }
      childOb = !shallow && observe(newVal)
      dep.notify()
    }

Add judgment conditions: (getter && !setter).The existence of getter ensures that the property is an accessor property.If an accessor property does not have a setter, the set method should return directly.

@yyx990803
Copy link
Member

@HcySunYang feel free to submit a PR

@HcySunYang
Copy link
Member

Ok, I will submit a PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants