refactor: make the set method more reasonable

[HcySunYang]

What kind of change does this PR introduce? (check at least one)

• Bugfix
• Feature
• Code style update
• Refactor
• Build-related changes
• Other, please describe:

Does this PR introduce a breaking change? (check one)

• Yes
• No

If yes, please describe the impact and migration path for existing applications:

The PR fulfills these requirements:

• It's submitted to the dev branch for v2.x (or to a previous version branch), not the master branch
• When resolving a specific issue, it's referenced in the PR's title (e.g. fix #xxx[,#xxx], where "xxx" is the issue number)
• All tests are passing: https://github.com/vuejs/vue/blob/dev/.github/CONTRIBUTING.md#development-setup
• New/updated tests are included

If adding a new feature, the PR's description includes:

• A convincing reason for adding this feature (to avoid wasting your time, it's best to open a suggestion issue first and wait for approval before working on it)

Other information:
Please check: #7280 (comment)

[dsonet]

Then when have the chance to invoke customSetter?

[HcySunYang]

@dsonet

The following content is my understanding, there may be incorrect place.

The first thing we can guarantee is that if the value of the property property is undefined, both the value of getter and setter will be undefined:

/* defineReactive */
const property = Object.getOwnPropertyDescriptor(obj, key)
// If `property === undefined`, both `setter` and `getter` are undefined
const getter = property && property.get
const setter = property && property.set

The customSetter method is mainly used to print prompt information when the property value is set, and mainly used in the following aspects:

• Define the injected value on the instance object:

/* inject.js  initInjections */
defineReactive(vm, key, result[key], customSetter)

At this point, the return value of Object.getOwnPropertyDescriptor(vm, key) is undefined, so both getter and setter are undefined

• Define the $attrs and $listeners properties on the instance object:

/* render.js  initRender */
    defineReactive(vm, '$attrs', parentData && parentData.attrs || emptyObject, () => {
      !isUpdatingChildComponent && warn(`$attrs is readonly.`, vm)
    }, true)
    defineReactive(vm, '$listeners', options._parentListeners || emptyObject, () => {
      !isUpdatingChildComponent && warn(`$listeners is readonly.`, vm)
    }, true)

We can still guarantee that the following statement's value is undefined:

Object.getOwnPropertyDescriptor(vm, '$attrs') // undefined
Object.getOwnPropertyDescriptor(vm, '$listeners') // undefined

• Define the props data on the vm._props object:

/* state  initProps */
const props = vm._props = {}
defineReactive(props, key, value, customSetter)

The value of Object.getOwnPropertyDescriptor(props, key) is undefined。

In summary, when the value of the Object.getOwnPropertyDescriptor method is undefined, both the getter and the setter are undefined, so (getter && !setter) will always be false and will not affect the execution of the customSetter.

[haoqunjiang]

But won't it make the code harder to maintain? Since every time you add a call to defineReactive with customSetter you'll have to check all these conditions.

IMHO, as this refactor aims to get rid of unnecessary observation, putting the getter and setter check right before assigning to val might be easier to understand.

[HcySunYang]

you are right

[HcySunYang]

@sodatea I have made some changes.

[xiaoyikeji]

niubi

[Meoworu]

厉害厉害