From ead2e8ac0b95047f7a359f730490fad9d2f82922 Mon Sep 17 00:00:00 2001 From: Leo Horie Date: Thu, 5 May 2016 23:20:31 -0400 Subject: [PATCH] shouldUpdate, component oninit order fix --- README.md | 4 +- examples/dbmonster/angular/app.js | 2 +- index.html | 72 -------- ospec/ospec.js | 2 +- render/hyperscript.js | 2 +- render/render.js | 23 ++- render/tests/index.html | 1 + render/tests/test-component.js | 15 ++ render/tests/test-shouldUpdate.js | 289 ++++++++++++++++++++++++++++++ render/tests/test-trust.js | 1 - 10 files changed, 329 insertions(+), 82 deletions(-) delete mode 100644 index.html create mode 100644 render/tests/test-shouldUpdate.js diff --git a/README.md b/README.md index aa8af271..c7e9c271 100644 --- a/README.md +++ b/README.md @@ -6,13 +6,13 @@ This rewrite aims to fix longstanding API design issues, significantly improve p ## Status -Code still is in flux. Most notably, components and thunks (`{subtree: "retain"}`) are currently not implemented yet and there are several use cases that still need to be polished. DO NOT USE IN PRODUCTION YET! +Code still is in flux. Most notably, thunks (`{subtree: "retain"}`) are currently not implemented yet and there are several use cases that still need to be polished. DO NOT USE IN PRODUCTION YET! Some examples of usage can be found in the [examples](examples) folder. [ThreadItJS](http://cdn.rawgit.com/lhorie/mithril.js/rewrite/examples/threaditjs/index.html) has the largest API surface coverage and comments indicating pending issues in framework usability. Note that the APIs those examples use may not become the final public API points in v1.0. ## Performance -Mithril's virtual DOM engine is now less than 400 lines of well organized code and it implements a modern search space reduction diff algorithm and a DOM recycling mechanism, which translate to top-of-class performance. See the [dbmon implementation (non-optimized)](http://cdn.rawgit.com/lhorie/mithril.js/rewrite/examples/dbmonster/mithril/index.html) (for comparison, here are optimized dbmon implementations for [React v15.0.2](http://cdn.rawgit.com/lhorie/mithril.js/rewrite/examples/dbmonster/react/index.html) and [Angular v2.0.0-beta.17](http://cdn.rawgit.com/lhorie/mithril.js/rewrite/examples/dbmonster/angular/index.html)). +Mithril's virtual DOM engine is now around 400 lines of well organized code and it implements a modern search space reduction diff algorithm and a DOM recycling mechanism, which translate to top-of-class performance. See the [dbmon implementation (non-optimized)](http://cdn.rawgit.com/lhorie/mithril.js/rewrite/examples/dbmonster/mithril/index.html) (for comparison, here are optimized dbmon implementations for [React v15.0.2](http://cdn.rawgit.com/lhorie/mithril.js/rewrite/examples/dbmonster/react/index.html) and [Angular v2.0.0-beta.17](http://cdn.rawgit.com/lhorie/mithril.js/rewrite/examples/dbmonster/angular/index.html)). ## Lifecycle methods and Animation Support diff --git a/examples/dbmonster/angular/app.js b/examples/dbmonster/angular/app.js index 3c366b7b..8bae1cf2 100644 --- a/examples/dbmonster/angular/app.js +++ b/examples/dbmonster/angular/app.js @@ -14,7 +14,7 @@ var AppComponent = ng.core.Component({selector: "my-app"}) "" + "{{db.lastSample.nbQueries}}" + "" + - "" + + "" + "{{q.formatElapsed}}" + "
" + "
{{q.query}}
" + diff --git a/index.html b/index.html deleted file mode 100644 index c27d0cd0..00000000 --- a/index.html +++ /dev/null @@ -1,72 +0,0 @@ - - - - - - - - - - - - - \ No newline at end of file diff --git a/ospec/ospec.js b/ospec/ospec.js index 71759de8..0bc6c73a 100644 --- a/ospec/ospec.js +++ b/ospec/ospec.js @@ -189,7 +189,7 @@ module.exports = new function init() { for (var i = 0, r; r = results[i]; i++) { if (!r.pass) console.error(r.context + ": " + highlight(r.message) + "\n\n" + r.error.match(/^(?:(?!Error|[\/\\]ospec[\/\\]ospec\.js).)*$/m) + "\n\n", hasProcess ? "" : "color:red", hasProcess ? "" : "color:black") } - console.log(results.length + " tests completed in " + Math.round(new Date - start) + "ms") + console.log(results.length + " assertions completed in " + Math.round(new Date - start) + "ms") } return o diff --git a/render/hyperscript.js b/render/hyperscript.js index e1dd49d9..58f8f3d5 100644 --- a/render/hyperscript.js +++ b/render/hyperscript.js @@ -63,7 +63,7 @@ function hyperscript(selector) { } if (typeof selector === "string") return selectorCache[selector](attrs || {}, Node.normalizeChildren(children)) - return Node(selector, attrs && attrs.key, attrs, Node.normalizeChildren(children), undefined, undefined) + return Node(selector, attrs && attrs.key, attrs || {}, Node.normalizeChildren(children), undefined, undefined) } function changeNS(ns, vnode) { diff --git a/render/render.js b/render/render.js index 53bb87b4..b2984c83 100644 --- a/render/render.js +++ b/render/render.js @@ -86,8 +86,8 @@ module.exports = function($window, onevent) { return element } function createComponent(vnode, hooks) { - vnode.instance = Node.normalize(vnode.tag.view(vnode)) initLifecycle(vnode.tag, vnode, hooks) + vnode.instance = Node.normalize(vnode.tag.view.call(vnode, vnode)) var element = createNode(vnode.instance, hooks) vnode.dom = vnode.instance.dom vnode.domSize = vnode.instance.domSize @@ -161,7 +161,10 @@ module.exports = function($window, onevent) { var oldTag = old.tag, tag = vnode.tag if (oldTag === tag) { vnode.state = old.state - if (vnode.attrs != null) updateLifecycle(vnode.attrs, vnode, hooks, recycling) + if (shouldUpdate(vnode, old)) return + if (vnode.attrs != null) { + updateLifecycle(vnode.attrs, vnode, hooks, recycling) + } if (typeof oldTag === "string") { switch (oldTag) { case "#": updateText(old, vnode); break @@ -218,7 +221,7 @@ module.exports = function($window, onevent) { } } function updateComponent(parent, old, vnode, hooks, nextSibling, recycling) { - vnode.instance = Node.normalize(vnode.tag.view(vnode)) + vnode.instance = Node.normalize(vnode.tag.view.call(vnode, vnode)) updateLifecycle(vnode.tag, vnode, hooks, recycling) updateNode(parent, old.instance, vnode.instance, hooks, nextSibling, recycling) vnode.dom = vnode.instance.dom @@ -373,7 +376,7 @@ module.exports = function($window, onevent) { } } function isLifecycleMethod(attr) { - return attr === "oninit" || attr === "oncreate" || attr === "onupdate" || attr === "onremove" || attr === "onbeforeremove" + return attr === "oninit" || attr === "oncreate" || attr === "onupdate" || attr === "onremove" || attr === "onbeforeremove" || attr === "shouldUpdate" } function isAttribute(attr) { return attr === "href" || attr === "list" || attr === "form"// || attr === "type" || attr === "width" || attr === "height" @@ -405,6 +408,18 @@ module.exports = function($window, onevent) { if (recycling) initLifecycle(source, vnode, hooks) else if (source.onupdate != null) hooks.push(source.onupdate.bind(vnode, vnode)) } + function shouldUpdate(vnode, old) { + var forceVnodeUpdate, forceComponentUpdate + if (vnode.attrs != null && typeof vnode.attrs.shouldUpdate === "function") forceVnodeUpdate = vnode.attrs.shouldUpdate(vnode, old) + if (typeof vnode.tag !== "string" && typeof vnode.tag.shouldUpdate === "function") forceComponentUpdate = vnode.tag.shouldUpdate(vnode, old) + if (!(forceVnodeUpdate === undefined && forceComponentUpdate === undefined) && !forceVnodeUpdate && !forceComponentUpdate) { + vnode.dom = old.dom + vnode.domSize = old.domSize + vnode.instance = old.instance + return true + } + return false + } function render(dom, vnodes) { var hooks = [] diff --git a/render/tests/index.html b/render/tests/index.html index baff8172..0082e240 100644 --- a/render/tests/index.html +++ b/render/tests/index.html @@ -32,6 +32,7 @@ + diff --git a/render/tests/test-component.js b/render/tests/test-component.js index e4c47f22..c9f72ae4 100644 --- a/render/tests/test-component.js +++ b/render/tests/test-component.js @@ -257,6 +257,21 @@ o.spec("component", function() { o(root.firstChild.attributes["id"].nodeValue).equals("a") o(root.firstChild.firstChild.nodeValue).equals("b") }) + o("calls oninit before view", function() { + var viewCalled = false + + render(root, { + tag: { + view: function() { + viewCalled = true + return [{tag: "div", attrs: {id: "a"}, text: "b"}] + }, + oninit: function(vnode) { + o(viewCalled).equals(false) + }, + } + }) + }) o("calls oncreate", function() { var called = 0 var component = { diff --git a/render/tests/test-shouldUpdate.js b/render/tests/test-shouldUpdate.js new file mode 100644 index 00000000..b4dbc57a --- /dev/null +++ b/render/tests/test-shouldUpdate.js @@ -0,0 +1,289 @@ +"use strict" + +var o = require("../../ospec/ospec") +var domMock = require("../../test-utils/domMock") +var vdom = require("../../render/render") + +o.spec("shouldUpdate", function() { + var $window, root, render + o.beforeEach(function() { + $window = domMock() + root = $window.document.createElement("div") + render = vdom($window).render + }) + + o("prevents update in element", function() { + var shouldUpdate = function() {return false} + var vnode = {tag: "div", attrs: {id: "a", shouldUpdate: shouldUpdate}} + var updated = {tag: "div", attrs: {id: "b", shouldUpdate: shouldUpdate}} + + render(root, [vnode]) + render(root, [updated]) + + o(root.firstChild.attributes["id"].nodeValue).equals("a") + }) + + o("prevents update in text", function() { + var shouldUpdate = function() {return false} + var vnode = {tag: "#", attrs: {shouldUpdate: shouldUpdate}, children: "a"} + var updated = {tag: "#", attrs: {shouldUpdate: shouldUpdate}, children: "b"} + + render(root, [vnode]) + render(root, [updated]) + + o(root.firstChild.nodeValue).equals("a") + }) + + o("prevents update in html", function() { + var shouldUpdate = function() {return false} + var vnode = {tag: "<", attrs: {shouldUpdate: shouldUpdate}, children: "a"} + var updated = {tag: "<", attrs: {shouldUpdate: shouldUpdate}, children: "b"} + + render(root, [vnode]) + render(root, [updated]) + + o(root.firstChild.nodeValue).equals("a") + }) + + o("prevents update in fragment", function() { + var shouldUpdate = function() {return false} + var vnode = {tag: "[", attrs: {shouldUpdate: shouldUpdate}, children: [{tag: "#", children: "a"}]} + var updated = {tag: "[", attrs: {shouldUpdate: shouldUpdate}, children: [{tag: "#", children: "b"}]} + + render(root, [vnode]) + render(root, [updated]) + + o(root.firstChild.nodeValue).equals("a") + }) + + o("prevents update in component", function() { + var component = { + shouldUpdate: function() {return false}, + view: function(vnode) { + return {tag: "div", children: vnode.children} + }, + } + var vnode = {tag: component, children: [{tag: "#", children: "a"}]} + var updated = {tag: component, children: [{tag: "#", children: "b"}]} + + render(root, [vnode]) + render(root, [updated]) + + o(root.firstChild.firstChild.nodeValue).equals("a") + }) + + o("prevents update if returning false in component and false in vnode", function() { + var component = { + shouldUpdate: function() {return false}, + view: function(vnode) { + return {tag: "div", attrs: {id: vnode.attrs.id}} + }, + } + var vnode = {tag: component, attrs: {id: "a", shouldUpdate: function() {return false}}} + var updated = {tag: component, attrs: {id: "b", shouldUpdate: function() {return false}}} + + render(root, [vnode]) + render(root, [updated]) + + o(root.firstChild.attributes["id"].nodeValue).equals("a") + }) + + o("does not prevent update if returning true in component and true in vnode", function() { + var component = { + shouldUpdate: function() {return true}, + view: function(vnode) { + return {tag: "div", attrs: {id: vnode.attrs.id}} + }, + } + var vnode = {tag: component, attrs: {id: "a", shouldUpdate: function() {return true}}} + var updated = {tag: component, attrs: {id: "b", shouldUpdate: function() {return true}}} + + render(root, [vnode]) + render(root, [updated]) + + o(root.firstChild.attributes["id"].nodeValue).equals("b") + }) + + o("does not prevent update if returning false in component but true in vnode", function() { + var component = { + shouldUpdate: function() {return false}, + view: function(vnode) { + return {tag: "div", attrs: {id: vnode.attrs.id}} + }, + } + var vnode = {tag: component, attrs: {id: "a", shouldUpdate: function() {return true}}} + var updated = {tag: component, attrs: {id: "b", shouldUpdate: function() {return true}}} + + render(root, [vnode]) + render(root, [updated]) + + o(root.firstChild.attributes["id"].nodeValue).equals("b") + }) + + o("does not prevent update if returning true in component but false in vnode", function() { + var component = { + shouldUpdate: function() {return true}, + view: function(vnode) { + return {tag: "div", attrs: {id: vnode.attrs.id}} + }, + } + var vnode = {tag: component, attrs: {id: "a", shouldUpdate: function() {return false}}} + var updated = {tag: component, attrs: {id: "b", shouldUpdate: function() {return false}}} + + render(root, [vnode]) + render(root, [updated]) + + o(root.firstChild.attributes["id"].nodeValue).equals("b") + }) + + o("does not prevent update if returning true", function() { + var shouldUpdate = function() {return true} + var vnode = {tag: "div", attrs: {id: "a", shouldUpdate: shouldUpdate}} + var updated = {tag: "div", attrs: {id: "b", shouldUpdate: shouldUpdate}} + + render(root, [vnode]) + render(root, [updated]) + + o(root.firstChild.attributes["id"].nodeValue).equals("b") + }) + + o("does not prevent update if returning true from component", function() { + var component = { + shouldUpdate: function() {return true}, + view: function(vnode) { + return {tag: "div", attrs: vnode.attrs} + }, + } + var vnode = {tag: component, attrs: {id: "a"}} + var updated = {tag: component, attrs: {id: "b"}} + + render(root, [vnode]) + render(root, [updated]) + + o(root.firstChild.attributes["id"].nodeValue).equals("b") + }) + + o("accepts arguments for comparison", function() { + var count = 0 + var vnode = {tag: "div", attrs: {id: "a", shouldUpdate: shouldUpdate}} + var updated = {tag: "div", attrs: {id: "b", shouldUpdate: shouldUpdate}} + + render(root, [vnode]) + render(root, [updated]) + + function shouldUpdate(vnode, old) { + count++ + + o(old.attrs.id).equals("a") + o(vnode.attrs.id).equals("b") + + return old.attrs.id !== vnode.attrs.id + } + + o(count).equals(1) + o(root.firstChild.attributes["id"].nodeValue).equals("b") + }) + + o("accepts arguments for comparison in component", function() { + var component = { + shouldUpdate: shouldUpdate, + view: function(vnode) { + return {tag: "div", attrs: vnode.attrs} + }, + } + var count = 0 + var vnode = {tag: component, attrs: {id: "a"}} + var updated = {tag: component, attrs: {id: "b"}} + + render(root, [vnode]) + render(root, [updated]) + + function shouldUpdate(vnode, old) { + count++ + + o(old.attrs.id).equals("a") + o(vnode.attrs.id).equals("b") + + return old.attrs.id !== vnode.attrs.id + } + + o(count).equals(1) + o(root.firstChild.attributes["id"].nodeValue).equals("b") + }) + + o("is not called on creation", function() { + var count = 0 + var vnode = {tag: "div", attrs: {id: "a", shouldUpdate: shouldUpdate}} + var updated = {tag: "div", attrs: {id: "b", shouldUpdate: shouldUpdate}} + + render(root, [vnode]) + + function shouldUpdate(vnode, old) { + count++ + return true + } + + o(count).equals(0) + }) + + o("is not called on component creation", function() { + var component = { + shouldUpdate: shouldUpdate, + view: function(vnode) { + return {tag: "div", attrs: vnode.attrs} + }, + } + + var count = 0 + var vnode = {tag: "div", attrs: {id: "a"}} + var updated = {tag: "div", attrs: {id: "b"}} + + render(root, [vnode]) + + function shouldUpdate(vnode, old) { + count++ + return true + } + + o(count).equals(0) + }) + + o("is called only once on update", function() { + var count = 0 + var vnode = {tag: "div", attrs: {id: "a", shouldUpdate: shouldUpdate}} + var updated = {tag: "div", attrs: {id: "b", shouldUpdate: shouldUpdate}} + + render(root, [vnode]) + render(root, [updated]) + + function shouldUpdate(vnode, old) { + count++ + return true + } + + o(count).equals(1) + }) + + o("is called only once on component update", function() { + var component = { + shouldUpdate: shouldUpdate, + view: function(vnode) { + return {tag: "div", attrs: vnode.attrs} + }, + } + + var count = 0 + var vnode = {tag: component, attrs: {id: "a"}} + var updated = {tag: component, attrs: {id: "b"}} + + render(root, [vnode]) + render(root, [updated]) + + function shouldUpdate(vnode, old) { + count++ + return true + } + + o(count).equals(1) + }) +}) \ No newline at end of file diff --git a/render/tests/test-trust.js b/render/tests/test-trust.js index 9143cc17..50ff257d 100644 --- a/render/tests/test-trust.js +++ b/render/tests/test-trust.js @@ -1,7 +1,6 @@ "use strict" var o = require("../../ospec/ospec") -var domMock = require("../../test-utils/domMock") var trust = require("../../render/trust") o.spec("trust", function() {