From 1dd5fe310127557ad19c679f9115a05981e0518e Mon Sep 17 00:00:00 2001 From: Pierre-Yves Gerardy Date: Sun, 19 Feb 2017 22:53:46 +0100 Subject: [PATCH 1/6] Tests: Some more render/tests/test-component.js refactoring --- render/tests/test-component.js | 307 ++++++++++++++++++--------------- 1 file changed, 171 insertions(+), 136 deletions(-) diff --git a/render/tests/test-component.js b/render/tests/test-component.js index 64cd6fcb..6398eaa0 100644 --- a/render/tests/test-component.js +++ b/render/tests/test-component.js @@ -656,6 +656,130 @@ o.spec("component", function() { o(vnode.dom).notEquals(updated.dom) }) + o("lifecycle timing megatest (for a single component)", function() { + var methods = { + view: o.spy(function() { + return "" + }) + } + var attrs = {} + var hooks = [ + "oninit", "oncreate", "onbeforeupdate", + "onupdate", "onbeforeremove", "onremove" + ] + hooks.forEach(function(hook) { + // the `attrs` hooks are called before the component ones + attrs[hook] = o.spy(function() { + o(attrs[hook].callCount).equals(methods[hook].callCount + 1) + }) + methods[hook] = o.spy(function() { + o(attrs[hook].callCount).equals(methods[hook].callCount) + }) + }) + + var component = createComponent(methods) + + o(methods.view.callCount).equals(0) + o(methods.oninit.callCount).equals(0) + o(methods.oncreate.callCount).equals(0) + o(methods.onbeforeupdate.callCount).equals(0) + o(methods.onupdate.callCount).equals(0) + o(methods.onbeforeremove.callCount).equals(0) + o(methods.onremove.callCount).equals(0) + + hooks.forEach(function(hook) { + o(attrs[hook].callCount).equals(methods[hook].callCount)(hook) + }) + + render(root, [{tag: component, attrs: attrs}]) + + o(methods.view.callCount).equals(1) + o(methods.oninit.callCount).equals(1) + o(methods.oncreate.callCount).equals(1) + o(methods.onbeforeupdate.callCount).equals(0) + o(methods.onupdate.callCount).equals(0) + o(methods.onbeforeremove.callCount).equals(0) + o(methods.onremove.callCount).equals(0) + + hooks.forEach(function(hook) { + o(attrs[hook].callCount).equals(methods[hook].callCount)(hook) + }) + + render(root, [{tag: component, attrs: attrs}]) + + o(methods.view.callCount).equals(2) + o(methods.oninit.callCount).equals(1) + o(methods.oncreate.callCount).equals(1) + o(methods.onbeforeupdate.callCount).equals(1) + o(methods.onupdate.callCount).equals(1) + o(methods.onbeforeremove.callCount).equals(0) + o(methods.onremove.callCount).equals(0) + + hooks.forEach(function(hook) { + o(attrs[hook].callCount).equals(methods[hook].callCount)(hook) + }) + + render(root, []) + + o(methods.view.callCount).equals(2) + o(methods.oninit.callCount).equals(1) + o(methods.oncreate.callCount).equals(1) + o(methods.onbeforeupdate.callCount).equals(1) + o(methods.onupdate.callCount).equals(1) + o(methods.onbeforeremove.callCount).equals(1) + o(methods.onremove.callCount).equals(1) + + hooks.forEach(function(hook) { + o(attrs[hook].callCount).equals(methods[hook].callCount)(hook) + }) + }) + o("hook state and arguments validation", function(){ + var methods = { + view: o.spy(function(vnode) { + o(this).equals(vnode.state) + return "" + }) + } + var attrs = {} + var hooks = [ + "oninit", "oncreate", "onbeforeupdate", + "onupdate", "onbeforeremove", "onremove" + ] + hooks.forEach(function(hook) { + attrs[hook] = o.spy(function(vnode){ + // #1638, the assertion passes for `oninit` because both values are wrong. + if (hook !== 'oncreate') o(this).equals(vnode.state)(hook) + }) + methods[hook] = o.spy(function(vnode){ + o(this).equals(vnode.state) + }) + }) + + var component = createComponent(methods) + + render(root, [{tag: component, attrs: attrs}]) + render(root, [{tag: component, attrs: attrs}]) + render(root, []) + + hooks.forEach(function(hook) { + // #1638 + if (hook !== "oninit" && hook !== 'oncreate') o(attrs[hook].this).equals(methods.view.this)(hook) + o(methods[hook].this).equals(methods.view.this)(hook) + }) + + o(methods.view.args.length).equals(1) + o(methods.oninit.args.length).equals(1) + o(methods.oncreate.args.length).equals(1) + o(methods.onbeforeupdate.args.length).equals(2) + o(methods.onupdate.args.length).equals(1) + o(methods.onbeforeremove.args.length).equals(1) + o(methods.onremove.args.length).equals(1) + + hooks.forEach(function(hook) { + o(methods[hook].args.length).equals(attrs[hook].args.length)(hook) + }) + + }) }) o.spec("state", function() { o("initializes state", function() { @@ -675,7 +799,7 @@ o.spec("component", function() { o(vnode.state.data).equals(data) } }) - o('state "copy" is shallow', function() { + o('state proxies to the component object/prototype', function() { var called = 0 var body = {a: 1} var data = [body] @@ -698,11 +822,10 @@ o.spec("component", function() { }) }) o.spec("Tests specific to certain component kinds", function() { - - o.spec("POJO state", function() { - o("copies state", function() { + o.spec("state", function() { + o("POJO", function() { var called = 0 - var data = {a: 1} + var data = {} var component = { data: data, oninit: init, @@ -721,142 +844,54 @@ o.spec("component", function() { o(vnode.state.x).equals(1) } }) - }) + o("Constructible", function() { + var oninit = o.spy() + var component = o.spy(function(vnode){ + o(vnode.state).equals(null) + // #1638 + // o(oninit.callCount).equals(0) + }) + var view = o.spy(function(){ + o(this instanceof component).equals(true) + return "" + }) + component.prototype.view = view + component.prototype.oninit = oninit - o("Classes can be used as components", function() { - function MyComponent(vnode){ - o(vnode.state).equals(null) - } - var proto = MyComponent.prototype + var context - var context + render(root, [{tag: component, attrs: {oninit: oninit}}]) + render(root, [{tag: component, attrs: {oninit: oninit}}]) + render(root, []) - proto.oninit = o.spy(function(vnode) { - o(this).equals(vnode.state) - context = this + o(component.callCount).equals(1) + o(oninit.callCount).equals(2) + o(view.callCount).equals(2) }) - proto.oncreate = o.spy() - proto.onbeforeupdate = o.spy() - proto.onupdate = o.spy() - proto.onbeforeremove = o.spy() - proto.onremove = o.spy() - proto.view = o.spy(function() { - return "" + o("Closure", function() { + var state + var oninit = o.spy() + var view = o.spy(function() { + o(this).equals(state) + return "" + }) + var component = o.spy(function(vnode) { + o(vnode.state).equals(null) + // #1638 + // o(oninit.callCount).equals(0) + return state = { + view: view + } + }) + + render(root, [{tag: component, attrs: {oninit: oninit}}]) + render(root, [{tag: component, attrs: {oninit: oninit}}]) + render(root, []) + + o(component.callCount).equals(1) + o(oninit.callCount).equals(1) + o(view.callCount).equals(2) }) - - render(root, [{tag: MyComponent}]) - - o(context instanceof MyComponent).equals(true) - - o(proto.view.callCount).equals(1) - o(proto.oncreate.callCount).equals(1) - o(proto.onbeforeupdate.callCount).equals(0) - o(proto.onupdate.callCount).equals(0) - o(proto.onbeforeremove.callCount).equals(0) - o(proto.onremove.callCount).equals(0) - - render(root, [{tag: MyComponent}]) - - o(proto.view.callCount).equals(2) - o(proto.oncreate.callCount).equals(1) - o(proto.onbeforeupdate.callCount).equals(1) - o(proto.onupdate.callCount).equals(1) - o(proto.onbeforeremove.callCount).equals(0) - o(proto.onremove.callCount).equals(0) - - render(root, []) - - o(proto.view.callCount).equals(2) - o(proto.oncreate.callCount).equals(1) - o(proto.onbeforeupdate.callCount).equals(1) - o(proto.onupdate.callCount).equals(1) - o(proto.onbeforeremove.callCount).equals(1) - o(proto.onremove.callCount).equals(1) - - o(proto.oninit.this).equals(context) - o(proto.view.this).equals(context) - o(proto.oncreate.this).equals(context) - o(proto.onbeforeupdate.this).equals(context) - o(proto.onupdate.this).equals(context) - o(proto.onbeforeremove.this).equals(context) - o(proto.onremove.this).equals(context) - - o(proto.oninit.args.length).equals(1) - o(proto.view.args.length).equals(1) - o(proto.oncreate.args.length).equals(1) - o(proto.onbeforeupdate.args.length).equals(2) - o(proto.onupdate.args.length).equals(1) - o(proto.onbeforeremove.args.length).equals(1) - o(proto.onremove.args.length).equals(1) - }) - o("Closure functions can be used as components", function() { - var state, context - function component(vnode) { - o(vnode.state).equals(null) - - return state = { - oninit: o.spy(function(vnode) { - o(this).equals(vnode.state) - context = this - }), - oncreate: o.spy(), - onbeforeupdate: o.spy(), - onupdate: o.spy(), - onbeforeremove: o.spy(), - onremove: o.spy(), - view: o.spy(function() { - return "" - }) - } - } - - render(root, [{tag: component}]) - - o(state).equals(context) - - o(state.oninit.callCount).equals(1) - o(state.view.callCount).equals(1) - o(state.oncreate.callCount).equals(1) - o(state.onbeforeupdate.callCount).equals(0) - o(state.onupdate.callCount).equals(0) - o(state.onbeforeremove.callCount).equals(0) - o(state.onremove.callCount).equals(0) - - render(root, [{tag: component}]) - - o(state.oninit.callCount).equals(1) - o(state.view.callCount).equals(2) - o(state.oncreate.callCount).equals(1) - o(state.onbeforeupdate.callCount).equals(1) - o(state.onupdate.callCount).equals(1) - o(state.onbeforeremove.callCount).equals(0) - o(state.onremove.callCount).equals(0) - - render(root, []) - - o(state.oninit.callCount).equals(1) - o(state.view.callCount).equals(2) - o(state.oncreate.callCount).equals(1) - o(state.onbeforeupdate.callCount).equals(1) - o(state.onupdate.callCount).equals(1) - o(state.onbeforeremove.callCount).equals(1) - o(state.onremove.callCount).equals(1) - - o(state.oninit.this).equals(state) - o(state.view.this).equals(state) - o(state.oncreate.this).equals(state) - o(state.onbeforeupdate.this).equals(state) - o(state.onupdate.this).equals(state) - o(state.onbeforeremove.this).equals(state) - o(state.onremove.this).equals(state) - - o(state.oninit.args.length).equals(1) - o(state.view.args.length).equals(1) - o(state.oncreate.args.length).equals(1) - o(state.onbeforeupdate.args.length).equals(2) - o(state.onupdate.args.length).equals(1) - o(state.onbeforeremove.args.length).equals(1) - o(state.onremove.args.length).equals(1) }) }) }) From fc038f9d859e5e458fb50938f7ef446337d5d620 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Gerardy Date: Sun, 19 Feb 2017 22:57:31 +0100 Subject: [PATCH 2/6] Tests: enable tests for #1638 --- render/tests/test-component.js | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/render/tests/test-component.js b/render/tests/test-component.js index 6398eaa0..212905d2 100644 --- a/render/tests/test-component.js +++ b/render/tests/test-component.js @@ -747,8 +747,7 @@ o.spec("component", function() { ] hooks.forEach(function(hook) { attrs[hook] = o.spy(function(vnode){ - // #1638, the assertion passes for `oninit` because both values are wrong. - if (hook !== 'oncreate') o(this).equals(vnode.state)(hook) + o(this).equals(vnode.state)(hook) }) methods[hook] = o.spy(function(vnode){ o(this).equals(vnode.state) @@ -762,8 +761,7 @@ o.spec("component", function() { render(root, []) hooks.forEach(function(hook) { - // #1638 - if (hook !== "oninit" && hook !== 'oncreate') o(attrs[hook].this).equals(methods.view.this)(hook) + o(attrs[hook].this).equals(methods.view.this)(hook) o(methods[hook].this).equals(methods.view.this)(hook) }) @@ -848,8 +846,7 @@ o.spec("component", function() { var oninit = o.spy() var component = o.spy(function(vnode){ o(vnode.state).equals(null) - // #1638 - // o(oninit.callCount).equals(0) + o(oninit.callCount).equals(0) }) var view = o.spy(function(){ o(this instanceof component).equals(true) @@ -877,8 +874,7 @@ o.spec("component", function() { }) var component = o.spy(function(vnode) { o(vnode.state).equals(null) - // #1638 - // o(oninit.callCount).equals(0) + o(oninit.callCount).equals(0) return state = { view: view } From 7668ddd120c00f4152cdaf5591043e5ce6bff70e Mon Sep 17 00:00:00 2001 From: Pierre-Yves Gerardy Date: Sun, 19 Feb 2017 23:00:34 +0100 Subject: [PATCH 3/6] fix #1638 --- render/render.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/render/render.js b/render/render.js index ea015ebd..eb31229b 100644 --- a/render/render.js +++ b/render/render.js @@ -20,8 +20,8 @@ module.exports = function($window) { } function createNode(parent, vnode, hooks, ns, nextSibling) { var tag = vnode.tag - if (vnode.attrs != null) initLifecycle(vnode.attrs, vnode, hooks) if (typeof tag === "string") { + if (vnode.attrs != null) initLifecycle(vnode.attrs, vnode, hooks) switch (tag) { case "#": return createText(parent, vnode, nextSibling) case "<": return createHTML(parent, vnode, nextSibling) @@ -116,6 +116,7 @@ module.exports = function($window) { sentinel.$$reentrantLock$$ = true } + if (vnode.attrs != null) initLifecycle(vnode.attrs, vnode, hooks) initLifecycle(vnode.state, vnode, hooks) vnode.instance = Vnode.normalize(vnode.state.view(vnode)) sentinel.$$reentrantLock$$ = null From e496f7bfa6e222fa07fea84effa1c55ddf127069 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Gerardy Date: Mon, 20 Feb 2017 16:46:34 +0100 Subject: [PATCH 4/6] Test: ensure that recycled components get a fresh state --- render/tests/test-component.js | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/render/tests/test-component.js b/render/tests/test-component.js index 212905d2..0ff4e07e 100644 --- a/render/tests/test-component.js +++ b/render/tests/test-component.js @@ -776,7 +776,28 @@ o.spec("component", function() { hooks.forEach(function(hook) { o(methods[hook].args.length).equals(attrs[hook].args.length)(hook) }) + }) + o("recycled components get a fresh state", function() { + var step = 0 + var firstState + var view = o.spy(function(vnode) { + if (step === 0) { + firstState = vnode.state + } else { + o(vnode.state).notEquals(firstState) + } + return {tag: 'div'} + }) + var component = createComponent({view: view}) + render(root, [{tag: 'div', children: [{tag: component, key: 1}]}]) + var child = root.firstChild.firstChild + render(root, []) + step = 1 + render(root, [{tag: 'div', children: [{tag: component, key: 1}]}]) + + o(child).equals(root.firstChild.firstChild) + o(view.callCount).equals(2) }) }) o.spec("state", function() { From 6a7144fc89b63944110ca4ce3169adcbbfee34a0 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Gerardy Date: Mon, 20 Feb 2017 20:29:18 +0100 Subject: [PATCH 5/6] Tests: add a test for onbeforeupdate and recycled nodes --- render/tests/test-onbeforeupdate.js | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/render/tests/test-onbeforeupdate.js b/render/tests/test-onbeforeupdate.js index cf16dd1c..93643409 100644 --- a/render/tests/test-onbeforeupdate.js +++ b/render/tests/test-onbeforeupdate.js @@ -120,6 +120,21 @@ o.spec("onbeforeupdate", function() { o(count).equals(1) }) + o("doesn't fire on recycled nodes", function() { + var onbeforeupdate = o.spy() + var vnodes = [{tag: "div", key: 1}] + var temp = [] + var updated = [{tag: "div", key: 1, attrs: {onbeforeupdate: onbeforeupdate}}] + + render(root, vnodes) + render(root, temp) + render(root, updated) + + o(vnodes[0].dom).equals(updated[0].dom) + o(updated[0].dom.nodeName).equals("DIV") + o(onbeforeupdate.callCount).equals(0) + }) + components.forEach(function(cmp){ o.spec(cmp.kind, function(){ var createComponent = cmp.create From 3e7649ef06faa7dbdb7ca69a61dada993e8b70ee Mon Sep 17 00:00:00 2001 From: Pierre-Yves Gerardy Date: Fri, 24 Feb 2017 09:26:26 +0100 Subject: [PATCH 6/6] Fix recycled components initialization fix #1641 --- render/render.js | 31 +++++++++++++++++++------------ 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/render/render.js b/render/render.js index eb31229b..19ca6ebe 100644 --- a/render/render.js +++ b/render/render.js @@ -100,7 +100,7 @@ module.exports = function($window) { } return element } - function createComponent(parent, vnode, hooks, ns, nextSibling) { + function initComponent(vnode, hooks) { var sentinel if (typeof vnode.tag === "function") { vnode.state = null @@ -120,7 +120,9 @@ module.exports = function($window) { initLifecycle(vnode.state, vnode, hooks) vnode.instance = Vnode.normalize(vnode.state.view(vnode)) sentinel.$$reentrantLock$$ = null - + } + function createComponent(parent, vnode, hooks, ns, nextSibling) { + initComponent(vnode, hooks) if (vnode.instance != null) { if (vnode.instance === vnode) throw Error("A view cannot return the vnode it received as arguments") var element = createNode(parent, vnode.instance, hooks, ns, nextSibling) @@ -233,11 +235,12 @@ module.exports = function($window) { if (oldTag === tag) { vnode.state = old.state vnode.events = old.events - if (shouldUpdate(vnode, old)) return - if (vnode.attrs != null) { - updateLifecycle(vnode.attrs, vnode, hooks, recycling) - } + if (!recycling && shouldNotUpdate(vnode, old)) return if (typeof oldTag === "string") { + if (vnode.attrs != null) { + if (recycling) initLifecycle(vnode.attrs, vnode, hooks) + else updateLifecycle(vnode.attrs, vnode, hooks) + } switch (oldTag) { case "#": updateText(old, vnode); break case "<": updateHTML(parent, old, vnode, nextSibling); break @@ -307,8 +310,13 @@ module.exports = function($window) { } } function updateComponent(parent, old, vnode, hooks, nextSibling, recycling, ns) { - vnode.instance = Vnode.normalize(vnode.state.view(vnode)) - updateLifecycle(vnode.state, vnode, hooks, recycling) + if (recycling) { + initComponent(vnode, hooks) + } else { + vnode.instance = Vnode.normalize(vnode.state.view(vnode)) + if (vnode.attrs != null) updateLifecycle(vnode.attrs, vnode, hooks) + updateLifecycle(vnode.state, vnode, hooks) + } if (vnode.instance != null) { if (old.instance == null) createNode(parent, vnode.instance, hooks, ns, nextSibling) else updateNode(parent, old.instance, vnode.instance, hooks, nextSibling, recycling, ns) @@ -562,11 +570,10 @@ module.exports = function($window) { if (typeof source.oninit === "function") source.oninit.call(vnode.state, vnode) if (typeof source.oncreate === "function") hooks.push(source.oncreate.bind(vnode.state, vnode)) } - function updateLifecycle(source, vnode, hooks, recycling) { - if (recycling) initLifecycle(source, vnode, hooks) - else if (typeof source.onupdate === "function") hooks.push(source.onupdate.bind(vnode.state, vnode)) + function updateLifecycle(source, vnode, hooks) { + if (typeof source.onupdate === "function") hooks.push(source.onupdate.bind(vnode.state, vnode)) } - function shouldUpdate(vnode, old) { + function shouldNotUpdate(vnode, old) { var forceVnodeUpdate, forceComponentUpdate if (vnode.attrs != null && typeof vnode.attrs.onbeforeupdate === "function") forceVnodeUpdate = vnode.attrs.onbeforeupdate.call(vnode.state, vnode, old) if (typeof vnode.tag !== "string" && typeof vnode.state.onbeforeupdate === "function") forceComponentUpdate = vnode.state.onbeforeupdate(vnode, old)