Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[refactor] Refactoring of hyperscript.js and render.js, including performance improvements #2983

Merged
merged 9 commits into from
Oct 31, 2024
46 changes: 23 additions & 23 deletions render/hyperscript.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,10 @@
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 = {}
var selectorCache = Object.create(null)

function isEmpty(object) {
for (var key in object) if (hasOwn.call(object, key)) return false
Expand All @@ -27,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}
}

Expand All @@ -37,32 +39,30 @@ 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
}

for (var key in state.attrs) {
if (hasOwn.call(state.attrs, key) && key !== "className" && !hasOwn.call(attrs, key)){
attrs[key] = state.attrs[key]
}
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
} else {
if (className != null) attrs.className = className
}
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 (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
23 changes: 7 additions & 16 deletions render/render.js
Original file line number Diff line number Diff line change
Expand Up @@ -671,25 +671,20 @@ 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)
setAttr(vnode, key, null, attrs[key], ns)
}
}
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)
else if (hasPropertyKey(vnode, key, ns)) {
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
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,14 +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) {
// 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)
setAttr(vnode, key, old && old[key], attrs[key], ns)
}
}
var val
Expand Down
8 changes: 1 addition & 7 deletions util/assign.js
Original file line number Diff line number Diff line change
@@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you just drop this module entirely and use Object.assign directly in the places that use it?

Shouldn't impact performance, but will provide a small bundle size win and it's one less source file to worry about.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I removed the module.