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

Stop blowing away the children of DOM nodes which we render into #158

Open
brainkim opened this issue Sep 21, 2020 · 4 comments
Open

Stop blowing away the children of DOM nodes which we render into #158

brainkim opened this issue Sep 21, 2020 · 4 comments
Assignees
Labels
enhancement New feature or request

Comments

@brainkim
Copy link
Member

Currently when we call renderer.render(<App />, node), all of the children of node which Crank doesn’t own are blown away. This means that rendering into document.body will blow away the rest of the document. I would like Crank to work more cooperatively with the existing DOM; maybe we can keep track of the first and last nodes which we have rendered into a node and only render in that range.

@cristianfalcone
Copy link

cristianfalcone commented Nov 17, 2020

Will a fix for this allow an hydration render? Currently I'm having issues trying to identify when dom nodes were rendered with HTMLRenderer.

cristianfalcone/pedal#1

@brainkim
Copy link
Member Author

brainkim commented Nov 18, 2020

@cristianfalcone Can you elaborate a little on what you’re looking for? Perhaps you could add a data-ssr attribute to your routes for SSR code perhaps, which is erased or set to false on the client?

I still think that the heroic effort of reusing HTML-generated DOM nodes, and making sure they match between server and client during the initial handoff of the page is mostly unnecessary, as I’m quoted as saying in this issue (#34). Blowing away those nodes and creating new ones is fine for most use-cases, and I can think of workarounds for media elements and FOUC issues, so I haven’t been prioritizing it. Until someone creates a concrete demo of a UX issue which is caused by not reusing nodes and proves that there are no possible workarounds, I’m inclined to believe hydration is a waste of effort.

This specific issue is less about hydration and more just about being a good DOM citizen. I want to be able to call renderer.render() with document.body and not blow away everything else which was in the body element. I’ve been working with content editables more recently, which poses a similar issue, how should Crank behave when a rendered DOM node’s children is mutated by something besides the renderer. The common thread between all these issues is dealing with DOM nodes that are unknown to Crank, and I’d like to come up with a coherent philosophy at some point for how we deal with them, in a way that isn’t just blowing up like React does.

Thanks for trying out Crank! I’ve been gearing up to create a similar sort of SSR solution but hit a hard wall when I came to the issue of bundling. I think I have a good grasp about what routing, navigation, and some of the other stuff should look like, but just haven’t really gotten to writing all of the code and bringing it together, and I’ve more or less been frustrated by everything from the current state of node/deno and their http servers, to the fact that no bundler seems to have a nice programmatic API. I’m happy to share my thoughts on routing or SSR or even bundling in a GitHub Discussion or by email. I’ll try to look through pedal soon!

@brainkim
Copy link
Member Author

Ah I just realized your point while thinking about this while trying to go to sleep. If we implemented this behavior, we wouldn’t be able to blow away DOM nodes anymore, so rendering into SSR rendered trees would potentially create duplicate applications under the root node. Gotta think some more about this and maybe consider hydration then. Big sigh haha.

@cristianfalcone
Copy link

cristianfalcone commented Nov 19, 2020

Thanks for you comments @brainkim!! Glad you realize my point (I'm not that good with english). What I wanted to accomplish was exactly that, being able to reuse server rendered components. I managed to use a global flag for SSR as you suggested, but if Crank would support hydration, I wouldn't have to serialize fetch data on the server and parse that on client side in order to rebuild the component without calling fetch on client side (which I'm currently doing: https://github.com/cristianfalcone/pedal/blob/master/src/routes/article.jsx#L10, https://github.com/cristianfalcone/pedal/blob/master/src/components/document.jsx#L4, https://github.com/cristianfalcone/pedal/blob/master/src/client.jsx#L13). It'll be a matter of returning a <Copy /> of the server rendered component when running on the client with the SSR flag (and because I'm calling the component code, components will also have events attached if any, which was a concern in #34).

Regarding Pedal POC, there's no much code in there, I'm just using two really small libraries apart from Crank, https://github.com/lukeed/polka and https://github.com/lukeed/navaid, as backend and frontend routers, respectively. You'll find out they are single file libraries with a few lines of code which both share the same router path matcher (polka is compatible with express middlewares, which is a big plus). And I'm bundling for client and server side with https://github.com/rollup with dynamic imports for code spliting (app routes are lazy loaded). So if you want to take a look, it won't take you too much time. If you want to take full ownership of that repo or use the same idea as a base for another project, it's ok too, I'd be happy to see what you could do with that.

@brainkim brainkim removed the good first issue Good for newcomers label Nov 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants