Skip to content

Commit

Permalink
Revert "refactor: remove unnecessary checks (#7875)"
Browse files Browse the repository at this point in the history 
This reverts commit 43551b4.
  • Loading branch information
@yyx990803
yyx990803 committed Mar 22, 2018
1 parent 49385e1 commit 903be9b
Showing 1 changed file with 3 additions and 2 deletions.
5 changes: 3 additions & 2 deletions src/core/instance/state.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,9 @@ function initProps (vm: Component, propsOptions: Object) {

function initData (vm: Component) {
let data = vm.$options.data
// $options.data is guaranteed to be a function after merge
data = vm._data = getData(data, vm)
data = vm._data = typeof data === 'function'
? getData(data, vm)
: data || {}
if (!isPlainObject(data)) {
data = {}
process.env.NODE_ENV !== 'production' && warn(
Expand Down

3 comments on commit 903be9b

@default-writer
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What a f$%@y (friendly) man decided to 'remove unnececssary checks'?
typeof data === 'function' is assumes that data must be a function SO it can't be undefined, if you remove these checks instead of default data value {}, original undefined can be bassed, or even worse, you stick to pass data with a function only and excludes passing something else. You can, but your code will end up with getData(undefined, vm).

@HcySunYang
Copy link
Member

@HcySunYang HcySunYang commented on 903be9b Mar 22, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here are a few examples:
First:

const ins = new Vue({
  data: {}
})
console.log(ins.$options.data)  // mergedInstanceDataFn

This is what we expect.
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! Dividing line !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
Second:

const Child = Vue.extend({
  data: '' // or {}
})
console.log(Child.options.data) // undefined

This is because the data option of the child component must be a function. If it is not a function then the strats.data function will return parentVal(Vue.options.data), but since Vue.options.data is equal to undefined, the value of Child.options.data is also undefined.

!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! Dividing line !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
Third:

const Child = Vue.extend({
  data: {}
})
const cins = new Child({
  data: {}
})
console.log('data: ', cins.$options.data) // mergedInstanceDataFn

According to the second example, the value of Child.options.data is undefined.So at this point new Child() is equivalent to new Vue(), so the value of cins.$options.data is the mergedInstanceDataFn function.
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! Dividing line !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
Fourth:

const Child = Vue.extend({
  data () {
    return {}
  }
})
console.log(Child.options.data) // data () {...}

When the child component's data option is a function, since the value of parentVal(Vue.options.data) is undefined, childValue(data(){...}) is directly used as the value of Child.options.data, and still Is a function.
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! Dividing line !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
Fifth:

const Child = Vue.extend({
  data () {}
})
const cins = new Child({
  data () {}
})

Even if the data function does not return a value, it will not be getData(undefined, vm). Simple analysis of the source code will know.

In short, the value of vm.$options.data is either a function or undefined. Where do I miss it?
@yyx990803 @hack2root

@yyx990803
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The core tests are passing, but on CI this commit caused some Weex-related tests to fail.

Please sign in to comment.