From 9dd498f4b4f3d6450fecf5a81456b7c180138cfd Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 14 Aug 2025 20:19:21 +0000 Subject: [PATCH 1/2] Bump actions/checkout from 4 to 5 in the normal group Bumps the normal group with 1 update: [actions/checkout](https://github.com/actions/checkout). Updates `actions/checkout` from 4 to 5 - [Release notes](https://github.com/actions/checkout/releases) - [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md) - [Commits](https://github.com/actions/checkout/compare/v4...v5) --- updated-dependencies: - dependency-name: actions/checkout dependency-version: '5' dependency-type: direct:production update-type: version-update:semver-major dependency-group: normal ... Signed-off-by: dependabot[bot] --- .github/workflows/publish-prerelease.yml | 2 +- .github/workflows/push-release.yml | 2 +- .github/workflows/rollback.yml | 2 +- .github/workflows/test.yml | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) 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 From cc0df94c0467ff6a342d6084827c89590c0b87f1 Mon Sep 17 00:00:00 2001 From: kfule Date: Mon, 25 Aug 2025 11:39:53 +0900 Subject: [PATCH 2/2] Assorted Performance Improvements (#3041) * hyperscriptVnode: use spread syntax The implementation adopts spread syntax, inspired by the approach mentioned in the comments. However, argument handling has been adjusted for better performance. Using spread syntax not only allows related comments to be deleted, but also slightly reduces bundle size and improves benchmarks by several percent. All existing tests pass successfully. Note that spread syntax does not seem to be supported by the existing internal bundler in some cases, so the bundler has also been updated to prevent incorrect suffix handling. * allow attrs to be undefined and execute early return in `execSelector()` Since commit f9e5163, attrs is never null/undefined. However, `m.render()` takes into account the case where attrs is null/undefined, and it is better to use undefined from a performance point of view. Also, early return within `execSelector()` contributes to further performance improvement. Some tests are changed because of the null/undefined attrs. This commit may be breaking from this point of view, but the v2.0.4 era allows for attrs to be null/undefined and the impact will be quite small. * use the cached attrs as-is and skip `updateAttrs()` if the static attrs are identical Since `compileSelector()` generates attrs objects containing only strings (no functions or nested objects). So, `updateAttrs()` can safely be skipped by checking the equality of the cached attrs objects themselves if the objects do not contain form attributes. If you mainly use the "static" attrs and do not use dynamic attrs, skipping `updateAttrs()` allows significant performance improvements. Also, the added checks are lightweight and there appears to be little or no performance degradation due to these checks. This commit itself is not a breaking change in the sense that it passes all existing tests. The checks in `m.render()` above and the form attribute checks in `compileSelector()` result in a minor increase in code volume, though. * refactor execSelector() This state.attrs.className != null check is redundant. * README: drop IE11 support It seems that the current codebase (including generator, dom-for and spread syntax) already has little support for IE11 because it requires transpiling as well as polyfills. * prevents existing tests from being changed As this results in a slight decrease in performance improvement, comments regarding potential performance improvements have also been added. * use a common empty attrs object This simplifies the processing within execSelector. * cachedAttrsIsStaticMap: use WeakMap to avoid potential memory leaks * hyperscriptVnode: add performance sensitivity comments * cachedAttrsIsStaticMap: revert back to Map from WeakMap with comments * pass emptyAttrs to the Map constructor This slightly reduces the bundle size. --- README.md | 2 +- render/cachedAttrsIsStaticMap.js | 12 +++++ render/emptyAttrs.js | 4 ++ render/fragment.js | 4 +- render/hyperscript.js | 38 ++++++++++----- render/hyperscriptVnode.js | 56 +++++---------------- render/render.js | 7 ++- scripts/_bundler-impl.js | 6 ++- scripts/tests/test-bundler.js | 83 ++++++++++++++++++++++++++++++++ 9 files changed, 150 insertions(+), 62 deletions(-) create mode 100644 render/cachedAttrsIsStaticMap.js create mode 100644 render/emptyAttrs.js 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}());") + }) + }) })