diff --git a/docs/change-log.md b/docs/change-log.md index 3873c6b8..c365276d 100644 --- a/docs/change-log.md +++ b/docs/change-log.md @@ -18,6 +18,7 @@ - API: `m.redraw()` is always asynchronous ([#1592](https://github.com/MithrilJS/mithril.js/pull/1592)) - API: `m.mount()` will only render its own root when called, it will not trigger a `redraw()` ([#1592](https://github.com/MithrilJS/mithril.js/pull/1592)) +- API: Assigning to `vnode.state` (as in `vnode.state = ...`) is no longer supported. Instead, an error is thrown if `vnode.state` changes upon the invocation of a lifecycle hook. #### News diff --git a/docs/vnodes.md b/docs/vnodes.md index cff0ebe4..783f432f 100644 --- a/docs/vnodes.md +++ b/docs/vnodes.md @@ -74,7 +74,6 @@ Property | Type | Description `dom` | `Element?` | Points to the element that corresponds to the vnode. This property is `undefined` in the `oninit` lifecycle method. In fragments and trusted HTML vnodes, `dom` points to the first element in the range. `domSize` | `Number?` | This is only set in fragment and trusted HTML vnodes, and it's `undefined` in all other vnode types. It defines the number of DOM elements that the vnode represents (starting from the element referenced by the `dom` property). `state` | `Object?` | An object that is persisted between redraws. It is provided by the core engine when needed. In POJO component vnodes, the `state` inherits prototypically from the component object/class. In class component vnodes it is an instance of the class. In closure components it is the object returned by the closure. -`_state` | `Object?` | For components, a reference to the original `vnode.state` object, used to lookup the `view` and hooks. This property is only used internally by Mithril, do not use or modify it. `events` | `Object?` | An object that is persisted between redraws and that stores event handlers so that they can be removed using the DOM API. The `events` property is `undefined` if there are no event handlers defined. This property is only used internally by Mithril, do not use or modify it. `instance` | `Object?` | For components, a storage location for the value returned by the `view`. This property is only used internally by Mithril, do not use or modify it. `skip` | `Boolean` | This property is only used internally by Mithril when diffing keyed lists, do not use or modify it. @@ -89,7 +88,7 @@ The `tag` property of a vnode determines its type. There are five vnode types: Vnode type | Example | Description ------------ | ------------------------------ | --- Element | `{tag: "div"}` | Represents a DOM element. -Fragment | `{tag: "[", children: []}` | Represents a list of DOM elements whose parent DOM element may also contain other elements that are not in the fragment. When using the [`m()`](hyperscript.md) helper function, fragment vnodes can only be created by nesting arrays into the `children` parameter of `m()`. `m("[")` does not create a valid vnode. +Fragment | `{tag: "[", children: []}` | Represents a list of DOM elements whose parent DOM element may also contain other elements that are not in the fragment. When using the [`m()`](hyperscript.md) helper function, fragment vnodes can only be created by nesting arrays into the `children` parameter of `m()`. `m("[")` does not create a valid vnode. Text | `{tag: "#", children: ""}` | Represents a DOM text node. Trusted HTML | `{tag: "<", children: "
"}` | Represents a list of DOM elements from an HTML string. Component | `{tag: ExampleComponent}` | If `tag` is a Javascript object with a `view` method, the vnode represents the DOM generated by rendering the component. diff --git a/render/render.js b/render/render.js index 121083ca..0b881b2b 100644 --- a/render/render.js +++ b/render/render.js @@ -18,6 +18,24 @@ module.exports = function($window) { return vnode.attrs && vnode.attrs.xmlns || nameSpace[vnode.tag] } + //sanity check to discourage people from doing `vnode.state = ...` + function checkState(vnode, original) { + if (vnode.state !== original) throw new Error("`vnode.state` must not be modified") + } + + //Note: the hook is passed as the `this` argument to allow proxying the + //arguments without requiring a full array allocation to do so. It also + //takes advantage of the fact the current `vnode` is the first argument in + //all lifecycle methods. + function callHook(vnode) { + var original = vnode.state + try { + return this.apply(original, arguments) + } finally { + checkState(vnode, original) + } + } + //create function createNodes(parent, vnodes, start, end, hooks, nextSibling, ns) { for (var i = start; i < end; i++) { @@ -121,10 +139,9 @@ module.exports = function($window) { sentinel.$$reentrantLock$$ = true vnode.state = (vnode.tag.prototype != null && typeof vnode.tag.prototype.view === "function") ? new vnode.tag(vnode) : vnode.tag(vnode) } - vnode._state = vnode.state if (vnode.attrs != null) initLifecycle(vnode.attrs, vnode, hooks) - initLifecycle(vnode._state, vnode, hooks) - vnode.instance = Vnode.normalize(vnode._state.view.call(vnode.state, vnode)) + initLifecycle(vnode.state, vnode, hooks) + vnode.instance = Vnode.normalize(callHook.call(vnode.state.view, vnode)) if (vnode.instance === vnode) throw Error("A view cannot return the vnode it received as argument") sentinel.$$reentrantLock$$ = null } @@ -240,7 +257,6 @@ module.exports = function($window) { var oldTag = old.tag, tag = vnode.tag if (oldTag === tag) { vnode.state = old.state - vnode._state = old._state vnode.events = old.events if (!recycling && shouldNotUpdate(vnode, old)) return if (typeof oldTag === "string") { @@ -321,10 +337,10 @@ module.exports = function($window) { if (recycling) { initComponent(vnode, hooks) } else { - vnode.instance = Vnode.normalize(vnode._state.view.call(vnode.state, vnode)) + vnode.instance = Vnode.normalize(callHook.call(vnode.state.view, vnode)) if (vnode.instance === vnode) throw Error("A view cannot return the vnode it received as argument") if (vnode.attrs != null) updateLifecycle(vnode.attrs, vnode, hooks) - updateLifecycle(vnode._state, vnode, hooks) + updateLifecycle(vnode.state, vnode, hooks) } if (vnode.instance != null) { if (old.instance == null) createNode(parent, vnode.instance, hooks, ns, nextSibling) @@ -410,15 +426,16 @@ module.exports = function($window) { } function removeNode(vnode, context) { var expected = 1, called = 0 + var original = vnode.state if (vnode.attrs && typeof vnode.attrs.onbeforeremove === "function") { - var result = vnode.attrs.onbeforeremove.call(vnode.state, vnode) + var result = callHook.call(vnode.attrs.onbeforeremove, vnode) if (result != null && typeof result.then === "function") { expected++ result.then(continuation, continuation) } } - if (typeof vnode.tag !== "string" && typeof vnode._state.onbeforeremove === "function") { - var result = vnode._state.onbeforeremove.call(vnode.state, vnode) + if (typeof vnode.tag !== "string" && typeof vnode.state.onbeforeremove === "function") { + var result = callHook.call(vnode.state.onbeforeremove, vnode) if (result != null && typeof result.then === "function") { expected++ result.then(continuation, continuation) @@ -427,6 +444,7 @@ module.exports = function($window) { continuation() function continuation() { if (++called === expected) { + checkState(vnode, original) onremove(vnode) if (vnode.dom) { var count = vnode.domSize || 1 @@ -450,9 +468,9 @@ module.exports = function($window) { if (parent != null) parent.removeChild(node) } function onremove(vnode) { - if (vnode.attrs && typeof vnode.attrs.onremove === "function") vnode.attrs.onremove.call(vnode.state, vnode) + if (vnode.attrs && typeof vnode.attrs.onremove === "function") callHook.call(vnode.attrs.onremove, vnode) if (typeof vnode.tag !== "string") { - if (typeof vnode._state.onremove === "function") vnode._state.onremove.call(vnode.state, vnode) + if (typeof vnode.state.onremove === "function") callHook.call(vnode.state.onremove, vnode) if (vnode.instance != null) onremove(vnode.instance) } else { var children = vnode.children @@ -611,16 +629,20 @@ module.exports = function($window) { //lifecycle function initLifecycle(source, vnode, hooks) { - if (typeof source.oninit === "function") source.oninit.call(vnode.state, vnode) - if (typeof source.oncreate === "function") hooks.push(source.oncreate.bind(vnode.state, vnode)) + if (typeof source.oninit === "function") callHook.call(source.oninit, vnode) + if (typeof source.oncreate === "function") hooks.push(callHook.bind(source.oncreate, vnode)) } function updateLifecycle(source, vnode, hooks) { - if (typeof source.onupdate === "function") hooks.push(source.onupdate.bind(vnode.state, vnode)) + if (typeof source.onupdate === "function") hooks.push(callHook.bind(source.onupdate, vnode)) } 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.call(vnode.state, vnode, old) + if (vnode.attrs != null && typeof vnode.attrs.onbeforeupdate === "function") { + forceVnodeUpdate = callHook.call(vnode.attrs.onbeforeupdate, vnode, old) + } + if (typeof vnode.tag !== "string" && typeof vnode.state.onbeforeupdate === "function") { + forceComponentUpdate = callHook.call(vnode.state.onbeforeupdate, vnode, old) + } if (!(forceVnodeUpdate === undefined && forceComponentUpdate === undefined) && !forceVnodeUpdate && !forceComponentUpdate) { vnode.dom = old.dom vnode.domSize = old.domSize diff --git a/render/tests/test-component.js b/render/tests/test-component.js index c391ddd2..ea3d1624 100644 --- a/render/tests/test-component.js +++ b/render/tests/test-component.js @@ -764,97 +764,6 @@ o.spec("component", function() { o(attrs[hook].callCount).equals(methods[hook].callCount)(hook) }) }) - o("lifecycle timing megatest (for a single component with the state overwritten)", function() { - var methods = { - view: o.spy(function(vnode) { - o(vnode.state).equals(1) - 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(vnode) { - o(vnode.state).equals(1) - o(attrs[hook].callCount).equals(methods[hook].callCount + 1) - }) - methods[hook] = o.spy(function(vnode) { - o(vnode.state).equals(1) - o(attrs[hook].callCount).equals(methods[hook].callCount) - }) - }) - - var attrsOninit = attrs.oninit - var methodsOninit = methods.oninit - attrs.oninit = o.spy(function(vnode){ - vnode.state = 1 - return attrsOninit.call(this, vnode) - }) - methods.oninit = o.spy(function(vnode){ - vnode.state = 1 - return methodsOninit.call(this, vnode) - }) - - 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) { diff --git a/render/tests/test-onbeforeremove.js b/render/tests/test-onbeforeremove.js index 0e83d4a0..834f7510 100644 --- a/render/tests/test-onbeforeremove.js +++ b/render/tests/test-onbeforeremove.js @@ -36,10 +36,7 @@ o.spec("onbeforeremove", function() { o(update.callCount).equals(0) }) o("calls onbeforeremove when removing element", function(done) { - var vnode = {tag: "div", attrs: { - oninit: function() {vnode.state = {}}, - onbeforeremove: remove - }} + var vnode = {tag: "div", attrs: {onbeforeremove: remove}} render(root, [vnode]) render(root, []) @@ -47,6 +44,7 @@ o.spec("onbeforeremove", function() { function remove(node) { o(node).equals(vnode) o(this).equals(vnode.state) + o(this != null && typeof this === "object").equals(true) o(root.childNodes.length).equals(1) o(root.firstChild).equals(vnode.dom) diff --git a/render/vnode.js b/render/vnode.js index ce137703..13ed393f 100644 --- a/render/vnode.js +++ b/render/vnode.js @@ -1,7 +1,7 @@ "use strict" function Vnode(tag, key, attrs, children, text, dom) { - return {tag: tag, key: key, attrs: attrs, children: children, text: text, dom: dom, domSize: undefined, state: undefined, _state: undefined, events: undefined, instance: undefined, skip: false} + return {tag: tag, key: key, attrs: attrs, children: children, text: text, dom: dom, domSize: undefined, state: undefined, events: undefined, instance: undefined, skip: false} } Vnode.normalize = function(node) { if (Array.isArray(node)) return Vnode("[", undefined, undefined, Vnode.normalizeChildren(node), undefined, undefined)