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

Fix: nested child elements can not be updated correctly, fix #5618 #5627

Merged
merged 3 commits into from May 9, 2017

Conversation

gebilaoxiong
Copy link
Member

#5618

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:

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:

@gebilaoxiong gebilaoxiong changed the title avoid elements to be set key twice, which created by slot, fix #5618 avoid elements to be set key, which created by slot, fix #5618 May 7, 2017
@gebilaoxiong gebilaoxiong changed the title avoid elements to be set key, which created by slot, fix #5618 avoid to be set the key which elements created by slot, fix #5618 May 7, 2017
@gebilaoxiong gebilaoxiong force-pushed the improve-normalizeArrayChildren branch from 28bc9a0 to a95f3d0 Compare May 7, 2017 18:20
@gebilaoxiong gebilaoxiong changed the title avoid to be set the key which elements created by slot, fix #5618 Fix: the elements in the component slot are re-created, fix #5618 May 7, 2017
@gebilaoxiong
Copy link
Member Author

gebilaoxiong commented May 7, 2017

Well, it's an interesting thing to name the title

@gebilaoxiong gebilaoxiong force-pushed the improve-normalizeArrayChildren branch 2 times, most recently from c262d62 to 9353514 Compare May 8, 2017 02:32
@@ -45,7 +45,9 @@ function normalizeArrayChildren (children: any, nestedIndex?: string): Array<VNo
last = res[res.length - 1]
// nested
if (Array.isArray(c)) {
res.push.apply(res, normalizeArrayChildren(c, `${nestedIndex || ''}_${i}`))
// avoid elements to be set key, which created by slot
const subNestedIndex = isTrue(c._rendered) ? undefined : `${nestedIndex || ''}_${i}`
Copy link
Contributor

Choose a reason for hiding this comment

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

_renndered is set when process.env.NODE_ENV !== 'production'.

and not only slot, hand-written render functions could have this problem too.

Copy link
Member Author

Choose a reason for hiding this comment

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

@pengchongfu

I did not find any problems in the render function

html:

 <div id="app"></div>

javascript:

var vm = new Vue({
  el: '#app',
  data: {
    count: 1
  },
  render: function (h) {
    var list = []

    for (var i = 0; i < this.count; i++) {
      list.push(h('span', [this.count]))
    }

    return h('div', [
      list,
      h('input', {attrs: { 
        value: 'a',
        type: 'text'
      }})
    ])
  }
})

You can input something in the input box,
then modify the value of count in the console

and input`s value did not changed

Copy link
Member Author

Choose a reason for hiding this comment

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

@pengchongfu

Can you give me a example about hand-written render functions? 🌹

Copy link
Contributor

Choose a reason for hiding this comment

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

@gebilaoxiong , demo, hack... maybe we should ignore it 😂

and _rendered is not set in production, see this line

Copy link
Member Author

Choose a reason for hiding this comment

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

@pengchongfu

💋 Thanks , I'll take a look at this question

@gebilaoxiong gebilaoxiong force-pushed the improve-normalizeArrayChildren branch from f5d07e6 to 094edcd Compare May 8, 2017 05:55
@gebilaoxiong
Copy link
Member Author

gebilaoxiong commented May 8, 2017

@pengchongfu

Now when using render function skip to set the key.

@pengchongfu
Copy link
Contributor

@gebilaoxiong , but all array in hand-written render function will not get a key, I don't know whether it is a good solution.

@gebilaoxiong
Copy link
Member Author

gebilaoxiong commented May 8, 2017

@pengchongfu

I think this happens only when there are nested arrays in children

just like:

children: [
  [vnode, vnode...]
]

In the render of the template, it will be treated as v-for

but slot and render function will also can generate this structure

so we just skip to set the key

@gebilaoxiong
Copy link
Member Author

@pengchongfu wait, I have a better way to solve this problem!

@gebilaoxiong gebilaoxiong force-pushed the improve-normalizeArrayChildren branch from 094edcd to 05da6e4 Compare May 8, 2017 06:57
@gebilaoxiong
Copy link
Member Author

gebilaoxiong commented May 8, 2017

@pengchongfu

I marked the v-for generated elements

Only v-for generated elements will be set key

avoid render function and slot to generate the same structure cause the error

@gebilaoxiong gebilaoxiong changed the title Fix: the elements in the component slot are re-created, fix #5618 Fix: nested children cant be rendered correctly, when render function and slot` to generated nested children, fix #5618 May 8, 2017
@gebilaoxiong gebilaoxiong changed the title Fix: nested children cant be rendered correctly, when render function and slot` to generated nested children, fix #5618 Fix: render function and slot created nested children can`t be rendered correctly, fix #5618 May 8, 2017
@gebilaoxiong
Copy link
Member Author

gebilaoxiong commented May 9, 2017

@yyx990803

Well, it looks like I'm the last one waiting for today

In addition to deleting the code, do we have other ways to solve this problem? 😄

ret[i] = render(val[key], key, i)
vnode = render(val[key], key, i)
vnode.isListItem = true
ret[i] = vnode
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Instead of adding a property to every vnode, we can simply mark the ret array here with something like:

if (isDef(ret)) {
  (ret: any)._isVList = true // work around flow
}

Then check children._isVList in normalizeArrayChildren.

@gebilaoxiong gebilaoxiong force-pushed the improve-normalizeArrayChildren branch from 05da6e4 to 7ff077a Compare May 9, 2017 14:41
@gebilaoxiong gebilaoxiong changed the title Fix: render function and slot created nested children can`t be rendered correctly, fix #5618 Fix: nested child elements can not be updated correctly, fix #5618 May 9, 2017
@gebilaoxiong
Copy link
Member Author

gebilaoxiong commented May 9, 2017

@yyx990803

Fix bug is really a very interesting thing

Maybe my level is not enough, need to chew and get the truth

@yyx990803
Copy link
Member

I pushed a commit to ensure nestedIndex is always passed down for nested arrays. This is necessary in cases where there are two v-for lists nested under non v-for arrays, e.g.

[[[v-for list], ..., [[v-for list]]]

@yyx990803 yyx990803 merged commit f2bd882 into vuejs:dev May 9, 2017
@gebilaoxiong
Copy link
Member Author

understand, thank you very much

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 this pull request may close these issues.

None yet

3 participants