mithril.js
mithril.js copied to clipboard
MithrilJS
I don't exactly trust `m.trust`... 😉
Expected Behavior
m.trust should be a bit more agnostic about its surrounding environment, and it shouldn't be so overly complicated.
Current Behavior
m.trust currently uses a really ugly series of hacks to render itself. It also only works with some HTML and SVG.
Possible Solution
Let's simplify it to something closer to this:
Edit: Simplify it further
var fragment = $doc.createDocumentFragment()
function createHTML(parent, vnode, nextSibling) {
// Not using the proper parent makes the child element(s) vanish.
// var div = document.createElement("div")
// div.innerHTML = "<td>i</td><td>j</td>"
// console.log(div.innerHTML)
// --> "ij", no <td> in sight.
//
// The below code does this generically regardless of namespace or tag name.
var temp = parent.firstChild == null ? parent : $doc.createElementNS(parent.nodeName, parent.namespaceURI)
temp.innerHTML = vnode.children
vnode.dom = temp.firstChild
vnode.domSize = temp.childNodes.length
if (temp !== parent) {
var child
while (child = temp.firstChild) fragment.appendChild(child)
insertNode(parent, fragment, nextSibling)
}
}
This is significantly smaller (like about 100 bytes saved), but I need to benchmark the rendering improvement though (it's not tested there currently). It does implement the optimization of if no other children exist, it can just render directly to the node and avoid the indirection.
Steps to Reproduce (for bugs)
Context
Just noticed it was weird, and that we were mostly just doing the wrong thing here.
Your Environment
- Version used:
- Browser Name and version:
- Operating System and version (desktop or mobile):
- Link to your project:
Hi @isiahmeadows,
do you have like any examples for a weird m.trust behavior? I'm using it for almost all my user generated markdown => html output and haven't experienced any problems ... which i actually expected. The only problem that i got is when using m.trust within an array, like:
m('div', [
m('span),
m.trush('<my><custom><html> ....')
])
@ChrisGitIt There's not any real pitfalls with the current implementation unless you're using MathML (which no special case exists for). This whole bug is just about future-proofing the relevant code so we don't have to update it for every little spec change and in case browsers start adding some non-standard whatever that needs "supported". It's also to simplify and optimize it.
@barneycarroll No. We already use innerHTML, so this does nothing that doesn't already.
Although that "fixed" snippet in the bug description is incredibly wrong. parent isn't necessarily a DOM element - we'd need to pass the real parent along as well.
@dead-claudia historically IIRC innerHTML was HTML-only, and we left out MathML because of lack of cross-browser support (and user demand). A parent-aware API is the correct solution.
Aside rather than passing the parent and nextSibling around, we may use the stack-managed "global" trick this could save some stack space and some perf.
@pygy I'm aware of that historical bit.
Aside rather than passing the parent and nextSibling around, we may use the stack-managed "global" trick this could save some stack space and some perf.
Stack space is cheap - we could go well over a thousand nodes deep without issue (we aren't computing the Ackermann function here). The concern is copying, but stuff near the top of the CPU stack are normally in either registers or L1 cache already (and if we're actively using it, that's where it belongs), so it's really a balancing act in practice.
Also, we'd have to basically do shotgun surgery on our render in order to put that in. I've already tried once years ago, and quickly ran into that issue. (Also, perf didn't impress, or else I would've filed a PR back then.)