From 4c93c3c371523cf68baa2e39626220afd5f07e5e Mon Sep 17 00:00:00 2001 From: kfule Date: Mon, 21 Oct 2024 20:43:38 +0900 Subject: [PATCH 1/7] [refactor] hyperscript: use assign() to construct attrs in execSelector() --- render/hyperscript.js | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/render/hyperscript.js b/render/hyperscript.js index 028c8bdbe..7566be94a 100644 --- a/render/hyperscript.js +++ b/render/hyperscript.js @@ -3,6 +3,7 @@ var Vnode = require("../render/vnode") var hyperscriptVnode = require("./hyperscriptVnode") var hasOwn = require("../util/hasOwn") +var assign = require("../util/assign") var selectorParser = /(?:(^|#|\.)([^#\.\[\]]+))|(\[(.+?)(?:\s*=\s*("|'|)((?:\\["'\]]|.)*?)\5)?\])/g var selectorCache = {} @@ -38,20 +39,9 @@ function execSelector(state, vnode) { vnode.tag = state.tag if (!isEmpty(state.attrs)) { - var newAttrs = {} - - for (var key in attrs) { - if (hasOwn.call(attrs, key)) newAttrs[key] = attrs[key] - } - - attrs = newAttrs + attrs = assign({}, state.attrs, attrs) } - for (var key in state.attrs) { - if (hasOwn.call(state.attrs, key) && key !== "className" && !hasOwn.call(attrs, key)){ - attrs[key] = state.attrs[key] - } - } if (className != null || state.attrs.className != null) attrs.className = className != null ? state.attrs.className != null From ba38410d1bc02298a2ad76ff0379d8064c5fcb49 Mon Sep 17 00:00:00 2001 From: kfule Date: Mon, 21 Oct 2024 20:52:38 +0900 Subject: [PATCH 2/7] [refactor] hyperscript: don't use isEmpty() in execSelector() --- render/hyperscript.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/render/hyperscript.js b/render/hyperscript.js index 7566be94a..ef709d5f9 100644 --- a/render/hyperscript.js +++ b/render/hyperscript.js @@ -28,6 +28,7 @@ function compileSelector(selector) { } } if (classes.length > 0) attrs.className = classes.join(" ") + if (isEmpty(attrs)) attrs = null return selectorCache[selector] = {tag: tag, attrs: attrs} } @@ -38,7 +39,7 @@ function execSelector(state, vnode) { vnode.tag = state.tag - if (!isEmpty(state.attrs)) { + if (state.attrs != null) { attrs = assign({}, state.attrs, attrs) } From 75b2f020e8f8274978a6df513a2ac77566f193e4 Mon Sep 17 00:00:00 2001 From: kfule Date: Mon, 21 Oct 2024 21:11:53 +0900 Subject: [PATCH 3/7] [refactor] hyperscript: simplify className assignment logic when state.attrs == null --- render/hyperscript.js | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/render/hyperscript.js b/render/hyperscript.js index ef709d5f9..3afd2c587 100644 --- a/render/hyperscript.js +++ b/render/hyperscript.js @@ -41,16 +41,18 @@ function execSelector(state, vnode) { if (state.attrs != null) { attrs = assign({}, state.attrs, attrs) - } - if (className != null || state.attrs.className != null) attrs.className = - className != null - ? state.attrs.className != null - ? String(state.attrs.className) + " " + String(className) - : className - : state.attrs.className != null - ? state.attrs.className - : null + if (className != null || state.attrs.className != null) attrs.className = + className != null + ? state.attrs.className != null + ? String(state.attrs.className) + " " + String(className) + : className + : state.attrs.className != null + ? state.attrs.className + : null + } else { + if (className != null) attrs.className = className + } if (hasClass) attrs.class = null From d4f8f506e507582702a67b6cc41a260d4bf0b265 Mon Sep 17 00:00:00 2001 From: kfule Date: Mon, 21 Oct 2024 21:14:55 +0900 Subject: [PATCH 4/7] [refactor] hyperscript: use Object.create(null) for selectorCache --- render/hyperscript.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/render/hyperscript.js b/render/hyperscript.js index 3afd2c587..f9a7e6782 100644 --- a/render/hyperscript.js +++ b/render/hyperscript.js @@ -6,7 +6,7 @@ var hasOwn = require("../util/hasOwn") var assign = require("../util/assign") var selectorParser = /(?:(^|#|\.)([^#\.\[\]]+))|(\[(.+?)(?:\s*=\s*("|'|)((?:\\["'\]]|.)*?)\5)?\])/g -var selectorCache = {} +var selectorCache = Object.create(null) function isEmpty(object) { for (var key in object) if (hasOwn.call(object, key)) return false From 69c00170b0de3eac1bbfe04c7b8655a8de5bd146 Mon Sep 17 00:00:00 2001 From: kfule Date: Mon, 21 Oct 2024 22:08:59 +0900 Subject: [PATCH 5/7] [refactor] performance improvement by replacing #2622 fix in render.js with another workaround in hyperscript.js The input[type] inspection at the beginning of setAttr() was called for each attribute. This had a negative impact on performance. The new workaround in execSelector() controls the order of setting attributes by reordering the keys in attrs. --- render/hyperscript.js | 7 +++++++ render/render.js | 16 ++++------------ 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/render/hyperscript.js b/render/hyperscript.js index f9a7e6782..72c3670a8 100644 --- a/render/hyperscript.js +++ b/render/hyperscript.js @@ -39,6 +39,13 @@ function execSelector(state, vnode) { vnode.tag = state.tag + // workaround for #2622 (reorder keys in attrs to set "type" first) + // The DOM does things to inputs based on the "type", so it needs set first. + // See: https://github.com/MithrilJS/mithril.js/issues/2622 + if (state.tag === "input" && hasOwn.call(attrs, "type")) { + attrs = assign({type: attrs.type}, attrs) + } + if (state.attrs != null) { attrs = assign({}, state.attrs, attrs) diff --git a/render/render.js b/render/render.js index a41244324..1f6ca3d7f 100644 --- a/render/render.js +++ b/render/render.js @@ -671,18 +671,13 @@ module.exports = function() { //attrs function setAttrs(vnode, attrs, ns) { - // If you assign an input type that is not supported by IE 11 with an assignment expression, an error will occur. - // - // Also, the DOM does things to inputs based on the value, so it needs set first. - // See: https://github.com/MithrilJS/mithril.js/issues/2622 - if (vnode.tag === "input" && attrs.type != null) vnode.dom.setAttribute("type", attrs.type) var isFileInput = attrs != null && vnode.tag === "input" && attrs.type === "file" for (var key in attrs) { setAttr(vnode, key, null, attrs[key], ns, isFileInput) } } function setAttr(vnode, key, old, value, ns, isFileInput) { - if (key === "key" || key === "is" || value == null || isLifecycleMethod(key) || (old === value && !isFormAttribute(vnode, key)) && typeof value !== "object" || key === "type" && vnode.tag === "input") return + if (key === "key" || key === "is" || value == null || isLifecycleMethod(key) || (old === value && !isFormAttribute(vnode, key)) && typeof value !== "object") return if (key[0] === "o" && key[1] === "n") return updateEvent(vnode, key, value) if (key.slice(0, 6) === "xlink:") vnode.dom.setAttributeNS("http://www.w3.org/1999/xlink", key.slice(6), value) else if (key === "style") updateStyle(vnode.dom, old, value) @@ -702,7 +697,9 @@ module.exports = function() { if (isFileInput && "" + value !== "") { console.error("`value` is read-only on file inputs!"); return } /* eslint-enable no-implicit-coercion */ } - vnode.dom[key] = value + // If you assign an input type that is not supported by IE 11 with an assignment expression, an error will occur. + if (vnode.tag === "input" && key === "type") vnode.dom.setAttribute(key, value) + else vnode.dom[key] = value } else { if (typeof value === "boolean") { if (value) vnode.dom.setAttribute(key, "") @@ -750,11 +747,6 @@ module.exports = function() { console.warn("Don't reuse attrs object, use new object for every redraw, this will throw in next major") } if (attrs != null) { - // If you assign an input type that is not supported by IE 11 with an assignment expression, an error will occur. - // - // Also, the DOM does things to inputs based on the value, so it needs set first. - // See: https://github.com/MithrilJS/mithril.js/issues/2622 - if (vnode.tag === "input" && attrs.type != null) vnode.dom.setAttribute("type", attrs.type) var isFileInput = vnode.tag === "input" && attrs.type === "file" for (var key in attrs) { setAttr(vnode, key, old && old[key], attrs[key], ns, isFileInput) From b673ed71ed2dd7adc0008994e6853a1826fddcdb Mon Sep 17 00:00:00 2001 From: kfule Date: Tue, 22 Oct 2024 00:51:51 +0900 Subject: [PATCH 6/7] [refactor] render: move isFileInput into setAttr() The isFileInput is needed only when key is "value", so moving the logic into setAttr() would not increase time of calculation. Also, the code outlook improves a bit, of course. --- render/render.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/render/render.js b/render/render.js index 1f6ca3d7f..578974cdd 100644 --- a/render/render.js +++ b/render/render.js @@ -671,9 +671,8 @@ module.exports = function() { //attrs function setAttrs(vnode, attrs, ns) { - var isFileInput = attrs != null && vnode.tag === "input" && attrs.type === "file" for (var key in attrs) { - setAttr(vnode, key, null, attrs[key], ns, isFileInput) + setAttr(vnode, key, null, attrs[key], ns) } } function setAttr(vnode, key, old, value, ns, isFileInput) { @@ -685,6 +684,7 @@ module.exports = function() { if (key === "value") { // Only do the coercion if we're actually going to check the value. /* eslint-disable no-implicit-coercion */ + var isFileInput = vnode.tag === "input" && vnode.attrs.type === "file" //setting input[value] to same value by typing on focused element moves cursor to end in Chrome //setting input[type=file][value] to same value causes an error to be generated if it's non-empty if ((vnode.tag === "input" || vnode.tag === "textarea") && vnode.dom.value === "" + value && (isFileInput || vnode.dom === activeElement(vnode.dom))) return @@ -747,9 +747,8 @@ module.exports = function() { console.warn("Don't reuse attrs object, use new object for every redraw, this will throw in next major") } if (attrs != null) { - var isFileInput = vnode.tag === "input" && attrs.type === "file" for (var key in attrs) { - setAttr(vnode, key, old && old[key], attrs[key], ns, isFileInput) + setAttr(vnode, key, old && old[key], attrs[key], ns) } } var val From d10e01d0d55fd6d3481a129e96592db2ca3d88ac Mon Sep 17 00:00:00 2001 From: kfule Date: Tue, 22 Oct 2024 00:52:30 +0900 Subject: [PATCH 7/7] remove polyfill for Object.assign() The polyfill doesn't support multiple source objects, and almost all browsers now natively support Object.assign(). --- util/assign.js | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/util/assign.js b/util/assign.js index 1695f4b3a..38a421f3f 100644 --- a/util/assign.js +++ b/util/assign.js @@ -1,10 +1,4 @@ // This exists so I'm only saving it once. "use strict" -var hasOwn = require("./hasOwn") - -module.exports = Object.assign || function(target, source) { - for (var key in source) { - if (hasOwn.call(source, key)) target[key] = source[key] - } -} +module.exports = Object.assign