search

glimmerjs / glimmer-application

[MOVED] This package is now part of the Glimmer.js monorepo
https://github.com/glimmerjs/glimmer.js
30 stars 13 forks source link

-in-element should not overlap #94

Closed chadhietala closed 6 years ago

chadhietala commented 6 years ago

What is going on here?

The top level render with -in-element just happened to work but is not really reflective of how it should work. What I mean by this is that doc.body is going to contain the DOM nodes that are the remote roots. This is like a recursive render call where our root node is the body but all the remote elements are children of the body. -in-element is intended to be used such that there is no overlap from where the application is rendered and where the remote roots are at.

Since there is no DOM emitted by the top level template and is merely a mechanism for splatting out components we should be able to just use the head as the root element passed to render.

Why Change This

As I was working on implementing rehydration for in-element it was pretty clear that this would run into issues where serialized elements would get blown away upon rehydrating because hydration occurs from the parentNode down. This meant that when we ran across a "remote" e.g. <div id="app"></div> it did not know what to do as remotes are supposed to be outside of the entry node and jump to them to fill them in.

Other Solutions

It is likely that we could just use a DocumentFragment here, but that creates a leak as the fragment will never be appended. Likely ok, but by spec we should always have a head element to use.

rwjblue commented 6 years ago

Seems reasonable to me.

krisselden commented 6 years ago

On the surface this seems to be sidestepping an issue not really addressing it and seems weird to me.

pittst3r commented 6 years ago

@krisselden An alternative I think is if parentNode were an element we appended to the body. That fixes the overlap problem but prevents people from being able to do renderComponent('foo-bar', document.body).

krisselden commented 6 years ago

@robbiepitts this is a bounds serialization issue, if you can create the bounds on the server, we should be able to serialize it, this isn't an ownership issue, parentNodes are always elements, elements can have multiple bounds. This is in fact side-stepping the issue not fixing it.

locks commented 6 years ago

This repo has been merged with glimmerjs/glimmer.js (into a monorepo setup). I am not sure if this issue is still applicable, but if you could confirm it is still an ongoing concern and open it over there that would be very helpful.

Sorry for the noise, but thank you for your help!