MithrilJS / mithril.js

A JavaScript Framework for Building Brilliant Applications
https://mithril.js.org
MIT License
13.98k stars 925 forks source link

Optimizing mounting to a serverside rendered HTML tree (DOM hydration) #1838

Closed dineshselvantdm-zz closed 1 week ago

dineshselvantdm-zz commented 7 years ago

Right now Mithril recalculates the VDOM while mounting over a server-side rendered HTML. Can it be optimized?

pygy commented 7 years ago

It is called DOM hydration, and it would need explicit support.

I was looking into it last autumn before being side tracked by the router.

dead-claudia commented 7 years ago

I also looked into it some in 0.2.x, and it's non-trivial to do in general. You literally have to build the initial tree differently, and it will inevitably run into complications with m.trust. You also have the secondary concern of whether the tree is even correct compared to the vnodes.

ChrisGitIt commented 7 years ago

Hi everybody,

i currently worked on a "hydration" feature for mithril. RIght now its properly working for me. There are just minor changes to mithril.js to make it work (mainly its passing my hydration function to "rendering" mithril methods. So for example m.route(document.body, '/', json_routes) ... becomes ... m.route(document.body, '/', json_routes, hydration_func)

I just experienced some smaller issues that i think are "unresolvable". For example, if your "create" a several following text nodes, it will ran into a hydration problem.

For example: m('div', [ $val1 ? 'OK': '', $val2 ? 'OK': '', ]) => theses renderings are problematic.

I thinks this is primarily the same issue as with using m.trust ... but as i write this ... i just realize that while having control about the server side rendering (https://github.com/StephanHoyer/mithril-isomorphic-example), it might be possible to enclose all subsequently following text nodes with a tag and this way having the possibility to fully rehydrate the dom on the client side.

Is anybody interested in my early alpha "rehydration" approach? My approach is a single function, currently 8 KB unzipped, commented, beautified. I might publish it, so my approach itself might be verified and extended.

Greats,

Chris

dead-claudia commented 7 years ago

@ChrisGitIt

I just experienced some smaller issues that i think are "unresolvable". For example, if your "create" a several following text nodes, it will ran into a hydration problem.

Try either joining Mithril text nodes where possible or just diffing the nodes without the backing components - that will resolve the diff.

Note that Mithril is missing a hook that would otherwise allow third-party integration with this: the ability to mount a root + component without rendering first. @pygy @tivac Would you all support such an addition, something like m.mount(root, component, {hydrated: true})?

ChrisGitIt commented 7 years ago

Hi isiahmeadows and thanks for your feedback!

Joining text nodes is very difficult. A component can return a text node, arrays can return text nodes ... to make my hydration function work, it needs two runs. The first run would be the thing that you described ({hydrated: true}), the second would be to actually assign the vnode.dom ... this actually was my first approach, and somehow i started all over ... i think i saw the opportunity to do it in one run and keep my code size small.

I would much prefer my solution right now despite the fact that two text nodes can occur ... i just have to be a little carefull writing my components and m() markup.

I think the {hydrated: true} will result in a much bigger mithril code base ... i mean like 1 or 2 more KB's, not much actually, but everybody who does not want hydration has to load 2 more KBs.

Passing a vnode accepting function as third argument is a perfect fit. It requires marginal change to mithril code base (just three functions need adjustment about the third parameter) and if you need hydration, you load and pass in the hydration function ...

Am i on the wrong track? Feedback appreciate!

dead-claudia commented 7 years ago

@ChrisGitIt

Joining text nodes is very difficult. A component can return a text node, arrays can return text nodes ... to make my hydration function work, it needs two runs. The first run would be the thing that you described ({hydrated: true}), the second would be to actually assign the vnode.dom ... this actually was my first approach, and somehow i started all over ... i think i saw the opportunity to do it in one run and keep my code size small.

Note the alternate (diffing the text nodes) would be much easier to do, at the cost of recreating text slices.

I think the {hydrated: true} will result in a much bigger mithril code base ... i mean like 1 or 2 more KB's, not much actually, but everybody who does not want hydration has to load 2 more KBs.

To clarify, that option would literally be just to skip this line, to enable third-party support (we've done similar in other areas to help mithril-node-render and a few others out).

ChrisGitIt commented 7 years ago

Hi isiahmeadows and thanks for your feedback!

I thought about your infos and i think it will unnecessarily add code to the mithril base.

Right now, it looks like all render/render.js -> createText -> createComponent etc. have to be rewritten to appreciate {hydrate:true}

My current soultion just needs marginal changes to mithril: /render/render.js function render(dom, vnodes) { ... if (dom.vnodes == null) dom.textContent = "" becomes

function render(dom, vnodes, hydrate) { ... hydrate(dom, vnodes, hooks, null);

/api/mount.js => replace two redrawService.render(root, *)

with redrawService.render(root, *, hydrate)

About the diffing text nodes: I'm currently experimenting with this. I think its possible to to properly hydrate text nodes. Its not actually joining the text nodes but to create extra text nodes and remove just the one that needs to be hydrated ... this might lead to a flicker ... i'm not totally convinced with this solution... there also might be a solution by using streams ... well, haven't dived much into them.

Any feedback appreciate!

Greats,

Chris

leeoniya commented 7 years ago

what reservations do you guys have against joining adjacent textnodes? i have not seen any issues with doing so, and it makes hydration pretty trivial [1].

[1] https://github.com/leeoniya/domvm/blob/2.x-dev/src/view/addons/attach.js

ChrisGitIt commented 7 years ago

Hi leeoniya!

And also thanks for your feedback! I just had a quick look at your provided trivial solution.

But i think the problem remains ... i think i have to recapitulate for myself:

If there is a call like m('div, ['A','B=',m(my_component)])) and var my_component = { view: function() { return Math.random() } } is given, the rendered root DOM Element would look like this=>

AB=0.23

When dom hydration takes place, m() call from before gives me a vnode like this { children: ['A', 'B=', my_component] tag: 'div' } and my root DOM Element "Node View(!!)" would look like this { nodeType: 1 nodeName: 'div' children: [{nodeType: 3, nodeValue: 'AB=0.23'}] } SOOOO ... i'm not sure if this leads to anywhere ...

When i hydrate through the VNODE, the vnode children will ALL be applied to the same DOM text node ... AND ... when there is a redraw, only the my_component would change ... and the change will directly applied to the vnode.dom attached dom node ... and this dom node is the text node with the content "AB=0.23" SOOOOO --- yeah, i think my thought were right --- > the updated vnode.dom.content would be 0.83 and not the expected AB=0.83.

OK, i'm done. I think this is a very simplified problem description, but the problems that arise while doing something like this are very tricky to track down and messes up the whole redraw cycle.

So my current work in progress solution is to create new text element WHILE hydrating, remove the suspicious "joined" DOM text node and append three dom text nodes childs to the DOM div node.

Any feedback appreciate! I'm also not sure if the problem might be easily resolved within the mithril core. Also not sure if the problem described is actually a problem. During developing my hydration function, it looks like this is a problem ... haven't tested it lately ...this might ruin several days of thinking and testing about resolving the problem ... but @isiahmeadows might have had the same problems ... so i'm confident, this IS a problem ;-)

Greats,

Chris

pygy commented 7 years ago

Alternatively, while hydrating, if you encounter several consecutive text vnodes, you can turn them into a fragment of textNodes and replace the textNode in the DOM with it.

Empty text vnodes between elements will also need special care.

Edit: Specifically:

m('p', '')
m('p', '', m('p'))
m('p', m('p'), '')
m('p', m('p'), '', m('p'))
m('p', m('p'), '', '', m('p'))
dead-claudia commented 7 years ago

@leeoniya I'm accounting for the case of fragments. Here's a contrived example of where joining text nodes would end up not matching Mithril's internal model, and Mithril doesn't currently have a layer between text nodes and its model of their slices.

const A = {
    oncreate({dom}) { /* vnode.dom is the text node's dom */ },
    view() { return ["foo"] },
}

const B = {
    view() { return [m(A), "bar"] },
}
leeoniya commented 7 years ago

you may be interested in @thysultan's feedback starting here:

https://github.com/leeoniya/domvm/issues/101#issuecomment-261228607

i don't know if the latest dio still works this way.

thysultan commented 7 years ago

It still does Hydrate.js#L54, though this only works if you have an internal data structure to represent text nodes, Element.js#L202.

dead-claudia commented 7 years ago

Mithril does have an internal data structure, but it'd require significant rewriting to avoid it.

I'm experimenting independently with a way of using slices to update text nodes and fragments while keeping a separate model for the actual DOM tree, so that the rendered internal model only sees fully normalized text nodes and an optimal tree. (It'd also make for easier hydration.)

(As for the status of this experiment, it's still local and closed-source until I actually get something functional and tested.)

jmooradi commented 6 years ago

Has any progress been made on this recently or anyway I can assist the active development?

ChrisGitIt commented 6 years ago

Hi jmooradi,

i'm currently using my implementation on a "larger site (+1000 pages) and so far it works really well. I think as soon as my project is done, i will publish my "mithril hydration" script.

dead-claudia commented 6 years ago

@jmooradi Not lately, but see here for a contributing FAQ.

@ChrisGitIt I'd love to see it. I'm personally curious how it would turn out.

jmooradi commented 6 years ago

@ChrisGitIt I'd also love to test it out before I try to take a shot at building it all myself :)

Would it make sense to have the server (mithril-node-render) return the vnode as serialized json along with the html output? The client can then unserialize and set the vnodes object on the root dom element before first render? Or is it better to parse the html into vnodes?

dead-claudia commented 6 years ago

@jmooradi

Believe it or not, I've actually profiled and found that serialized templates + frag.cloneNode(true) is far faster than document.createElement, even if you only use it once. It loads faster by 10s to 100s of milliseconds for all but the smallest of trees.

ChrisGitIt commented 6 years ago

@jmooradi

I think there is no deeper meaning to use Json and send it to the client, because client side, there is mithril to handle all dom related. Mainly Javascript SSR (Server Side Rendering / Isomorphic) has only two, but important advantages:

So my approach is just simply do the server side rendering via https://github.com/MithrilJS/mithril-node-render and then, hook in a little script that will read all DOM data and create a mithril vdom.

jmooradi commented 6 years ago

@ChrisGitIt I'm guessing when you build the vnode you have have to then also run a client render in order to bind any events set in view?

ChrisGitIt commented 6 years ago

@jmooradi: The client rendering is crucial to add events. It might be possible to add "simple" events like "onclick" on a much easier (and faster) way, but to get full mithril power, we need the full mithril vdom. So mainly i took the "Mithril first time vdom CREATION" and made it a ""Mithril first time vdom HYDRATION".

Actually, you COULD put mithril on top of every server side rendered webpage (m.mount(document.body, ...)), but this results on visible redraw (short flash of nothing). For my project, this was not acceptable. Also, the more DOM there is, the longer and more visible the redraw became. This is very noticeable on mobile devices!

I think i will upload my script and a example today. I will leave a note here when done! Any feedback or comments on my implementation is greatly appreciate!

jmooradi commented 6 years ago

Also on the subject of text nodes that I saw above, wouldn't splitting the text node during hydration work? You can compare the string to vdom to get the offset and split it to match what mithril expects. The implementation I'm thinking is that the vdom needs to be parsed normally during hydration in order the bind events but nothing is actually drawn to prevent flashing. https://developer.mozilla.org/en-US/docs/Web/API/Text/splitText

The lazy way of implementing this would to have the client completely trust that the server's html output matches the client's vdom.

jmooradi commented 6 years ago

Here my first draft at a hydrate script: https://gist.github.com/jmooradi/6f2a3d5ae279ddef76f3b42ed5c6f393

Its pretty lazy and makes a lot of assumptions that the server side rendered dom will be exactly the same as the vnodes (but I'll be adding some addition dom reading overtime). It pretty much is just the normal mithril createNodes but with all the dom creation parts stripped out, it might be a little buggy but I figured it could be a good starting place for discussion and feedback to get this feature integrated into mithril core.

Its used by calling hydrate(element, vnodes) before a render(element, vnodes)

dead-claudia commented 5 years ago

So I've got a thought for hydration:

This would be really fast to interpret, because the hydration module wouldn't be diffing anything, just reviving from a single static string.

If implementation details interest you... The bytecode would be a tree specified via a single string. - The root would be `C...`, where: - `N` is an instruction, one of: - `fC...` = Add fragment - `cIN` = Add component instance - `eL...` = Add element - `hLO` = Add raw HTML with `innerHTML` start offset - `n` = Add `null` - `tL` = Add text slice - `L` is an integer string or fragment length - `I` is an integer ID - `O` is an integer offset - `...` is a sequence of `L` instructions, with no whitespace or other characters separating them. - Integers are formatted as a modified [unsigned LEB128](https://en.wikipedia.org/wiki/LEB128#Unsigned_LEB128) with 5 value bits instead of 7, and each resulting 6-bit number encoded using the following table to ensure it's all ASCII and a valid unquoted attribute string: - 0-9 = `0-9` - 10-36 = `A-Z` - 37-61 = `a-z` - 62 = `+`, 63 = `-` In addition to the grammar above, there's one other big rule: - New child IDs must be no more than one more than the last highest child ID. Since we also validate before we even attempt to interpret the string, this lets us figure out the number of component instances we need to revive, so we can allocate a fixed-size array of child components. This saves a lot on space and time on our part, and we can use `Array.prototype.fill` (with fallback) as a hint to engines that we just want to allocate a fixed array. This format was specifically chosen to be very concise, highly compressible, and quickly interpreted. - It's concise in that even non-trivial subtrees can be expressed in a relatively small number of bytes. Consider that the entire component subtree for the example below is only 44 bytes. - It's highly compressible because the format is specifically designed to be regular, and the structure is intentionally very similar between types. Consider how even in the relatively contrived example below, there's already some repeating substructures in it, like `e1t6` and `e0`. - It's quickly interpretable because of two reasons: the grammar can be easily parsed and interpreted in streaming fashion and it aligns very well with the vnode structure. It's very amenable to a hybrid state machine- and pushdown automaton-based interpreter loop where the string's characters are iterated in the outer loop, and the verification process only needs an ID counter, a stack of lengths, and a few state variables. It's also intentionally fully within the ASCII space and valid as an unquoted HTML attribute, for a few more size gains. If you're curious how this'd look, here's a concrete example if you statically rendered @barneycarroll's [lifecycle explorer](https://flems.io/#0=N4IgtglgJlA2CmIBcBmArAOgAwBYA0IAZhAgM7IDaoAdgIZiJIgYAWALmLCAQMYD21NvEHIQIAL54a9RswBW5XgKEimhAK7UebCAIAEAGT4BzABQBKPcAA61Pfb0I2e-pqEAnPQF49ARlsOjvDOsCbeehQAurYBDk56DOE2doEOtDCmDKSktMbwlsmpRY4mGAAO6qQspsDQSC58bvDuANQteHo6DPXU8ADuegAitEIWHVk5efUTufDi5rHFDu7B6u52DIupkjEpRaFmBVtL-NSkfAgYB6YHC3vbeLtLYLRlpoRH90srbGt2BxgXm8PscHOItuDqFsfn8EvBbJDbAA3WieA7hXoDIyHWwAelxAPSUFMAHIhKRnL4SeYMETSeTnAAmal4gmlOlk+AUvQoFlQ6gAQTKZSSWyREH6pHqwEhiMFwow4slGGx4Q0Wh0AgsVieSziwT0UD4YHCmig8GIvSgur1QWciR8yVBtqVfXqpiR1D45ssXgAfDqvrbUvijSaAGThvSkYIAFQgDEabFMK2o5vcd2Deph6wizqzqTApIARuo2GwBCSOoUC7XAgIeLAIDwANbu30Bmt1uuQ7tZnZBvupEkAJXgUHctD6JPz3fMj35Q6KRZJpfLlers6X9a0Tdb7vy3k7W+3RXgGBWE6natosBjJ9PgXRPkxhhMFgfj-sqfTH8HX-sXsALBBdgIcEkAGEEFRGd-znUCvxXNcK2oKtAzA+wIGoXp3AACVjABZAx6hJAAhFZaBbKtPzrBs9zbPRTEPf10IwwJz0vScBh8Qhb3vOCv0JDISQAKjEvk2MCH9mj-NigMfeYFxo20VzQrtgIbCihHdT1vWY48BMfMNwl080MDDZS+2k9xZIwgc2IEdQyigEZ4HbI89Gs2yFPnG0+2iQztgfOUsxzDZ4UXYN1W0XQ7C8z5uzAC9hF-MMOgoSzAiLawQFIMpaChbg8xACC1lTZwujcnKOlfYZRhpCsAGU2HcLDDkiXzAqWAEgVMUwIDYDtWMfMKElJMoVjQgApRqAHkADkMApVrqGMCBCAAT36wbMyXeZLI6vylnkoLqHxQJIXxOUhTKRUJT6UgMHmvS1U0GL9A9L0fUDLZ4hbeANpMr7zxGFrHv+jbfoNUh1GLYHSHCKI-NG9SdywgadOBobUeKITiQhvQWj0Ek9AAWlJvQBHRtgJOKAd800+BXMxvTsYfPHTAJomSfJymtC0+BaaKen-0c5zmcY0z9OGvUOa54myYpsWXKEIWHiOhwBGLC0+BWJyVbcyWsY8nH9nZDJ5Z5pXqG1whdfgfXXLVi6lNFm2dZWFYwD4JFDc+1mTfZ838YBwmFd5rWPeaeBvd952QI1+wBC9n2-altnArl0PucVvmU7j3bhddpZXXdU2szYXIEL7UH3ClKx87mavuyTCo2Gb4N5kDyLbSzwGc9511GIpVFBr5dmDR+AA1YHwhXCgKQ2hAvGLXX03qXwygAD2jC5oAAbj0fKYDa+oMDQGPD5edw1uoM+L7ASI1MkPMuvA57zWot-7AhjvgxJr+llG56EjGNVcZYUJqUyg4LCOF8JERIlvL+246LNgYo3P+tpFKWRXCwdw1Ji51hhnDPSj1eqcwBh0CAHZoH2CLJXYwm5v56l-rQ1Ijd3L+jYUsYh8MlplD3PAfqHRfCdW3PtZhnUcElggRuGW3ZYHNHgcRYmLRkFLlQfuRiNDmE8NhnwioVQKH92JhgEmRNTC8NIVcYQxg2AsDDqIwuBZsGBUOj3PU+IYxsHjImMsfUM5Zj7mHK2egh5MTTOYeORQrp3AfKNaewN8wnTlHKJK3s3CmCNDwdQDBBAYFXlADaTDAium1OXUaGVArz0XsvQgCAt71CwH4PQaAsAAFJD6p3cPUvgfRSYbXqLQMsfB95P3GKYG6d1lQf3gNWCGG95hiODDUtgS94BeHqfARpLTfCtI6V032PTQj9MGXoYZFYxloSLFM10j1sTmGcYEAKF0ER3G4LleACB3pnFEDgAAnEgNpEgpAgDoAwUQGAeDZA+acFQbBRCFMBjWKAEA8qwFoGcrZW996xGxaTVFKwfn1H4LAXJ1BcUCVXlvUmpAIAAC9T56FXu4dMpNqWUqWKi9FmL6gOGxZy4odtBB8tSBgAAHDHaMBVSC0uaOtQVRQyh8DpZqO+-KIBb3HIq1IfRoD2NFQ4XwWADn5hYPACAxh2CGr8Cazp+Zr63xtVgHVgRj6otWoa8+l98wVjKDahwLr8wIEIGwANegg2iyOb0t0DgWDQHNBShEuwJrwHkcUR1WFnUYEZA-Sl4gPkxm+Wq8gTBfAAHYkBYAkJEAgTZqAtlLVQMFMhRCQHsa1LgBA1hcCYOwNgZQpT4k0GUFsxgoXGlxO2vBJAAACvgMALoAGxToGjO2AGAFAfLWWUWQpAeCtTKAi8QkRxBAA) (with added whitespace to show HTML structure): ```html
Node1
```

Edit: @barneycarroll @pygy @StephanHoyer Thoughts?

ChrisGitIt commented 5 years ago

Hi @isiahmeadows,

i had now time to think about your approach for some time ... what problem does it solve? Is it for "Static" hypertext only? Where are the lifecyle methods? Why use the bytecode when each rendered hyperscript element need to get a dom attached anyway? I have to admit: I don't understand your approach. Maybe its best to have a real life example (proper component and lifecycle methods defined, then your hydrate approach explained in correlation etc.).

Wish you all had a merry Christmas!

Greets,

Chris

dead-claudia commented 5 years ago

The bytecode exists not for the DOM but to track the vnode state when rendered, particularly text, trusted, fragment, and component vnodes.

It does duplicate the DOM structure a little, but I chose that over a data-mithril-hydrate on every element for its children (it's more compressible), and it acts as a convenient sanity check on the DOM itself. Also, the bytecode carries only "is this an element", not any details on tag name, attributes, or children, so it's only very mildly duplicative. On Wed, Dec 26, 2018 at 05:28 ChrisGitIt notifications@github.com wrote:

Hi @isiahmeadows https://github.com/isiahmeadows,

i had now time to think about your approach for some time ... what problem does it solve? Is it for "Static" hypertext only? Where are the lifecyle methods? Why use the bytecode when each rendered hyperscript element need to get a dom attached anyway? I have to admit: I don't understand your approach. Maybe its best to have a real life example (proper component and lifecycle methods defined, then your hydrate approach explained in correlation etc.).

Wish you all had a merry Christmas!

Greets,

Chris

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/MithrilJS/mithril.js/issues/1838#issuecomment-449946090, or mute the thread https://github.com/notifications/unsubscribe-auth/AERrBMjAr5Xi93ToBrCBUbtsD_cRUKSkks5u809CgaJpZM4NN7bM .

dead-claudia commented 5 years ago

Update: I'll instead just use a list of comma-separated m.trust node counts for data-mithril-hydrate, with no spaces. It's optional and can be omitted if no such nodes exist in the tree. I'll simply throw a TypeError if things don't match, so it's still blazingly fast and relatively simple.

The rest can be inferred just as quickly from the DOM and resulting initial vnode tree as it could a bytecode string, and 99% of the temporary stuff could just be local variables. For example:

dead-claudia commented 5 years ago

I may implement both approaches and see which one gives me better performance. In either case, I won't diff anything: I'll throw a TypeError if anything ends up different from what I expected.

gilbert commented 5 years ago

Any updates on this? Very interested in getting both-side rendering in mithril.

dead-claudia commented 5 years ago

@gilbert It's most certainly planned, but it's low priority, so we haven't gotten to it yet.

barneycarroll commented 5 years ago

Been processing the technical proposal in the background - sorry for delayed response. How do we reconcile component identities?

dead-claudia commented 5 years ago

@barneycarroll I suggest we just don't worry about it. Let's assume the tree is correct, and if the DOM differs, we throw an error instead of trying to patch it. That way, the developer finds out much quicker that something went wrong in the SSR translation.

gilbert commented 5 years ago

This is a minor point, but I think an incorrect tree should throw a warning instead of an error, and redraw as normal to overwrite the DOM. That way the page still functions, even if there's a flash of white on load.

barneycarroll commented 5 years ago

@isiahmeadows gotcha. The initial front end draw will be the implicit reconciliation. Divergence (render state X on server, state Y on JS initialisation) is not going to happen.

dead-claudia commented 5 years ago

@gilbert If you've got it set up for SSR, you have the framework for ensuring the data is consistent. Divergence means either a bug in your pipeline or your client just got MitM'd. The first case is generally pretty easy to fix: it's almost always just a content delivery problem, not a problem with your client-side app, and fixing it is sometimes as easy as running a single command. The second case is, of course, nothing you can do about, but is obviously something your user should be aware of.

Honestly, most issues this would provide are things that would force users to do things the right way in the first place.


I would be open to offering an escape hatch for tolerating outdated information in attributes and/or text differing only in content or existence, patching those up as necessary by just clearing (if text) and updating the value, but I would still prefer the element structure to throw an error on mismatch, and I'd rather keep the escape hatch down to only what's expected to change. The API for tolerating changes to attributes or text could be as simple as this magic property: nohydrate: ["array", "of", "attributes"] or nohydrate: "single-attribute".

leeoniya commented 5 years ago

Divergence means either a bug in your pipeline or your client just got MitM'd.

the main case where i've run into divergence has been with third-party scripts creating dom elements. sometimes browser plugins do this also (e.g. password managers). this situation becomes more of a nuisance if the entire document is SSRd from the root <html>, but less so if your app root is an element inside <body>.

EDIT: err, i guess this isn't SSR divergence but DOM divergence post-hydration (assuming the hydration is done prior to third party script exec), so affects future redraws.

dead-claudia commented 5 years ago

@leeoniya Most of the time, that divergence is either in text nodes or attributes. As for utilities screwing with the DOM after initial redraw, we've had a recent issue come up with Font Awesome's SVG insanity - they changed the DOM underneath us and installed a mutation observer to continue mucking with it, but offered no hook to allow us to re-sync our own model. So in that case, it's just not playing well with virtual DOM frameworks.

Font Awesome does have an API that would let us hook into their IR and render it how we want it, though, and I'm slapping together a gist to port their react-fontawesome (which uses SVGs internally) to Mithril.

gilbert commented 5 years ago

I understand it's supposed to be the end dev that sets everything up, but what's the benefit of throwing an error and breaking the app instead of a warning hiccup behavior?

dead-claudia commented 5 years ago

@gilbert Two reasons:

  1. It's much simpler to implement - the hydration function doesn't need to concern itself with diffing or DOM building, and it can work completely independently from m.render.
  2. It encourages best practice of tying the two versions together and automating the process, so it's less likely the problem gets ignored. It's opinionated, I know, but Mithril doesn't really stray away from injecting opinions on obvious ticking time bombs.
dead-claudia commented 5 years ago

There's a third reason, too: what if it's just missing one vnode in an inner component, like this?

var hasData = false
var Data = {view: function () {
    return [
        m("div.data", ...),
    ]
}}

var Header = {view: function () {
    return [
        m("nav.stuff", ...),
        m("div.divider"),
        hasData ? m(Data) : null,
        m("div.page", currentPage),
    ]
}}

var Home = {view: function () {
    return [
        m(Header),
        m(Body, ...),
        m(Footer),
    ]
}}

// In your initialization
hasData = true
m.hydrate(root, m(Home))
<!-- Pretend the extra whitespace isn't here. -->
<!-- Original tree -->
<div id="root">
    <nav class="stuff">...</nav>
    <div class="divider"></div>
    <div class="page">Current Page</div>
    ...
</div>

<!-- Rendered tree -->
<div id="root">
    <nav class="stuff">...</nav>
    <div class="divider"></div>
    <div class="data">...</div>
    <div class="page">Current Page</div>
    ...
</div>

Specifically, how should it consume <div class="page">? The obvious error correction would be that it should infer a hole and insert a <div class="data"> before it, but this makes it much harder to iterate thanks to the need for lookahead, and Mithril lacks the static analysis needed to know this generically. The naïve, purely iterative error-correcting method would say it should change the class to data, replace the text content with the appropriate children, then insert <div class="page">, resulting in a lot of unnecessary DOM mutation. In fact, this iterative method is basically a full diff + patch pass.

There's also the question of how to handle keyed fragments whose entries are in the wrong order. The obvious solution is to ignore the keys and just update them appropriately, but then you're just doing a giant unkeyed diff/patch that's doing a lot of unnecessary DOM mutation. If you want to avoid mutation, then you're turning an already complex problem into an even more complex statistical problem by trying to infer the keys for each of the DOM nodes before then diffing them. (And by that point, you might as well just do the unkeyed diff/patch.)


Or, TL;DR: Requiring consistency is easy. Fixing mistakes is not. What's obvious in error correction isn't easy, and what's easy in error correction is often wrong. Forcing the developer to follow best practices is a lot easier than trying to figure out how to correct their mistakes, and Mithril isn't in the business of magic here.

gilbert commented 5 years ago

Sorry I wasn't clear. What I'm suggesting is: upon an inconsistency, abandon the hydration process altogether and rerender from the top level node, completely from scratch. I'm not suggesting anything fancy or magical, which is why it was a minor point in my mind.

dead-claudia commented 5 years ago

@gilbert You could always catch the error and manually remount. 😉

ACXgit commented 5 years ago

Detecting the inconsistency would be very useful to spot a misbehavior in the isomorphic process (from my perspective, any difference between the two rendered DOMs would mean that something has gone wrong). If throwing an error is the cleanest approach, go for it :thumbsup:

ChrisGitIt commented 5 years ago

@isiahmeadows: Can i catch a hydration error on components level? With the "onmatch" Lifecycle? Otherwise, i'm not sure if a complete redraw would be a good idea. Maybe just a "parent" redraw would be a good idea...

dead-claudia commented 5 years ago

@ChrisGitIt I'd instead call onremove on every successfully mounted component, clear the incomplete tree, and then throw the error. You could detect hydration error in this case by checking in onremove that no other lifecycle method (except oninit) was called.