diff --git a/.gitattributes b/.gitattributes index 7296866f..91f1f822 100644 --- a/.gitattributes +++ b/.gitattributes @@ -1,5 +1,7 @@ * text=auto /mithril.js binary /mithril.min.js binary +/mithril.mjs binary +/mithril.min.mjs binary /package-lock.json binary /yarn.lock binary diff --git a/docs/change-log.md b/docs/change-log.md index a9b46161..380fd761 100644 --- a/docs/change-log.md +++ b/docs/change-log.md @@ -54,6 +54,7 @@ - All of the `m.*` properties from `mithril` are re-exported as named exports in addition to being attached to `m`. - `m()` itself from `mithril` is exported as the default export. - `mithril/stream`'s primary export is exported as the default export. +- fragments: allow same attrs/children overloading logic as hyperscript ([#2328](https://github.com/MithrilJS/mithril.js/pull/2328)) #### Bug fixes diff --git a/docs/fragment.md b/docs/fragment.md index d3bddc7a..5a914165 100644 --- a/docs/fragment.md +++ b/docs/fragment.md @@ -37,9 +37,9 @@ Generates a fragment [vnode](vnodes.md) Argument | Type | Required | Description ----------- | --------------------------------------------------- | -------- | --- -`attrs` | `Object` | Yes | A map of attributes -`children` | `Array` | Yes | A list of vnodes -**returns** | `Vnode` | | A fragment [vnode](vnodes.md) +`attrs` | `Object` | No | HTML attributes or element properties +`children` | `Array|String|Number|Boolean` | No | Child [vnodes](vnodes.md#structure). Can be written as [splat arguments](signatures.md#splats) +**returns** | `Vnode` | | A fragment [vnode](vnodes.md#structure) [How to read signatures](signatures.md) @@ -49,14 +49,14 @@ Argument | Type | Required | D `m.fragment()` creates a [fragment vnode](vnodes.md) with attributes. It is meant for advanced use cases involving [keys](keys.md) or [lifecyle methods](lifecycle-methods.md). -A fragment vnode represents a list of DOM elements. If you want a regular element vnode that represents only one DOM element, you should use [`m()`](hyperscript.md) instead. +A fragment vnode represents a list of DOM elements. If you want a regular element vnode that represents only one DOM element and don't require keyed logic, you should use [`m()`](hyperscript.md) instead. -Normally you can use simple arrays instead to denote a list of nodes: +Normally you can use simple arrays or splats instead to denote a list of nodes: ```javascript var groupVisible = true -m("ul", [ +m("ul", m("li", "child 1"), m("li", "child 2"), groupVisible ? [ @@ -64,7 +64,7 @@ m("ul", [ m("li", "child 3"), m("li", "child 4"), ] : null -]) +) ``` However, Javascript arrays cannot be keyed or hold lifecycle methods. One option would be to create a wrapper element to host the key or lifecycle method, but sometimes it is not desirable to have an extra element (for example in complex table structures). In those cases, a fragment vnode can be used instead. diff --git a/docs/hyperscript.md b/docs/hyperscript.md index 07808bdc..4adb8516 100644 --- a/docs/hyperscript.md +++ b/docs/hyperscript.md @@ -40,12 +40,12 @@ You can also [use HTML syntax](https://babeljs.io/repl/#?code=%2F**%20%40jsx%20m ### Signature -`vnode = m(selector, attributes, children)` +`vnode = m(selector, attrs, children)` Argument | Type | Required | Description ------------ | ------------------------------------------ | -------- | --- `selector` | `String|Object` | Yes | A CSS selector or a [component](components.md) -`attributes` | `Object` | No | HTML attributes or element properties +`attrs` | `Object` | No | HTML attributes or element properties `children` | `Array|String|Number|Boolean` | No | Child [vnodes](vnodes.md#structure). Can be written as [splat arguments](signatures.md#splats) **returns** | `Vnode` | | A [vnode](vnodes.md#structure) @@ -55,7 +55,7 @@ Argument | Type | Required | Descripti ### How it works -Mithril provides a hyperscript function `m()`, which allows expressing any HTML structure using javascript syntax. It accepts a `selector` string (required), an `attributes` object (optional) and a `children` array (optional). +Mithril provides a hyperscript function `m()`, which allows expressing any HTML structure using javascript syntax. It accepts a `selector` string (required), an `attrs` object (optional) and a `children` array (optional). ```javascript m("div", {id: "box"}, "hello") diff --git a/render/fragment.js b/render/fragment.js index 82918975..90cad40d 100644 --- a/render/fragment.js +++ b/render/fragment.js @@ -1,7 +1,12 @@ "use strict" var Vnode = require("../render/vnode") +var hyperscriptVnode = require("./hyperscriptVnode") -module.exports = function(attrs, children) { - return Vnode("[", attrs.key, attrs, Vnode.normalizeChildren(children), undefined, undefined) +module.exports = function() { + var vnode = hyperscriptVnode.apply(0, arguments) + + vnode.tag = "[" + vnode.children = Vnode.normalizeChildren(vnode.children) + return vnode } diff --git a/render/hyperscript.js b/render/hyperscript.js index dce92e45..c6205410 100644 --- a/render/hyperscript.js +++ b/render/hyperscript.js @@ -1,6 +1,7 @@ "use strict" var Vnode = require("../render/vnode") +var hyperscriptVnode = require("./hyperscriptVnode") var selectorParser = /(?:(^|#|\.)([^#\.\[\]]+))|(\[(.+?)(?:\s*=\s*("|'|)((?:\\["'\]]|.)*?)\5)?\])/g var selectorCache = {} @@ -29,18 +30,21 @@ function compileSelector(selector) { return selectorCache[selector] = {tag: tag, attrs: attrs} } -function execSelector(state, attrs, children) { - var hasAttrs = false, childList, text - var classAttr = hasOwn.call(attrs, "class") ? "class" : "className" - var className = attrs[classAttr] +function execSelector(state, vnode) { + var attrs = vnode.attrs + var children = Vnode.normalizeChildren(vnode.children) + var hasClass = hasOwn.call(attrs, "class") + var className = hasClass ? attrs.class : attrs.className + + vnode.tag = state.tag + vnode.attrs = null + vnode.children = undefined if (!isEmpty(state.attrs) && !isEmpty(attrs)) { var newAttrs = {} - for(var key in attrs) { - if (hasOwn.call(attrs, key)) { - newAttrs[key] = attrs[key] - } + for (var key in attrs) { + if (hasOwn.call(attrs, key)) newAttrs[key] = attrs[key] } attrs = newAttrs @@ -60,22 +64,22 @@ function execSelector(state, attrs, children) { ? state.attrs.className : null - if (classAttr === "class") attrs.class = null + if (hasClass) attrs.class = null for (var key in attrs) { if (hasOwn.call(attrs, key) && key !== "key") { - hasAttrs = true + vnode.attrs = attrs break } } if (Array.isArray(children) && children.length === 1 && children[0] != null && children[0].tag === "#") { - text = children[0].children + vnode.text = children[0].children } else { - childList = children + vnode.children = children } - return Vnode(state.tag, attrs.key, hasAttrs ? attrs : null, childList, text) + return vnode } function hyperscript(selector) { @@ -83,27 +87,13 @@ function hyperscript(selector) { throw Error("The selector must be either a string or a component."); } - var attrs = arguments[1], start = 2, children - - if (attrs == null) { - attrs = {} - } else if (typeof attrs !== "object" || attrs.tag != null || Array.isArray(attrs)) { - attrs = {} - start = 1 - } - - if (arguments.length === start + 1) { - children = arguments[start] - if (!Array.isArray(children)) children = [children] - } else { - children = [] - while (start < arguments.length) children.push(arguments[start++]) - } + var vnode = hyperscriptVnode.apply(1, arguments) if (typeof selector === "string") { - return execSelector(selectorCache[selector] || compileSelector(selector), attrs, Vnode.normalizeChildren(children)) + return execSelector(selectorCache[selector] || compileSelector(selector), vnode) } else { - return Vnode(selector, attrs.key, attrs, children) + vnode.tag = selector + return vnode } } diff --git a/render/hyperscriptVnode.js b/render/hyperscriptVnode.js new file mode 100644 index 00000000..37e92b35 --- /dev/null +++ b/render/hyperscriptVnode.js @@ -0,0 +1,53 @@ +"use strict" + +var Vnode = require("../render/vnode") + +// Call via `hyperscriptVnode.apply(startOffset, arguments)` +// +// The reason I do it this way, forwarding the arguments and passing the start +// offset in `this`, is so I don't have to create a temporary array in a +// performance-critical path. +// +// In native ES6, I'd instead add a final `...args` parameter to the +// `hyperscript` and `fragment` factories and define this as +// `hyperscriptVnode(...args)`, since modern engines do optimize that away. But +// ES5 (what Mithril requires thanks to IE support) doesn't give me that luxury, +// and engines aren't nearly intelligent enough to do either of these: +// +// 1. Elide the allocation for `[].slice.call(arguments, 1)` when it's passed to +// another function only to be indexed. +// 2. Elide an `arguments` allocation when it's passed to any function other +// than `Function.prototype.apply` or `Reflect.apply`. +// +// In ES6, it'd probably look closer to this (I'd need to profile it, though): +// module.exports = function(attrs, ...children) { +// if (attrs == null || typeof attrs === "object" && attrs.tag == null && !Array.isArray(attrs)) { +// if (children.length === 1 && Array.isArray(children[0])) children = children[0] +// } else { +// children = children.length === 0 && Array.isArray(attrs) ? attrs : [attrs, ...children] +// attrs = undefined +// } +// +// if (attrs == null) attrs = {} +// return Vnode("", attrs.key, attrs, children) +// } +module.exports = function() { + var attrs = arguments[this], start = this + 1, children + + if (attrs == null) { + attrs = {} + } else if (typeof attrs !== "object" || attrs.tag != null || Array.isArray(attrs)) { + attrs = {} + start = this + } + + if (arguments.length === start + 1) { + children = arguments[start] + if (!Array.isArray(children)) children = [children] + } else { + children = [] + while (start < arguments.length) children.push(arguments[start++]) + } + + return Vnode("", attrs.key, attrs, children) +} diff --git a/render/tests/test-fragment.js b/render/tests/test-fragment.js index b10fc850..2883cd44 100644 --- a/render/tests/test-fragment.js +++ b/render/tests/test-fragment.js @@ -32,4 +32,166 @@ o.spec("fragment", function() { o(frag.key).equals(7) }) + o.spec("children with no attrs", function() { + o("handles string single child", function() { + var vnode = fragment(["a"]) + + o(vnode.children[0].tag).equals("#") + o(vnode.children[0].children).equals("a") + }) + o("handles falsy string single child", function() { + var vnode = fragment([""]) + + o(vnode.children[0].tag).equals("#") + o(vnode.children[0].children).equals("") + }) + o("handles number single child", function() { + var vnode = fragment([1]) + + o(vnode.children[0].tag).equals("#") + o(vnode.children[0].children).equals(1) + }) + o("handles falsy number single child", function() { + var vnode = fragment([0]) + + o(vnode.children[0].tag).equals("#") + o(vnode.children[0].children).equals(0) + }) + o("handles boolean single child", function() { + var vnode = fragment([true]) + + o(vnode.children[0].tag).equals("#") + o(vnode.children[0].children).equals(true) + }) + o("handles falsy boolean single child", function() { + var vnode = fragment([false]) + + o(vnode.children[0].tag).equals("#") + o(vnode.children[0].children).equals("") + }) + o("handles null single child", function() { + var vnode = fragment([null]) + + o(vnode.children[0]).equals(null) + }) + o("handles undefined single child", function() { + var vnode = fragment([undefined]) + + o(vnode.children[0]).equals(undefined) + }) + o("handles multiple string children", function() { + var vnode = fragment(["", "a"]) + + o(vnode.children[0].tag).equals("#") + o(vnode.children[0].children).equals("") + o(vnode.children[1].tag).equals("#") + o(vnode.children[1].children).equals("a") + }) + o("handles multiple number children", function() { + var vnode = fragment([0, 1]) + + o(vnode.children[0].tag).equals("#") + o(vnode.children[0].children).equals(0) + o(vnode.children[1].tag).equals("#") + o(vnode.children[1].children).equals(1) + }) + o("handles multiple boolean children", function() { + var vnode = fragment([false, true]) + + o(vnode.children[0].tag).equals("#") + o(vnode.children[0].children).equals("") + o(vnode.children[1].tag).equals("#") + o(vnode.children[1].children).equals(true) + }) + o("handles multiple null/undefined child", function() { + var vnode = fragment([null, undefined]) + + o(vnode.children[0]).equals(null) + o(vnode.children[1]).equals(undefined) + }) + o("handles falsy number single child without attrs", function() { + var vnode = fragment(0) + + o(vnode.children[0].tag).equals("#") + o(vnode.children[0].children).equals(0) + }) + }) + o.spec("children with attrs", function() { + o("handles string single child", function() { + var vnode = fragment({}, ["a"]) + + o(vnode.children[0].tag).equals("#") + o(vnode.children[0].children).equals("a") + }) + o("handles falsy string single child", function() { + var vnode = fragment({}, [""]) + + o(vnode.children[0].tag).equals("#") + o(vnode.children[0].children).equals("") + }) + o("handles number single child", function() { + var vnode = fragment({}, [1]) + + o(vnode.children[0].tag).equals("#") + o(vnode.children[0].children).equals(1) + }) + o("handles falsy number single child", function() { + var vnode = fragment({}, [0]) + + o(vnode.children[0].tag).equals("#") + o(vnode.children[0].children).equals(0) + }) + o("handles boolean single child", function() { + var vnode = fragment({}, [true]) + + o(vnode.children[0].tag).equals("#") + o(vnode.children[0].children).equals(true) + }) + o("handles falsy boolean single child", function() { + var vnode = fragment({}, [false]) + + o(vnode.children[0].tag).equals("#") + o(vnode.children[0].children).equals("") + }) + o("handles null single child", function() { + var vnode = fragment({}, [null]) + + o(vnode.children[0]).equals(null) + }) + o("handles undefined single child", function() { + var vnode = fragment({}, [undefined]) + + o(vnode.children[0]).equals(undefined) + }) + o("handles multiple string children", function() { + var vnode = fragment({}, ["", "a"]) + + o(vnode.children[0].tag).equals("#") + o(vnode.children[0].children).equals("") + o(vnode.children[1].tag).equals("#") + o(vnode.children[1].children).equals("a") + }) + o("handles multiple number children", function() { + var vnode = fragment({}, [0, 1]) + + o(vnode.children[0].tag).equals("#") + o(vnode.children[0].children).equals(0) + o(vnode.children[1].tag).equals("#") + o(vnode.children[1].children).equals(1) + }) + o("handles multiple boolean children", function() { + var vnode = fragment({}, [false, true]) + + o(vnode.children[0].tag).equals("#") + o(vnode.children[0].children).equals("") + o(vnode.children[1].tag).equals("#") + o(vnode.children[1].children).equals(true) + }) + o("handles multiple null/undefined child", function() { + var vnode = fragment({}, [null, undefined]) + + o(vnode.children[0]).equals(null) + o(vnode.children[1]).equals(undefined) + }) + }) })