Component instances are always fragments - fixes #1713 #1720
Conversation
render/render.js
Outdated
| initComponent(vnode, hooks) | ||
| if (vnode.instance != null) { | ||
| if (vnode.instance === vnode) throw Error("A view cannot return the vnode it received as arguments") | ||
| if (vnode.instance[0] === vnode) throw Error("A view cannot return the vnode it received as arguments") |
There was a problem hiding this comment.
This isn't passing the associated tests. Any insights appreciated.
There was a problem hiding this comment.
That's odd... it should work... Looking into it.
There was a problem hiding this comment.
vnode.normalize() turns the array into a fragment, it should be vnode.instance.children[0].
Note that this is the kind of change that will most likely affect perf negatively (adding one updateNodes() per component to the call stack), even if we define vnode.instance = Vnode.normalize(output).children and change downstream methods to deal with it.
barneycarroll
changed the title
|
@barneycarroll LGTM, mod (lack of) tests. Given the documented grief it caused in #1713, tests would be preferred here. |
dead-claudia
added
the
Status: Needs test case
|
Actually, this is only a partial fix, in that it adds another special case for the component Adding a check for |
|
Intriguing insight @pygy. Can it really be that simple? @isiahmeadows 👍 |
|
@barneycarroll I think so, but I'll need to look in depth. #1724 describes in depth what I meant by " keys out of lists further down the tree" in my previous comment. |
|
@pygy that's a really nasty test case. Try restructuring that code to meet author-land requirements (eg fragments, changed keys on single element) and it reveals further difficulties… |
|
@barneycarroll I had read the #1724 code too fast, it was a case of mixed keyed/non-keyed nodes. |
Attempt to fix MithrilJS#1713, superseding MithrilJS#1720, as per @pygy's insight here MithrilJS#1720 (comment)
|
Closing this in favour of #1725 |
3 participants
By ensuring component instances are always lists - even if the view output is a single node -
keycan be used for identification semantics universally. This also makes vnode traversal easier, since it standardizes component instance type.There component tests for self-returning vnodes are failing for reasons I don't understand.