Skip to content
This repository has been archived by the owner on Jan 4, 2018. It is now read-only.

Render composed DOM #6

Closed
bedeoverend opened this issue Jul 26, 2017 · 13 comments
Closed

Render composed DOM #6

bedeoverend opened this issue Jul 26, 2017 · 13 comments

Comments

@bedeoverend
Copy link
Contributor

bedeoverend commented Jul 26, 2017

Not really an issue - more a question / throwing out an idea. Is it worthwhile to try render the composed DOM, rather than the Shadow and Light side by side? I think there's a few pros / cons here:

Pros

  • Crawled as composed DOM by non-js SEO / regular HTML scrapers (e.g. social media shares)
  • Much more accessible initial render in non-js envs - though probably not interactive
  • Lowers priority of rehydration (maybe?) given composition is already done

Last point there is making the assumption that the priority of Shadow Root hydration is for composition - not sure if this is correct? Also style encapsulation, but go into that below...

Cons

  • Much more complex render script on the server
  • More complex rehydration script on the client
  • Non-JS still not accomplished unless style encapsulation is also emulated

That last point I feel is a biggun - to truly be able to give static rendered without any JS, you'd need to shim the styles and inject them into the head. You'd then need to move the styles back out into their respective roots on rehydration.

I feel most of the cons are complexity, but quite probably perf as well. I also expect I'm missing some large problems that would come out of this...

Thoughts?

@treshugart
Copy link
Member

Yes! This is definitely something I want to try and do because content becomes out of order. I'm currently at a loss for how to do this simply for many reasons you mentioned. One of the biggies here is that any content in a slot becomes default content once it's hydrated, which may be unexpected, or cause issues. Not with Skate probably because it will update it on the initial render.

All of your pros are spot on, but I think rehydration priority should still be high because of your last con :). I don't really care about catering to non-JS, at least yet. This simplifies things because you don't need to worry about encapsulation simulation. If you did want to do this, using a CSS-in-JS library gets you most of the way there there's just some edge cases because you don't have true encapsulation until hydrated.

That last point I feel is a biggun - to truly be able to give static rendered without any JS, you'd need to shim the styles and inject them into the head. You'd then need to move the styles back out into their respective roots on rehydration.

To address this directly, I don't feel we need to cater to non-JS.

To address the first two cons, they're definitely concerning if we were to go down this path. I feel it's worth spiking to see how far we can take it and what's involved.

@bedeoverend
Copy link
Contributor Author

Great! So I did do a POC. But it's very much a POC - instead of injecting the slots, I replaced them with <slot-node> a la <shadow-root>, meaning slots now need to be a part of the rehydration script.

but I think rehydration priority should still be high because of your last con

So I'll check back, but hydrating with script tags after every root was way too heavy, time to hydrate quadrupled. Given that, I did the hydration in one big script tag at the end of the string - the hydration was then a lot faster than original non-composed render (I think ~300ms vs ~500ms). But I believe this means FOUC if the encapsulation only kicks in at the very end?

At this point, you'd want to introduce the style encaps. emulation just to prevent the FOUC, regardless of the JS env - but, that might hurt performance too much...that part should definitely be dug into further.

Given not catering for non-JS environments (which I agree, shouldn't be a priority right now), this would then be catering for scrapers / SEO. Perhaps as an immediate option, the render function could take a flag to render composed vs flat? This would allow users to sniff the user-agent and supply a composed DOM where necessary, but use flat for better user experience.

@treshugart
Copy link
Member

So I'll check back, but hydrating with script tags after every root was way too heavy, time to hydrate quadrupled.

On my machine the difference between 10k script tag rehydrations and 10k custom elements was negligible and was faster when not optimised. Or is this test when also rehydrating <slot-node />?

But I believe this means FOUC if the encapsulation only kicks in at the very end?

Shouldn't so long as nothing breaks the current paint mid-render (this might be likely) but this wouldn't work with streaming then, right?

Perhaps as an immediate option, the render function could take a flag to render composed vs flat?

Maybe. I think I'd much rather try and address the performance issue if possible and introduce something like that as a last resort.

@bedeoverend
Copy link
Contributor Author

Or is this test when also rehydrating

Ah yeah sorry - that was when I adjusted script tag hydration for <slot-node /> but it was a pretty naive implementation.

but this wouldn't work with streaming then, right?

Ah of course, didn't think of streaming.

Sounds good re: addressing perf then if a must look at flag for composed. I'll put the code up in a fork at least - ultimately it sounds like best bet is trying to keep with script hydration (for streaming) and just work on perf yeah? Also that style encapsulation is something that can come later as needed.

@treshugart
Copy link
Member

treshugart commented Jul 26, 2017

I'll put the code up in a fork at least - ultimately it sounds like best bet is trying to keep with script hydration (for streaming) and just work on perf yeah?

Sounds good. Maybe having a perf hit for the sake of ergonomics is okay until it's prudent to start using custom elements instead.

Also that style encapsulation is something that can come later as needed.

I'm still not convinced this will ever be necessary if we rehydrate components immediately. Only if we target non-JS will it ever be an issue and I'm terrified of the implementation that caters for this, haha!

@treshugart
Copy link
Member

I got curious and started messing with this in the composed-dom branch. I got it rendering, even through nested slots, and removing the normal light DOM (so it doesn't dupe). The rehydration script is a bit sketchy, though. Maybe we can collab on that? I've made you a contributor to the Skate org, so you should have access.

@bedeoverend
Copy link
Contributor Author

Great - thanks! I'll have a dig around. Are you happy for me to push to that branch, or prefer if I work off of another? Don't want to clobber on your work.

@treshugart
Copy link
Member

treshugart commented Jul 26, 2017 via email

@treshugart
Copy link
Member

treshugart commented Jul 26, 2017 via email

@treshugart
Copy link
Member

treshugart commented Jul 27, 2017

So I think I have a rehydration algorithm close: http://jsbin.com/kuhevi/21/edit?html,console,output. The hardest part here is figuring out what is slotted content and what hosts it needs to go to upon rehydration. The <light-dom> marker is necessary (I think) because if you have a <slot> next to some text, there's no way to tell where the boundary of the composed slot was.

@bedeoverend
Copy link
Contributor Author

bedeoverend commented Jul 27, 2017

Nice! Did you decide to break up the scripts to avoid using something like querySelector, perf issues? I wasn't sure whether more scripts or querySelector style stuff would hurt more overall..? Not quite sure what you mean by the <light-dom> DOM issue.

I had a shot - http://jsbin.com/civuqan/8/edit?html,output - much closer to the original script, and all bundled into one, but relies on getElementsByTagName to fetch the slots. I don't think it'll run into the issues you've flagged - but I'm not quite sure I understand them, so it quite possibly does ha.

@bedeoverend
Copy link
Contributor Author

Bah, realised there's a bug in the slot rehydration part - should work fine on just one slot, but in named slots it'll break down. Should be an easy patch.

@treshugart
Copy link
Member

Closing this as we've gotten most of the way there. I raised separate issues (including #10 for finishing up slots).

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

No branches or pull requests

2 participants