Skip to content

Commit

Permalink
[refactor] performance improvement by replacing #2622 fix in render.j…
Browse files Browse the repository at this point in the history
…s 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.
  • Loading branch information
kfule authored and dead-claudia committed Oct 31, 2024
1 parent 99107d5 commit b329095
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 12 deletions.
7 changes: 7 additions & 0 deletions render/hyperscript.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,13 @@ function execSelector(state, vnode) {

if (hasClass) attrs.class = null

// 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)
}

vnode.attrs = attrs

return vnode
Expand Down
16 changes: 4 additions & 12 deletions render/render.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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, "")
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit b329095

Please sign in to comment.