Re-purpose / reuse root for SSR hydration / prerendering. #319

jorgebucaran · 2017-07-30T13:25:03Z

Our current hydration implementation is broken! We are simply avoiding the duplication of your SSR nodes, but we are still patching everything from scratch. 😆

We could improve it this way:

function hydrate(element, map) {
  return element
    ? {
        tag: element.tagName.toLowerCase(),
        data: {},
        children: map.call(element.childNodes, function(element) {
          return element.nodeType === 3
            ? element.nodeValue
            : hydrate(element, map)
        })
      }
    : element
}

Unfortunately, this still does not take into account empty text nodes, which if not trimmed, would yield a tree that is not consistent with the new tree returned by the view, causing patch to do more work than it needs if we were hydrating correctly.

A more complex solution could be explored, but then how much is enough and can this be moved outside core?

I've been thinking about this today and considered many alternatives, and here is my proposal.

Re-purpose root for SSR-use for what-would-be the old element during the initial render.

I originally chose the name root because it was short and simple, but now I think that root should describe your application's top element: <main>, <div>, etc., and NOT the HTML element to act as a container / host of your application tree (which is what we do now).

How will this affect existing apps?

Apps not using `root`.

Everything will continue to work as usual. 🎉

Apps using `root`.

Say you have this HTML:

<body>
  <div id="app"></div>
</body>

and used root like this:

app({
  view: () => <h1>Hello.</h1>,
  root: document.getElementById("main")
})

Now your app will render overwriting / obliterating <div id="app">, so the final HTML will look like this:

<body>
  <h1>Hello.</h1>
</body>

This is similar to how React behaves.

SSR

Using root for SSR pages would allow us to remove this part from core.

element = document.querySelector("[data-ssr]")

And use instead:

app({
  ...,
  root: document.querySelector("[data-ssr]")
})

Or a different data attribute; it's up to you.

Finally, the old node for the initial render. Instead of introducing a new property, I propose using the load event, currently unused.

app({
  events: {
    load(state, actions, root) {
      return myOwnHydrationFormula(root)
    }
  }
})

so we can assign that result value to the node inside core like this:

node = emit("load", root)

Using a mixin for that would work as well. Obviously, we'd call that mixin Hydrator!

app({
  mixins: [Hydrator]
})

SSR Example

A full SSR page example would work like this:

<html>

<head>
  <script defer src="bundle.js"></script>
</head>

<body>
  <main data-hyperapp-ssr>
    <h1>My app.</h1>
  </main>
</body>

</html>

app({
  view: () => 
     <main>
        <h1>My app.</h1>
     </main>,
  mixins: [Hydrator],
  root: document.querySelector("[data-hyperapp-ssr]"),
})

/cc @andyrj @ngryman @SkaterDad @lukejacksonn @zaceno

The text was updated successfully, but these errors were encountered:

jorgebucaran · 2017-07-30T14:45:42Z

By the way, with this now it's also possible to do what until now was impossible. Use the <body> inside the view.

app({
  root: document.body,
  view: () =>
    <body>
       ...
    </body>  
})

zaceno · 2017-07-30T22:55:29Z

I really liked @Swizz 's idea of providing emit to the load event, so by default, the app would return emit.

I cant say for sure which is most rational/useful to pass emit("load", ...) - emit or root? But my hunch and personal opinion is that it should be emit. Esp if it's possible to solve SSR anyway, with a mixin.

jorgebucaran · 2017-07-31T02:22:31Z

@zaceno The load event must receive the root element so that you can hydrate it if you want. An alternative event could be devised as well.

By the way, this has nothing to do with the return value of the app() call. We can continue to return emit as usual.

andyrj · 2017-07-31T05:55:00Z

I still think you would need node, not root passed to the load event @jbucaran .

Worrying about the extra work of hydration seems extreme since it only ever runs once. But I am completely onboard with moving this out of core if that's what you want.

zaceno · 2017-07-31T06:06:39Z

@jbucaran Isn't the idea that app will return the result of the load-event? Or am I getting old and new app event names mixed up?

jorgebucaran · 2017-07-31T06:30:37Z

