vndb/mithril-vndb
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

[render/render] Simplify updateNodes, fix #2128

Browse source
This commit is contained in:
Pierre-Yves Gérardy 2018-04-20 21:27:22 +02:00 • committed by Pierre-Yves Gérardy
parent dd6ceca31d
commit 9490950c30
2 changed files with 47 additions and 42 deletions
Download patch file Download diff file Expand all files Collapse all files

25
docs/change-log.md
View file

@ -36,18 +36,19 @@
#### Bug fixes #### Bug fixes
- API: `m.route.set()` causes all mount points to be redrawn ([#1592](https://github.com/MithrilJS/mithril.js/pull/1592)) - API: `m.route.set()` causes all mount points to be redrawn ([#1592](https://github.com/MithrilJS/mithril.js/pull/1592))
- API: Using style objects in hyperscript calls will now properly diff style properties from one render to another as opposed to re-writing all element style properties every render. - render/attrs: Using style objects in hyperscript calls will now properly diff style properties from one render to another as opposed to re-writing all element style properties every render.
- core: `addEventListener` and `removeEventListener` are always used to manage event subscriptions, preventing external interference. - render/attrs: `xlink:href` attributes are now correctly removed
- core: Event listeners allocate less memory, swap at low cost, and are properly diffed now when rendered via `m.mount()`/`m.redraw()`. - render/attrs: fix element value don't change if new value is undefined [#2082](https://github.com/MithrilJS/mithril.js/issues/2082)
- core: `Object.prototype` properties can no longer interfere with event listener calls. (https://github.com/MithrilJS/mithril.js/pull/2130)
- API: Event handlers, when set to literally `undefined` (or any non-function), are now correctly removed. - render/core: Render state correctly on select change event [#1916](https://github.com/MithrilJS/mithril.js/issues/1916) ([#1918](https://github.com/MithrilJS/mithril.js/pull/1918) [@robinchew](https://github.com/robinchew), [#2052](https://github.com/MithrilJS/mithril.js/pull/2052))
- core: `xlink:href` attributes are now correctly removed - render/core: fix various updateNodes/removeNodes issues when the pool and fragments are involved [#1990](https://github.com/MithrilJS/mithril.js/issues/1990), [#1991](https://github.com/MithrilJS/mithril.js/issues/1991), [#2003](https://github.com/MithrilJS/mithril.js/issues/2003), [#2021](https://github.com/MithrilJS/mithril.js/pull/2021)
- render: fixed an ommission that caused `oninit` to be called unnecessarily in some cases [#1992](https://github.com/MithrilJS/mithril.js/issues/1992) - render/core: fix crashes when the keyed vnodes with the same `key` had different `tag` values [#2128](https://github.com/MithrilJS/mithril.js/issues/2128) [@JacksonJN](https://github.com/JacksonJN) ([#2130]
- render: Render state correctly on select change event [#1916](https://github.com/MithrilJS/mithril.js/issues/1916) ([#1918](https://github.com/MithrilJS/mithril.js/pull/1918) [@robinchew](https://github.com/robinchew), [#2052](https://github.com/MithrilJS/mithril.js/pull/2052)) - render/events: `addEventListener` and `removeEventListener` are always used to manage event subscriptions, preventing external interference.
- render: fix various updateNodes/removeNodes issues when the pool and fragments are involved [#1990](https://github.com/MithrilJS/mithril.js/issues/1990), [#1991](https://github.com/MithrilJS/mithril.js/issues/1991), [#2003](https://github.com/MithrilJS/mithril.js/issues/2003), [#2021](https://github.com/MithrilJS/mithril.js/pull/2021) - render/events: Event listeners allocate less memory, swap at low cost, and are properly diffed now when rendered via `m.mount()`/`m.redraw()`.
- render: fix element value don't change if new valor is undefined [#2082](https://github.com/MithrilJS/mithril.js/issues/2082) - render/events: `Object.prototype` properties can no longer interfere with event listener calls.
- render: fix typo, missing `)` - render/events: Event handlers, when set to literally `undefined` (or any non-function), are now correctly removed.
- render/hooks: fixed an ommission that caused `oninit` to be called unnecessarily in some cases [#1992](https://github.com/MithrilJS/mithril.js/issues/1992)
- docs: fix typo ([#2104](https://github.com/MithrilJS/mithril.js/pull/2104) [@mikeyb](https://github.com/mikeyb))
--- ---
### v1.1.7 ### v1.1.7

64
render/render.js
View file

@ -51,18 +51,17 @@ module.exports = function($window) {
vnode.state = {} vnode.state = {}
if (vnode.attrs != null) initLifecycle(vnode.attrs, vnode, hooks) if (vnode.attrs != null) initLifecycle(vnode.attrs, vnode, hooks)
switch (tag) { switch (tag) {
case "#": return createText(parent, vnode, nextSibling) case "#": createText(parent, vnode, nextSibling); break
case "<": return createHTML(parent, vnode, ns, nextSibling) case "<": createHTML(parent, vnode, ns, nextSibling); break
case "[": return createFragment(parent, vnode, hooks, ns, nextSibling) case "[": createFragment(parent, vnode, hooks, ns, nextSibling); break
default: return createElement(parent, vnode, hooks, ns, nextSibling) default: createElement(parent, vnode, hooks, ns, nextSibling)
} }
} }
else return createComponent(parent, vnode, hooks, ns, nextSibling) else createComponent(parent, vnode, hooks, ns, nextSibling)
} }
function createText(parent, vnode, nextSibling) { function createText(parent, vnode, nextSibling) {
vnode.dom = $doc.createTextNode(vnode.children) vnode.dom = $doc.createTextNode(vnode.children)
insertNode(parent, vnode.dom, nextSibling) insertNode(parent, vnode.dom, nextSibling)
return vnode.dom
} }
var possibleParents = {caption: "table", thead: "table", tbody: "table", tfoot: "table", tr: "tbody", th: "tr", td: "tr", colgroup: "table", col: "colgroup"} var possibleParents = {caption: "table", thead: "table", tbody: "table", tfoot: "table", tr: "tbody", th: "tr", td: "tr", colgroup: "table", col: "colgroup"}
function createHTML(parent, vnode, ns, nextSibling) { function createHTML(parent, vnode, ns, nextSibling) {
@ -87,7 +86,6 @@ module.exports = function($window) {
fragment.appendChild(child) fragment.appendChild(child)
} }
insertNode(parent, fragment, nextSibling) insertNode(parent, fragment, nextSibling)
return fragment
} }
function createFragment(parent, vnode, hooks, ns, nextSibling) { function createFragment(parent, vnode, hooks, ns, nextSibling) {
var fragment = $doc.createDocumentFragment() var fragment = $doc.createDocumentFragment()
@ -98,7 +96,6 @@ module.exports = function($window) {
vnode.dom = fragment.firstChild vnode.dom = fragment.firstChild
vnode.domSize = fragment.childNodes.length vnode.domSize = fragment.childNodes.length
insertNode(parent, fragment, nextSibling) insertNode(parent, fragment, nextSibling)
return fragment
} }
function createElement(parent, vnode, hooks, ns, nextSibling) { function createElement(parent, vnode, hooks, ns, nextSibling) {
var tag = vnode.tag var tag = vnode.tag
@ -132,7 +129,6 @@ module.exports = function($window) {
setLateAttrs(vnode) setLateAttrs(vnode)
} }
} }
return element
} }
function initComponent(vnode, hooks) { function initComponent(vnode, hooks) {
var sentinel var sentinel
@ -157,15 +153,12 @@ module.exports = function($window) {
function createComponent(parent, vnode, hooks, ns, nextSibling) { function createComponent(parent, vnode, hooks, ns, nextSibling) {
initComponent(vnode, hooks) initComponent(vnode, hooks)
if (vnode.instance != null) { if (vnode.instance != null) {
var element = createNode(parent, vnode.instance, hooks, ns, nextSibling) createNode(parent, vnode.instance, hooks, ns, nextSibling)
vnode.dom = vnode.instance.dom vnode.dom = vnode.instance.dom
vnode.domSize = vnode.dom != null ? vnode.instance.domSize : 0 vnode.domSize = vnode.dom != null ? vnode.instance.domSize : 0
insertNode(parent, element, nextSibling)
return element
} }
else { else {
vnode.domSize = 0 vnode.domSize = 0
return $emptyFragment
} }
} }
@ -241,10 +234,20 @@ module.exports = function($window) {
// parts are actually implemented as only two loops, one for the first two parts, and one for // parts are actually implemented as only two loops, one for the first two parts, and one for
// the other two. I'm not sure it wins us anything except maybe a few bytes of file size. // the other two. I'm not sure it wins us anything except maybe a few bytes of file size.
// ## DOM node operations // ## Finding the next sibling.
//
// `updateNode()` and `createNode()` expect a nextSibling parameter. When the list is being
// traversed top-down, the next sibling must be looked for in the old list using
// `getNextSibling()`. In the other scenarios (swaps, upwards traversal, map-based diff),
// the new vnodes list is traversed upwards, which enables us to fetch the `nextSibling`
// parameter on the go (set to the last `v.dom` when present).
// ## DOM node moves
// //
// In most cases `updateNode()` and `createNode()` perform the DOM operations. However, // In most cases `updateNode()` and `createNode()` perform the DOM operations. However,
// this is not the case if the node moved (second and fourth part of the diff algo). // this is not the case if the node moved (second and fourth part of the diff algo). We move
// the old DOM nodes before updateNode runs because it enables us to use the cached `nextSibling`
// variable rather than fetching it using `getNextSibling()`.
// //
// The fourth part of the diff currently inserts nodes unconditionally, leading to issues // The fourth part of the diff currently inserts nodes unconditionally, leading to issues
// like #1791 and #1999. We need to be smarter about those situations where adjascent old // like #1791 and #1999. We need to be smarter about those situations where adjascent old
@ -300,14 +303,15 @@ module.exports = function($window) {
oldStart++, start++ oldStart++, start++
updateNode(parent, o, v, hooks, getNextSibling(old, oldStart, nextSibling), ns) updateNode(parent, o, v, hooks, getNextSibling(old, oldStart, nextSibling), ns)
} else { } else {
o = old[oldEnd] v = vnodes[end]
if (o === v) oldEnd--, start++ if (o === v) oldStart++, end--
else if (o == null) oldEnd-- else if (o == null) oldStart++
else if (v == null) start++ else if (v == null) end--
else if (o.key === v.key) { else if (o.key === v.key) {
updateNode(parent, o, v, hooks, getNextSibling(old, oldEnd + 1, nextSibling), ns) oldStart++
if (start < end) insertNode(parent, toFragment(v), getNextSibling(old, oldStart, nextSibling)) if (start < end--) insertNode(parent, toFragment(o), nextSibling)
oldEnd--, start++ updateNode(parent, o, v, hooks, nextSibling, ns)
if(v.dom != null) nextSibling = v.dom
} }
else break else break
} }
@ -319,8 +323,8 @@ module.exports = function($window) {
else if (o == null) oldEnd-- else if (o == null) oldEnd--
else if (v == null) end-- else if (v == null) end--
else if (o.key === v.key) { else if (o.key === v.key) {
updateNode(parent, o, v, hooks, getNextSibling(old, oldEnd + 1, nextSibling), ns) updateNode(parent, o, v, hooks, nextSibling, ns)
if (o.dom != null) nextSibling = o.dom if (v.dom != null) nextSibling = v.dom
oldEnd--, end-- oldEnd--, end--
} else { } else {
if (!map) map = getKeyMap(old, oldEnd) if (!map) map = getKeyMap(old, oldEnd)
@ -328,13 +332,13 @@ module.exports = function($window) {
var oldIndex = map[v.key] var oldIndex = map[v.key]
if (oldIndex != null) { if (oldIndex != null) {
o = old[oldIndex] o = old[oldIndex]
updateNode(parent, o, v, hooks, getNextSibling(old, oldEnd + 1, nextSibling), ns) insertNode(parent, toFragment(o), nextSibling)
insertNode(parent, toFragment(v), nextSibling) updateNode(parent, o, v, hooks, nextSibling, ns)
o.skip = true o.skip = true
if (o.dom != null) nextSibling = o.dom if (v.dom != null) nextSibling = v.dom
} else { } else {
var dom = createNode(parent, v, hooks, ns, nextSibling) createNode(parent, v, hooks, ns, nextSibling)
nextSibling = dom if (v.dom != null) nextSibling = v.dom
} }
} }
end-- end--
@ -474,7 +478,7 @@ module.exports = function($window) {
} }
function insertNode(parent, dom, nextSibling) { function insertNode(parent, dom, nextSibling) {
if (nextSibling) parent.insertBefore(dom, nextSibling) if (nextSibling != null) parent.insertBefore(dom, nextSibling)
else parent.appendChild(dom) else parent.appendChild(dom)
} }