Closed archywillhe closed 8 years ago
Oh, I just saw this thread: https://github.com/dekujs/deku/issues/190
Is my question the same as laterne's? (I haven't worked with react before so I couldn't tell if we were asking the same question)
That's something I want to add soon. At the moment it just blasts everything away and re-renders to be safe. We could just leave the HTML in there and try and render as normal, but if even one element is wrong it could blow up the diff and weird things will happen.
To do it properly we'd need some sort of hash of the content so we can tell that the server-rendered content is exactly the same.
Another option would be to make it "self-healing", in that if it gets to a node and notices that it's incorrect it destroys the node and recreates it rather than throwing an error. That would probably be the most user-friendly approach and would probably take place somewhere near this line.
At the moment it just blasts everything away and re-renders to be safe.
I see, this explains these lines in app/index.js.
Cool. I like the self-healing idea. So how do you plan to parse the pre-existing HTML into a virtue element? (so that we can diff it with the to-be-rendered vnode and return a set of actions to be feed into patch in update.js) Any library you have in mind?
Oh wait. I think I understand what you mean now. You don't plan to parse the pre-existing HTML into a virtue element. Instead, you intend to let the vnode (i.e., which is passed into the DOM renderer) to represent the DOM (even if it may be different from the actual virtue representation of the DOM) and only have it fixed when the difference is detected at a updateChild
event.
That's truly minimalistic and elegant :+1:
^ Yup that'd be the plan. That might be a bigger feature. But I'll have a look at it this weekend. I really want to ship the final v2.
What do you think of this implementation:
Inside updateChild
, instead of forEach
, we use something like this
let isSuccessful = actions.every(action =>{
update(childNodes[index],action)
})
if(! isSuccessful) removeAndInsert(vnode, index, path)
and for the function returned by patch
, we have it returned DOMElement
(like usual) or false
based on if the action performed is successful (i.e., we would need to have every Actions.case to return a boolean value to indicate the action's success.)
If you think this is looking fine I can have some tests written & the self-healing feature implemented (according to this) and send you a PR around this friday or saturday (and then just let me know how you think about it (e.g. if there's any part needs to be improved or reimplemented)).
Awesome! If you want to work on that, that would be fantastic. I'm not 100% sure about the implementation yet.
You'd need to do something around checking the tagName
, children
count, and maybe compare all the attributes and attributes count. If they don't match the previous virtual element, then you should trash it and just run createElement
on the next virtual element, otherwise just run updateChild
as normal. Hopefully that makes sense.
The main downside is that these extra operations will slow down the diffing and updating process. So we'd probably want to only do this on the first run eventually.
It seems like it could be slightly complex, but not impossible. If you have any questions about the code within deku just let me know and I'll help you out :)
You'd need to do something around checking the tagName, children count, and maybe compare all the attributes and attributes count. If they don't match the previous virtual element, then you should trash it and just run createElement on the next virtual element, otherwise just run updateChild as normal. Hopefully that makes sense.
Yup, that's what I planned to do.
The main downside is that these extra operations will slow down the diffing and updating process. So we'd probably want to only do this on the first run eventually.
That's what I thought as well.
It seems like it could be slightly complex, but not impossible. If you have any questions about the code within deku just let me know and I'll help you out :)
Sure! Thanks. I will get started then.
I also think we should include an attribute for the container to indicate that it has been pre-rendered i.e. in create
:
if(container && container.childNodes.length > 0 && !container.attributes.preRendered)
container.innerHTML = ''
and in the local create
function
if (container && !container.attributes.preRendered)
container.appendChild(node)
Once the feature is implemented, the preRendered
attr should be mentioned in the doc regarding createApp
. Blasting everything away if there's something inside should remain the default behavior.
So at the moment I'm coding according to the following specification:
for container with pre-rendered HTML (i.e., with preRendered
attribute)
1) nothing happens when createDOMRenderer
(no cleaning up innerHTML)
2) self-healing only happens at the second call to render
:
at the first call to render
we take it as that everything is rendered nicely
at the second call to render
all incorrect parts (if there's any) would all be corrected (i.e., self-healing)
According to this, the developer may face a situation where:
in the html:
<div id="container" preRendered><span>Meow</span></div>
in the js:
let render = createDOMRenderer(document.getElementById("container")
render(<span>Thrr</span>)
//nothing would happen
render(<span>Thrr</span>)
//self-healing should be executed.
which is to say, according to the specification, in the second call to render, the action sameNode
will need to examine the accuracy of the Virtual DOM's description of the HTML element, and in this case, runs a self-healing to the <span>
. This may slow down the performance of the second call to render
dramatically (imagine every sameNode
action will bring forth a checking for differences between the vnode and the HTML element: after all these checking, the app concludes that there is no self-healing to be executed because they are the same).
Let's consider the specification where in the second call to render
, not all incorrect parts (if there's any) need to be corrected. This means we should have self-healing be present at every call to render (excluding the first call when render
does nothing). It may sound like a huge slow-down to the diffing and updating process, but if we keep the things we do to a minimal (e.g., in updateChildren
, only checking if the index
is within the length of childNodes
), the performance difference should be negligible.
So now the question is: what is the purpose of the self-healing feature?
do we want self-healing to heal everything that isn't pre-rendered properly?
If that is the case, for apps with hundreds and thousands of elements, I believe react's checksum approach would be more efficient performance-wise than diving into the structure to look for the differences, no matter how it is implemented (because for a complete self-healing we will need to compare every element: this should be avoided).
or do we simply want self-healing to automatically fix exceptions/errors that may be caused by things that aren't pre-rendered properly?
If that is the case, the self-healing feature is more of a way we use for exception/error-handling in the scenario when the pre-rendered elements only have small errors (which is to say, we trust that the pre-rendered HTML are correct most of the time). This self-healing feature will then need to be included in every updateChild
and as a result we should keep the things we do to a minimal such that it doesn't affect the performance of the diffing and updating process.
So for now, should I implement the self-healing
to be called in every updateChild
, or one that ensures everything is healed by the first call to updateChild
(which I don't think is a good idea)? Or one relying on adler-32 checksum similar to react?
In the end I think it is good to use an approach similar to react.
Yeah I'm happy to go with the checksum approach. That will solve most of the use-cases.
I'm still pretty new to the Virtual DOM paradigm so I've got a question about working with pre-rendered html in deku:
Suppose I am using a static-site generator like Hakyll and I have it compiled an
index.html
andindex.js
for the home page.The
index.html
works just fine by itself so that anyone using w3m or any non-JS browser can still use the site. On the other hand, theindex.js
is a deku script that adds in some nice js magic to the UI (for those with up-to-date browsers). According to the doc, the first time I dorender(v)
(whererender = createApp(document.body)
), it would re-render everything indocument.body
with HTML elements that can be manipulated depending on how the virtual elementv
is defined. So let's say in theindex.html
, the children ofdocument.body
are the exact same elements to be rendered by the first call ofrender(v)
.Is there a way to avoid the re-rendering? (From the doc it doesn't look like the diff-ing algorithm would be able to tell if they are the same.)