@zaceno I should have made it clearer that we can't reuse load for that anymore (unless we introduce an event dedicated only for hydration).

So, hyperapp 0.11.0 will continue to return emit.

zaceno · 2017-07-31T06:49:03Z

Oh, yes, I misunderstood - thanks for clarifying!

jorgebucaran · 2017-07-31T10:05:11Z

I still think you would need node, not root passed to the load event @jbucaran.

I don't see why that would be needed, but can you provide an example?

Worrying about the extra work of hydration seems extreme since it only ever runs once. But I am completely onboard with moving this out of core if that's what you want.

Is it really extreme?

Before

app({
  view: () => 
     <main>
        <h1>My app.</h1>
     </main>
})

After

app({
  view: () => 
     <main>
        <h1>My app.</h1>
     </main>,
  mixins: [Hydrator],
  root: document.querySelector("[data-hyperapp-ssr]"),
})

The new approach has more code and requires more user intervention, but it also makes it very obvious and at the same time offers more flexibility as it leaves it up to you to implement hydration.

I like how it forces the user to target the root [data-hyperapp-ssr] while at the same time making this property more consistent with what other frameworks (React) do and without breaking existing apps not using the prop (only requiring little changes to those using it).

SkaterDad · 2017-07-31T13:00:52Z

👍

I'm in favor of this idea. Replacing the root node makes more sense, and it's one less useless <div> wrapper around an app. Also good that we can now specify what attributes/id to target.

However you decide to move the hydration out of core, that's probably for the best also. No reason to ship code in core that 75% of your users won't be needing (number pull out of thin air).

jorgebucaran · 2017-07-31T14:08:31Z

@SkaterDad It also lets you render a <body> tag. 😄

jorgebucaran · 2017-07-31T14:20:53Z

By the way, I want to clarify that I do plan to use SSR in apps that require SSR, e.g., systems that want to adopt hyperapp, but are already largely SSR-based like a rails app.

But I don't see why we should include it in core when we could use a mixin just as well.

P.S: Obviously this.

andyrj · 2017-07-31T14:57:48Z

@jbucaran the node variable is required for hydration in general, but root is already declared by the user so can be sent to the mixin by the user rather trivially. Without node the result of your Hydrator.events.load() can't provide the app with the vnode skeleton. Maybe it is accessible via some js voodoo that I am missing but app is basically a closure and the node var is hidden from the mixin unless I am missing something.

jorgebucaran · 2017-07-31T15:00:37Z

@andyrj Actually events can return values. I wrote this above, but here it is again for convenience:

app({
  events: {
    load(state, actions, root) {
      return myOwnHydrationFormula(root)
    }
  }
})

jorgebucaran · 2017-07-31T15:01:41Z

\cc everyone When I refer to root now, I am not talking about the "application tree container", but rather what a root actually is, your application top-level element. For those familiar with the source code, root is what was previously known as element and root is now host. (Could be called parent just as well, but host made sense to me.)

andyrj · 2017-07-31T15:04:20Z

I apologize for missing that, I see what you are going to do, you are right node is not necessary if you do that. Still getting my coffee this morning heh.

jorgebucaran · 2017-07-31T15:05:24Z

@andyrj With this new information, how do you feel about the proposal? You are who originally brought SSR to HyperApp, so I value your opinion here! 🙏

EDIT: (Everyone's as well, but you know what I mean ^^')

andyrj · 2017-07-31T15:07:38Z

But I still think adding root to the load event is un-necessary.

var root = document.querySelector("[data-hyperapp-ssr]")
app({
  view: ...,
  mixin: [Hydrator(root)],
  root
})

Edit: just to clarify I am fine with either way I just don't see the need to add root to the args of load is all.

jorgebucaran · 2017-07-31T15:10:20Z

@andyrj Yeah, sure, it's just for convenience.

andyrj · 2017-07-31T15:28:46Z

I do have one more concern with this proposal. If we overwrite the target node during init that will cause issues for my current webpack hmr setup. If I target a node with id="root" and our app obliterates it, it does make the final output "cleaner", but then in module.hot I have no element to target for the re-initialization of app(). 😞

This wouldn't be a problem if we didn't have the weird append default case as I could just use body without specifying a root...

