Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions render/domFor.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@

var delayedRemoval = new WeakMap

function *domFor(vnode, object = {}) {
function *domFor(vnode) {
// To avoid unintended mangling of the internal bundler,
// parameter destructuring is not used here.
var dom = vnode.dom
var domSize = vnode.domSize
var generation = object.generation
var generation = delayedRemoval.get(dom)
if (dom != null) do {
var nextSibling = dom.nextSibling

Expand Down
3 changes: 3 additions & 0 deletions render/hyperscript.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,9 @@ function execSelector(state, vnode) {
attrs = Object.assign({type: attrs.type}, attrs)
}

// This reduces the complexity of the evaluation of "is" within the render function.
vnode.is = attrs.is

vnode.attrs = attrs

return vnode
Expand Down
131 changes: 51 additions & 80 deletions render/render.js
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ module.exports = function() {
function createElement(parent, vnode, hooks, ns, nextSibling) {
var tag = vnode.tag
var attrs = vnode.attrs
var is = attrs && attrs.is
var is = vnode.is

ns = getNameSpace(vnode) || ns

Expand Down Expand Up @@ -396,7 +396,7 @@ module.exports = function() {
}
function updateNode(parent, old, vnode, hooks, nextSibling, ns) {
var oldTag = old.tag, tag = vnode.tag
if (oldTag === tag) {
if (oldTag === tag && old.is === vnode.is) {
vnode.state = old.state
vnode.events = old.events
if (shouldNotUpdate(vnode, old)) return
Expand Down Expand Up @@ -426,7 +426,7 @@ module.exports = function() {
}
function updateHTML(parent, old, vnode, ns, nextSibling) {
if (old.children !== vnode.children) {
removeDOM(parent, old, undefined)
removeDOM(parent, old)
createHTML(parent, vnode, ns, nextSibling)
}
else {
Expand Down Expand Up @@ -585,71 +585,38 @@ module.exports = function() {
if (vnode != null) removeNode(parent, vnode)
}
}
function removeNode(parent, vnode) {
var mask = 0
function tryBlockRemove(parent, vnode, source, counter) {
var original = vnode.state
var stateResult, attrsResult
if (typeof vnode.tag !== "string" && typeof vnode.state.onbeforeremove === "function") {
var result = callHook.call(vnode.state.onbeforeremove, vnode)
if (result != null && typeof result.then === "function") {
mask = 1
stateResult = result
}
}
if (vnode.attrs && typeof vnode.attrs.onbeforeremove === "function") {
var result = callHook.call(vnode.attrs.onbeforeremove, vnode)
if (result != null && typeof result.then === "function") {
// eslint-disable-next-line no-bitwise
mask |= 2
attrsResult = result
}
}
checkState(vnode, original)
var generation
// If we can, try to fast-path it and avoid all the overhead of awaiting
if (!mask) {
var result = callHook.call(source.onbeforeremove, vnode)
if (result == null) return

var generation = currentRender
for (var dom of domFor(vnode)) delayedRemoval.set(dom, generation)
counter.v++

Promise.resolve(result).finally(function () {
checkState(vnode, original)
tryResumeRemove(parent, vnode, counter)
})
}
function tryResumeRemove(parent, vnode, counter) {
if (--counter.v === 0) {
onremove(vnode)
removeDOM(parent, vnode, generation)
} else {
generation = currentRender
for (var dom of domFor(vnode)) delayedRemoval.set(dom, generation)
if (stateResult != null) {
stateResult.finally(function () {
// eslint-disable-next-line no-bitwise
if (mask & 1) {
// eslint-disable-next-line no-bitwise
mask &= 2
if (!mask) {
checkState(vnode, original)
onremove(vnode)
removeDOM(parent, vnode, generation)
}
}
})
}
if (attrsResult != null) {
attrsResult.finally(function () {
// eslint-disable-next-line no-bitwise
if (mask & 2) {
// eslint-disable-next-line no-bitwise
mask &= 1
if (!mask) {
checkState(vnode, original)
onremove(vnode)
removeDOM(parent, vnode, generation)
}
}
})
}
removeDOM(parent, vnode)
}
}
function removeDOM(parent, vnode, generation) {
function removeNode(parent, vnode) {
var counter = {v: 1}
if (typeof vnode.tag !== "string" && typeof vnode.state.onbeforeremove === "function") tryBlockRemove(parent, vnode, vnode.state, counter)
if (vnode.attrs && typeof vnode.attrs.onbeforeremove === "function") tryBlockRemove(parent, vnode, vnode.attrs, counter)
tryResumeRemove(parent, vnode, counter)
}
function removeDOM(parent, vnode) {
if (vnode.dom == null) return
if (vnode.domSize == null) {
// don't allocate for the common case
if (delayedRemoval.get(vnode.dom) === generation) parent.removeChild(vnode.dom)
parent.removeChild(vnode.dom)
} else {
for (var dom of domFor(vnode, {generation})) parent.removeChild(dom)
for (var dom of domFor(vnode)) parent.removeChild(dom)
}
}

Expand All @@ -676,7 +643,7 @@ module.exports = function() {
}
}
function setAttr(vnode, key, old, value, ns) {
if (key === "key" || key === "is" || value == null || isLifecycleMethod(key) || (old === value && !isFormAttribute(vnode, key)) && typeof value !== "object") return
if (key === "key" || 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 Down Expand Up @@ -709,7 +676,7 @@ module.exports = function() {
}
}
function removeAttr(vnode, key, old, ns) {
if (key === "key" || key === "is" || old == null || isLifecycleMethod(key)) return
if (key === "key" || old == null || isLifecycleMethod(key)) return
if (key[0] === "o" && key[1] === "n") updateEvent(vnode, key, undefined)
else if (key === "style") updateStyle(vnode.dom, old, null)
else if (
Expand Down Expand Up @@ -743,22 +710,24 @@ module.exports = function() {
if ("selectedIndex" in attrs) setAttr(vnode, "selectedIndex", null, attrs.selectedIndex, undefined)
}
function updateAttrs(vnode, old, attrs, ns) {
if (old && old === attrs) {
console.warn("Don't reuse attrs object, use new object for every redraw, this will throw in next major")
}
if (attrs != null) {
for (var key in attrs) {
setAttr(vnode, key, old && old[key], attrs[key], ns)
}
}
// Some attributes may NOT be case-sensitive (e.g. data-***),
// so removal should be done first to prevent accidental removal for newly setting values.
var val
if (old != null) {
if (old === 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) {
if (((val = old[key]) != null) && (attrs == null || attrs[key] == null)) {
removeAttr(vnode, key, val, ns)
}
}
}
if (attrs != null) {
for (var key in attrs) {
setAttr(vnode, key, old && old[key], attrs[key], ns)
}
}
}
function isFormAttribute(vnode, attr) {
return attr === "value" || attr === "checked" || attr === "selectedIndex" || attr === "selected" && vnode.dom === activeElement(vnode.dom) || vnode.tag === "option" && vnode.dom.parentNode === activeElement(vnode.dom)
Expand All @@ -770,7 +739,7 @@ module.exports = function() {
// Filter out namespaced keys
return ns === undefined && (
// If it's a custom element, just keep it.
vnode.tag.indexOf("-") > -1 || vnode.attrs != null && vnode.attrs.is ||
vnode.tag.indexOf("-") > -1 || vnode.is ||
// If it's a normal element, let's try to avoid a few browser bugs.
key !== "href" && key !== "list" && key !== "form" && key !== "width" && key !== "height"// && key !== "type"
// Defer the property check until *after* we check everything.
Expand All @@ -789,7 +758,7 @@ module.exports = function() {
element.style = style
} else if (old == null || typeof old !== "object") {
// `old` is missing or a string, `style` is an object.
element.style.cssText = ""
element.style = ""
// Add new style properties
for (var key in style) {
var value = style[key]
Expand All @@ -800,6 +769,15 @@ module.exports = function() {
}
} else {
// Both old & new are (different) objects.
// Remove style properties that no longer exist
// Style properties may have two cases(dash-case and camelCase),
// so removal should be done first to prevent accidental removal for newly setting values.
for (var key in old) {
if (old[key] != null && style[key] == null) {
if (key.includes("-")) element.style.removeProperty(key)
else element.style[key] = ""
}
}
// Update style properties that have changed
for (var key in style) {
var value = style[key]
Expand All @@ -808,13 +786,6 @@ module.exports = function() {
else element.style[key] = value
}
}
// Remove style properties that no longer exist
for (var key in old) {
if (old[key] != null && style[key] == null) {
if (key.includes("-")) element.style.removeProperty(key)
else element.style[key] = ""
}
}
}
}

Expand Down
40 changes: 40 additions & 0 deletions render/tests/manual/case-handling.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
<!DOCTYPE html>
<html>
<head>
<meta charset="utf-8">
</head>
<body>
<p>This is a test for special case-handling of attribute and style properties. (#2988).</p>
<p>Open your browser's Developer Console and follow these steps:</p>
<ol>
<li>Check the background color of the "foo" below.</li>
<ul>
<li>If it is light green, it is correct. The style has been updated properly.</li>
<li>If it is red or yellow, the style has not been updated properly.</li>
</ul>
<li>Check the logs displayed in the console.</li>
<ul>
<li>If the attribute has been updated correctly, you should see the following message: "If you see this message, the update process is correct."</li>
<li>If "null" is displayed, the attribute has not been updated properly.</li>
</ul>
</ol>

<div id="root" style="background-color: red;"></div>
<script src="../../../mithril.js"></script>
<script>
// data-*** is NOT case-sensitive
// style properties have two cases (camelCase and dash-case)
var a = m("div#a", {"data-sampleId": "If you see this message, something is wrong.", style: {backgroundColor: "yellow"}}, "foo")
var b = m("div#a", {"data-sampleid": "If you see this message, the update process is correct.", style: {"background-color": "lightgreen"}}, "foo")

// background color is yellow
m.render(document.getElementById("root"), a)

// background color is lightgreen?
m.render(document.getElementById("root"), b)

// data-sampleid is "If you see this message, the update process is correct."?
console.log(document.querySelector("#a").getAttribute("data-sampleid"))
</script>
</body>
</html>
8 changes: 4 additions & 4 deletions render/tests/test-attributes.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,8 @@ o.spec("attributes", function() {
o(spies[0].callCount).equals(0)
o(spies[2].callCount).equals(0)
o(spies[3].calls).deepEquals([{this: spies[3].elem, args: ["custom", "x"]}])
o(spies[4].calls).deepEquals([{this: spies[4].elem, args: ["custom", "x"]}])
o(spies[5].calls).deepEquals([{this: spies[5].elem, args: ["custom", "x"]}])
o(spies[4].calls).deepEquals([{this: spies[4].elem, args: ["is", "something-special"]}, {this: spies[4].elem, args: ["custom", "x"]}])
o(spies[5].calls).deepEquals([{this: spies[5].elem, args: ["is", "something-special"]}, {this: spies[5].elem, args: ["custom", "x"]}])
})

o("when vnode is customElement with property, custom setAttribute not called", function(){
Expand Down Expand Up @@ -124,8 +124,8 @@ o.spec("attributes", function() {
o(spies[1].callCount).equals(0)
o(spies[2].callCount).equals(0)
o(spies[3].callCount).equals(0)
o(spies[4].callCount).equals(0)
o(spies[5].callCount).equals(0)
o(spies[4].callCount).equals(1) // setAttribute("is", "something-special") is called
o(spies[5].callCount).equals(1) // setAttribute("is", "something-special") is called
o(getters[0].callCount).equals(0)
o(getters[1].callCount).equals(0)
o(getters[2].callCount).equals(0)
Expand Down
Loading