diff --git a/.github/workflows/publish-prerelease.yml b/.github/workflows/publish-prerelease.yml index 3e4f89f13..1364fa7f2 100644 --- a/.github/workflows/publish-prerelease.yml +++ b/.github/workflows/publish-prerelease.yml @@ -9,7 +9,7 @@ jobs: concurrency: prr:pre-release runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v5 - uses: actions/setup-node@v4 with: node-version: 20 diff --git a/.github/workflows/push-release.yml b/.github/workflows/push-release.yml index 4e131505e..4d11bae73 100644 --- a/.github/workflows/push-release.yml +++ b/.github/workflows/push-release.yml @@ -12,7 +12,7 @@ jobs: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v5 with: ref: main - uses: actions/setup-node@v4 diff --git a/.github/workflows/rollback.yml b/.github/workflows/rollback.yml index 51847fbd9..1e8f428eb 100644 --- a/.github/workflows/rollback.yml +++ b/.github/workflows/rollback.yml @@ -10,7 +10,7 @@ jobs: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v5 - uses: actions/setup-node@v4 with: node-version: 20 diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 811eb8185..45958296a 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -23,7 +23,7 @@ jobs: concurrency: prr:pre-release runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v5 - uses: actions/setup-node@v4 with: node-version: 20 diff --git a/README.md b/README.md index 691967b05..10d680b17 100644 --- a/README.md +++ b/README.md @@ -19,7 +19,7 @@ A modern client-side JavaScript framework for building Single Page Applications. Mithril.js is used by companies like Vimeo and Nike, and open source platforms like Lichess 👍. -Mithril.js supports IE11, Firefox ESR, and the last two versions of Firefox, Edge, Safari, and Chrome. No polyfills required. 👌 +Mithril.js supports Firefox ESR, and the last two versions of Firefox, Edge, Safari, and Chrome. No polyfills required. 👌 ## Installation diff --git a/render/cachedAttrsIsStaticMap.js b/render/cachedAttrsIsStaticMap.js new file mode 100644 index 000000000..288721a89 --- /dev/null +++ b/render/cachedAttrsIsStaticMap.js @@ -0,0 +1,12 @@ +"use strict" + +var emptyAttrs = require("./emptyAttrs") + +// This Map manages the following: +// - Whether an attrs is cached attrs generated by compileSelector(). +// - Whether the cached attrs is "static", i.e., does not contain any form attributes. +// These information will be useful to skip updating attrs in render(). +// +// Since the attrs used as keys in this map are not released from the selectorCache object, +// there is no risk of memory leaks. Therefore, Map is used here instead of WeakMap. +module.exports = new Map([[emptyAttrs, true]]) diff --git a/render/emptyAttrs.js b/render/emptyAttrs.js new file mode 100644 index 000000000..8d278ded4 --- /dev/null +++ b/render/emptyAttrs.js @@ -0,0 +1,4 @@ +"use strict" + +// This is an attrs object that is used by default when attrs is undefined or null. +module.exports = {} diff --git a/render/fragment.js b/render/fragment.js index 90cad40da..3ee22f5b7 100644 --- a/render/fragment.js +++ b/render/fragment.js @@ -3,8 +3,8 @@ var Vnode = require("../render/vnode") var hyperscriptVnode = require("./hyperscriptVnode") -module.exports = function() { - var vnode = hyperscriptVnode.apply(0, arguments) +module.exports = function(attrs, ...children) { + var vnode = hyperscriptVnode(attrs, children) vnode.tag = "[" vnode.children = Vnode.normalizeChildren(vnode.children) diff --git a/render/hyperscript.js b/render/hyperscript.js index 2f7edf6db..7429faf54 100644 --- a/render/hyperscript.js +++ b/render/hyperscript.js @@ -3,6 +3,8 @@ var Vnode = require("../render/vnode") var hyperscriptVnode = require("./hyperscriptVnode") var hasOwn = require("../util/hasOwn") +var emptyAttrs = require("./emptyAttrs") +var cachedAttrsIsStaticMap = require("./cachedAttrsIsStaticMap") var selectorParser = /(?:(^|#|\.)([^#\.\[\]]+))|(\[(.+?)(?:\s*=\s*("|'|)((?:\\["'\]]|.)*?)\5)?\])/g var selectorCache = Object.create(null) @@ -12,8 +14,12 @@ function isEmpty(object) { return true } +function isFormAttributeKey(key) { + return key === "value" || key === "checked" || key === "selectedIndex" || key === "selected" +} + function compileSelector(selector) { - var match, tag = "div", classes = [], attrs = {} + var match, tag = "div", classes = [], attrs = {}, isStatic = true while (match = selectorParser.exec(selector)) { var type = match[1], value = match[2] if (type === "" && value !== "") tag = value @@ -23,22 +29,32 @@ function compileSelector(selector) { var attrValue = match[6] if (attrValue) attrValue = attrValue.replace(/\\(["'])/g, "$1").replace(/\\\\/g, "\\") if (match[4] === "class") classes.push(attrValue) - else attrs[match[4]] = attrValue === "" ? attrValue : attrValue || true + else { + attrs[match[4]] = attrValue === "" ? attrValue : attrValue || true + if (isFormAttributeKey(match[4])) isStatic = false + } } } if (classes.length > 0) attrs.className = classes.join(" ") - if (isEmpty(attrs)) attrs = null - return selectorCache[selector] = {tag: tag, attrs: attrs} + if (isEmpty(attrs)) attrs = emptyAttrs + else cachedAttrsIsStaticMap.set(attrs, isStatic) + return selectorCache[selector] = {tag: tag, attrs: attrs, is: attrs.is} } function execSelector(state, vnode) { + vnode.tag = state.tag + var attrs = vnode.attrs + if (attrs == null) { + vnode.attrs = state.attrs + vnode.is = state.is + return vnode + } + var hasClass = hasOwn.call(attrs, "class") var className = hasClass ? attrs.class : attrs.className - vnode.tag = state.tag - - if (state.attrs != null) { + if (state.attrs !== emptyAttrs) { attrs = Object.assign({}, state.attrs, attrs) if (className != null || state.attrs.className != null) attrs.className = @@ -46,9 +62,7 @@ function execSelector(state, vnode) { ? state.attrs.className != null ? String(state.attrs.className) + " " + String(className) : className - : state.attrs.className != null - ? state.attrs.className - : null + : state.attrs.className } else { if (className != null) attrs.className = className } @@ -70,12 +84,12 @@ function execSelector(state, vnode) { return vnode } -function hyperscript(selector) { +function hyperscript(selector, attrs, ...children) { if (selector == null || typeof selector !== "string" && typeof selector !== "function" && typeof selector.view !== "function") { throw Error("The selector must be either a string or a component."); } - var vnode = hyperscriptVnode.apply(1, arguments) + var vnode = hyperscriptVnode(attrs, children) if (typeof selector === "string") { vnode.children = Vnode.normalizeChildren(vnode.children) diff --git a/render/hyperscriptVnode.js b/render/hyperscriptVnode.js index 9f31235c4..b6474e85c 100644 --- a/render/hyperscriptVnode.js +++ b/render/hyperscriptVnode.js @@ -2,52 +2,22 @@ var Vnode = require("../render/vnode") -// Call via `hyperscriptVnode.apply(startOffset, arguments)` +// Note: the processing of variadic parameters is perf-sensitive. // -// 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, it might be preferable to define hyperscript and fragment +// factories with a final ...args parameter and call hyperscriptVnode(...args), +// since modern engines can optimize spread calls. // -// 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.js 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] +// However, benchmarks showed this was not faster. As a result, spread is used +// only in the parameter lists of hyperscript and fragment, while an array is +// passed to hyperscriptVnode. +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 = [] - while (start < arguments.length) children.push(arguments[start++]) + children = children.length === 0 && Array.isArray(attrs) ? attrs : [attrs, ...children] + attrs = undefined } - return Vnode("", attrs.key, attrs, children) + return Vnode("", attrs && attrs.key, attrs, children) } diff --git a/render/render.js b/render/render.js index d9b0f9d76..1c97dfab1 100644 --- a/render/render.js +++ b/render/render.js @@ -4,6 +4,7 @@ var Vnode = require("../render/vnode") var df = require("../render/domFor") var delayedRemoval = df.delayedRemoval var domFor = df.domFor +var cachedAttrsIsStaticMap = require("./cachedAttrsIsStaticMap") module.exports = function() { var nameSpace = { @@ -453,7 +454,9 @@ module.exports = function() { var element = vnode.dom = old.dom ns = getNameSpace(vnode) || ns - updateAttrs(vnode, old.attrs, vnode.attrs, ns) + if (old.attrs != vnode.attrs || (vnode.attrs != null && !cachedAttrsIsStaticMap.get(vnode.attrs))) { + updateAttrs(vnode, old.attrs, vnode.attrs, ns) + } if (!maybeSetContentEditable(vnode)) { updateNodes(element, old.children, vnode.children, hooks, null, ns) } @@ -715,7 +718,7 @@ module.exports = function() { // so removal should be done first to prevent accidental removal for newly setting values. var val if (old != null) { - if (old === attrs) { + if (old === attrs && !cachedAttrsIsStaticMap.has(attrs)) { console.warn("Don't reuse attrs object, use new object for every redraw, this will throw in next major") } for (var key in old) { diff --git a/scripts/_bundler-impl.js b/scripts/_bundler-impl.js index ae7b4faf8..5f289aa92 100644 --- a/scripts/_bundler-impl.js +++ b/scripts/_bundler-impl.js @@ -158,10 +158,12 @@ module.exports = async (input) => { }) //fix props - const props = new RegExp(`((?:[^:]\\/\\/.*)?\\.\\s*)(${candidates})|([\\{,]\\s*)(${candidates})(\\s*:)`, "gm") - code = code.replace(props, (match, dot, a, pre, b, post) => { + const props = new RegExp(`(\\.\\.)?((?:[^:]\\/\\/.*)?\\.\\s*)(${candidates})|([\\{,]\\s*)(${candidates})(\\s*:)`, "gm") + code = code.replace(props, (match, dotdot, dot, a, pre, b, post) => { // Don't do anything because dot was matched in a comment if (dot && dot.indexOf("//") === 1) return match + // Don't do anything because dot is a part of spread syntax or destructuring + if (dotdot) return match if (dot) return dot + a.replace(/\d+$/, "") return pre + b.replace(/\d+$/, "") + post }) diff --git a/scripts/tests/test-bundler.js b/scripts/tests/test-bundler.js index a3f368b1c..0c9949010 100644 --- a/scripts/tests/test-bundler.js +++ b/scripts/tests/test-bundler.js @@ -368,4 +368,87 @@ o.spec("bundler", async () => { // check that the argument z2 is not z00 o(await bundle(p("a.js"))).equals(";(function() {\nvar z0 = {}\nvar _1 = function(z1){}\nvar b = _1(z0)\nvar z = z0\nvar _5 = function(z2){}\nvar c = _5(z)\n}());") }) + o.spec("spread syntax and destructuring (...)", () => { + o("rest parameter", async () => { + await setup({ + "a.js": 'var b = require("./b")\nvar c = require("./c")', + "b.js": "var a = 1\nmodule.exports = a", + "c.js": "function f(d, ...a){}\nmodule.exports = f", + }) + + o(await bundle(p("a.js"))).equals(";(function() {\nvar a = 1\nvar b = a\nfunction f(d, ...a0){}\nvar c = f\n}());") + }) + o("function call", async () => { + await setup({ + "a.js": 'var b = require("./b")\nvar c = require("./c")', + "b.js": "var a = 1\nmodule.exports = a", + "c.js": "var a = [1, 2, 3]\nvar d = f(...a)\nmodule.exports = d", + }) + + o(await bundle(p("a.js"))).equals(";(function() {\nvar a = 1\nvar b = a\nvar a0 = [1, 2, 3]\nvar d = f(...a0)\nvar c = d\n}());") + }) + o("new", async () => { + await setup({ + "a.js": 'var b = require("./b")\nvar c = require("./c")', + "b.js": "var a = 1\nmodule.exports = a", + "c.js": "var a = [1, 2, 3]\nvar d = new f(...a)\nmodule.exports = d", + }) + + o(await bundle(p("a.js"))).equals(";(function() {\nvar a = 1\nvar b = a\nvar a0 = [1, 2, 3]\nvar d = new f(...a0)\nvar c = d\n}());") + }) + o("array spread", async () => { + await setup({ + "a.js": 'var b = require("./b")\nvar c = require("./c")', + "b.js": "var a = 1\nmodule.exports = a", + "c.js": "var a = [1, 2, 3]\nvar arr = [...a]\nmodule.exports = arr", + }) + + o(await bundle(p("a.js"))).equals(";(function() {\nvar a = 1\nvar b = a\nvar a0 = [1, 2, 3]\nvar arr = [...a0]\nvar c = arr\n}());") + }) + o("array spread (merge)", async () => { + await setup({ + "a.js": 'var b = require("./b")\nvar c = require("./c")', + "b.js": "var a = 1\nmodule.exports = a", + "c.js": "var a = [1, 2, 3]\nvar arr = [0, ...a, 4]\nmodule.exports = arr", + }) + + o(await bundle(p("a.js"))).equals(";(function() {\nvar a = 1\nvar b = a\nvar a0 = [1, 2, 3]\nvar arr = [0, ...a0, 4]\nvar c = arr\n}());") + }) + o("array destructuring", async () => { + await setup({ + "a.js": 'var b = require("./b")\nvar c = require("./c")', + "b.js": "var a = 1\nmodule.exports = a", + "c.js": "var a = [1, 2, 3]\nvar d\n[d, ...a] = a\nmodule.exports = a", + }) + + o(await bundle(p("a.js"))).equals(";(function() {\nvar a = 1\nvar b = a\nvar a0 = [1, 2, 3]\nvar d\n[d, ...a0] = a0\nvar c = a0\n}());") + }) + o("object spread", async () => { + await setup({ + "a.js": 'var b = require("./b")\nvar c = require("./c")', + "b.js": "var a = 1\nmodule.exports = a", + "c.js": "var a = { p: 1, q: 2, r: 3 }\nvar d = {...a}\nmodule.exports = d", + }) + + o(await bundle(p("a.js"))).equals(";(function() {\nvar a = 1\nvar b = a\nvar a0 = { p: 1, q: 2, r: 3 }\nvar d = {...a0}\nvar c = d\n}());") + }) + o("object spread (merge)", async () => { + await setup({ + "a.js": 'var b = require("./b")\nvar c = require("./c")', + "b.js": "var a = 1\nmodule.exports = a", + "c.js": "var a = { p: 1, q: 2, r: 3 }\nvar d = {o:0,...a}\nmodule.exports = d", + }) + + o(await bundle(p("a.js"))).equals(";(function() {\nvar a = 1\nvar b = a\nvar a0 = { p: 1, q: 2, r: 3 }\nvar d = {o:0,...a0}\nvar c = d\n}());") + }) + o("object destructuring", async () => { + await setup({ + "a.js": 'var b = require("./b")\nvar c = require("./c")', + "b.js": "var a = 1\nmodule.exports = a", + "c.js": "var obj = { p: 1, q: 2, r: 3 }\nvar p,a\n({p,...a}=obj)\nmodule.exports = a", + }) + + o(await bundle(p("a.js"))).equals(";(function() {\nvar a = 1\nvar b = a\nvar obj = { p: 1, q: 2, r: 3 }\nvar p,a0\n({p,...a0}=obj)\nvar c = a0\n}());") + }) + }) })