jorgebucaran · 2017-07-31T15:30:32Z

@andyrj overwrite the target node during init that will cause issues for my current webpack hmr setup

I am sure there must be a workaround, since overwriting the target node is how, e.g., React behaves.

jorgebucaran · 2017-07-31T15:32:48Z

@andyrj then in module.hot I have no element to target for the re-initialization of app()

Can you share some code with the problem? I have no idea what you mean.

andyrj · 2017-07-31T15:38:40Z

I haven't messed with react official for some time, but last I checked it left the render target in tact, as do inferno and preact.

I will use your proposed example above as a base.

<body>
  <div id="app"></div>
</body>

/* app.js */
import { h, app } from "hyperapp";

export default (state, target) => {
  app({
    view: () => 
       <main>
          <h1>My app.</h1>
       </main>,
      root: target
  })
};

/* index.js */
import app from "./app";

var target = document.getElementById('app');
app(state, target);

if (module.hot) {
  module.hot.accept("./app", () => {
    // blows up below because target no longer exists in the document if we obliterate it...
    app(window.state, target)
  })
}

edit: corrected code example for basic webpack hmr to show where the problem will show up if render target is removed from dom

jorgebucaran · 2017-07-31T15:43:50Z

@andyrj Are you sure you don't know how to fix this? 😏

andyrj · 2017-07-31T15:50:35Z

I don't see a good work around, I would need to

document.body.innerHTML='<div id="app"></div>'

before the call to app() in module.hot, so back to the flickering lol. Don't change what you want to do based around my issue, I am just saying it will cause issues for my use case of wanting ssr with hmr without flickering. I can always keep my private fork going... I still despise having to add data attributes and not being able to SSR to body tag to keep the strange append use case so I will likely always be using my private fork. But I could see this being strange to others that also use webpack-hmr.

jorgebucaran · 2017-07-31T16:04:18Z

@andyrj I can see two clean ways.

Using rehydration!

<body>
  <div id="app">
   <!-- rest of the tree (or just the top-level element a.k.a root) -->
  </div>
</body>

export default (state, root) =>
  app({
    view: () => 
       <div id="app">
          <h1>My app.</h1>
       </div>,
      root
  })

If your hydrator is doing its job correctly, then HyperApp's patch() should not end up replacing the element, but reusing the root (and as many children as provided).

Simulating the previous behavior (no-hydration)

<body>
  <main> <!-- safe -->
      <div id="app"></div> <!-- will be replaced -->
  </main>
</body>

const root = document.getElementById("app")
// ...

A third workaround is to call getElementById("app") twice, but I don't see why you'd prefer that.

Let me know if this makes sense.

MatejBransky · 2017-07-31T21:00:03Z

@andyrj @jbucaran Just a quick notice: I'm trying HMR and it's working without flickering at least if you don't use SSR (I haven't used SSR yet so I don't know). Now I'm testing HMR usage in one game project with the latest changes in API. So webpack-hmr is perfectly doable for client side dev. 👍

andyrj · 2017-08-01T04:13:43Z

LGTM! Looks great on my projects this will let me get rid of my fork I have been using. 💯 👍

edit: talking about #317 of course...

jorgebucaran · 2017-08-09T10:54:19Z

Implemented in #317.

jorgebucaran added bug Something isn't working discussion enhancement New feature or request labels Jul 30, 2017

jorgebucaran changed the title ~~Re-purpose root for SSR hydration / prerendering.~~ Re-purpose / reuse root for SSR hydration / prerendering. Jul 31, 2017

jorgebucaran closed this as completed Aug 9, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Re-purpose / reuse root for SSR hydration / prerendering. #319

Re-purpose / reuse root for SSR hydration / prerendering. #319

jorgebucaran commented Jul 30, 2017 •

edited

Loading

jorgebucaran commented Jul 30, 2017 •

edited

Loading

zaceno commented Jul 30, 2017

jorgebucaran commented Jul 31, 2017 •

edited

Loading

andyrj commented Jul 31, 2017

zaceno commented Jul 31, 2017

jorgebucaran commented Jul 31, 2017

zaceno commented Jul 31, 2017

jorgebucaran commented Jul 31, 2017

SkaterDad commented Jul 31, 2017

jorgebucaran commented Jul 31, 2017

jorgebucaran commented Jul 31, 2017 •

edited

Loading

andyrj commented Jul 31, 2017

jorgebucaran commented Jul 31, 2017

jorgebucaran commented Jul 31, 2017 •

edited

Loading

andyrj commented Jul 31, 2017 •

edited

Loading

jorgebucaran commented Jul 31, 2017 •

edited

Loading

andyrj commented Jul 31, 2017 •

edited

Loading

jorgebucaran commented Jul 31, 2017

andyrj commented Jul 31, 2017 •

edited

Loading

jorgebucaran commented Jul 31, 2017 •

edited

Loading

jorgebucaran commented Jul 31, 2017

andyrj commented Jul 31, 2017 •

edited

Loading

jorgebucaran commented Jul 31, 2017

andyrj commented Jul 31, 2017 •

edited

Loading

jorgebucaran commented Jul 31, 2017 •

edited

Loading

MatejBransky commented Jul 31, 2017 •

edited

Loading

andyrj commented Aug 1, 2017 •

edited

Loading

jorgebucaran commented Aug 9, 2017

Re-purpose / reuse root for SSR hydration / prerendering. #319

Re-purpose / reuse root for SSR hydration / prerendering. #319

Comments

jorgebucaran commented Jul 30, 2017 • edited Loading

How will this affect existing apps?

Apps not using root.

Apps using root.

SSR

SSR Example

jorgebucaran commented Jul 30, 2017 • edited Loading

zaceno commented Jul 30, 2017

jorgebucaran commented Jul 31, 2017 • edited Loading

andyrj commented Jul 31, 2017

zaceno commented Jul 31, 2017

jorgebucaran commented Jul 31, 2017

zaceno commented Jul 31, 2017

jorgebucaran commented Jul 31, 2017

Before

After

SkaterDad commented Jul 31, 2017

jorgebucaran commented Jul 31, 2017

jorgebucaran commented Jul 31, 2017 • edited Loading

andyrj commented Jul 31, 2017

jorgebucaran commented Jul 31, 2017

jorgebucaran commented Jul 31, 2017 • edited Loading

andyrj commented Jul 31, 2017 • edited Loading

jorgebucaran commented Jul 31, 2017 • edited Loading

andyrj commented Jul 31, 2017 • edited Loading

jorgebucaran commented Jul 31, 2017

andyrj commented Jul 31, 2017 • edited Loading

jorgebucaran commented Jul 31, 2017 • edited Loading

jorgebucaran commented Jul 31, 2017

andyrj commented Jul 31, 2017 • edited Loading

jorgebucaran commented Jul 31, 2017

andyrj commented Jul 31, 2017 • edited Loading

jorgebucaran commented Jul 31, 2017 • edited Loading

Using rehydration!

Simulating the previous behavior (no-hydration)

MatejBransky commented Jul 31, 2017 • edited Loading

andyrj commented Aug 1, 2017 • edited Loading

jorgebucaran commented Aug 9, 2017

jorgebucaran commented Jul 30, 2017 •

edited

Loading

Apps not using `root`.

Apps using `root`.

jorgebucaran commented Jul 30, 2017 •

edited

Loading

jorgebucaran commented Jul 31, 2017 •

edited

Loading

jorgebucaran commented Jul 31, 2017 •

edited

Loading

jorgebucaran commented Jul 31, 2017 •

edited

Loading

andyrj commented Jul 31, 2017 •

edited

Loading

jorgebucaran commented Jul 31, 2017 •

edited

Loading

andyrj commented Jul 31, 2017 •

edited

Loading

andyrj commented Jul 31, 2017 •

edited

Loading

jorgebucaran commented Jul 31, 2017 •

edited

Loading

andyrj commented Jul 31, 2017 •

edited

Loading

andyrj commented Jul 31, 2017 •

edited

Loading

jorgebucaran commented Jul 31, 2017 •

edited

Loading

MatejBransky commented Jul 31, 2017 •

edited

Loading

andyrj commented Aug 1, 2017 •

edited

Loading