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

Working with pre-rendered html from a static-site-generator #390

Closed
archywillhe opened this issue Jan 24, 2016 · 13 comments
Closed

Working with pre-rendered html from a static-site-generator #390

archywillhe opened this issue Jan 24, 2016 · 13 comments

Comments

@archywillhe
Copy link
Contributor

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 and index.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, the index.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 do render(v) (where render = createApp(document.body)), it would re-render everything in document.body with HTML elements that can be manipulated depending on how the virtual element v is defined. So let's say in the index.html, the children of document.body are the exact same elements to be rendered by the first call of render(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.)

@archywillhe
Copy link
Contributor Author

Oh, I just saw this thread: #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)

@archywillhe archywillhe changed the title Working with pre-rendered html Working with pre-rendered html from a static-site-generator Jan 24, 2016
@anthonyshort
Copy link
Owner

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.

@archywillhe
Copy link
Contributor Author

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?

@archywillhe
Copy link
Contributor Author

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 👍

@anthonyshort
Copy link
Owner

^ 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.

@archywillhe
Copy link
Contributor Author

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)).

@anthonyshort
Copy link
Owner

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 :)

@archywillhe
Copy link
Contributor Author

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.

@archywillhe
Copy link
Contributor Author

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.

@archywillhe
Copy link
Contributor Author

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?

  1. 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).

  2. 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.

@archywillhe
Copy link
Contributor Author

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?

@archywillhe
Copy link
Contributor Author

In the end I think it is good to use an approach similar to react.

@anthonyshort
Copy link
Owner

Yeah I'm happy to go with the checksum approach. That will solve most of the use-cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants