feat: support teleports, add bodyHtml{Start,End} hooks (#88, fix #87))

4350pChris commented 1 month ago

@brillout Let me know what you think.

brillout commented 1 month ago

Good idea to also apply it in onRenderClient(), although how about we use some kind of delimiters to remove previously added HTML? For example:

<body>
<!-- bodyHtmlStart begin -->
<!-- bodyHtmlStart finish -->
<div id="app"></div> 
<!-- bodyHtmlEnd begin -->
<!-- bodyHtmlEnd finish -->
</body>

In principle, it should be possible to remove all DOM nodes between two DOM node comments.

Once Vike's support for cumulative for non-serializable values (e.g. functions) is released, I think it would be nice to make bodyHtml{Start,End} cumulative so that Vike extensions can use it without overriding the user-defined settings. I guess we can already merge this, and then make them cumulative after Vike's new version is released.

4350pChris commented 1 month ago

Good idea to also apply it in onRenderClient(), although how about we use some kind of delimiters to remove previously added HTML? For example:
<body>


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


</body>
In principle, it should be possible to remove all DOM nodes between two DOM node comments.

Once Vike's support for cumulative for non-serializable values (e.g. functions) is released, I think it would be nice to make bodyHtml{Start,End} cumulative so that Vike extensions can use it without overriding the user-defined settings. I guess we can already merge this, and then make them cumulative after Vike's new version is released.

What benefit does this bring? Also I'm not quite sure how to implement this... Would this only apply to onRenderHtml?

If you're not opposed I would merge this as is and make a separate branch for those things.

Btw I'll try to refactor the other packages using cumulative hooks, great job!

brillout commented 1 month ago

My thinking is that, because of https://vike.dev/config#inheritance, some teleport HTML won't be relevant for the next page. Leaking / not removing that HTML may lead to bugs? E.g. some modal not being removed?

If you're not opposed I would merge this as is and make a separate branch for those things.

In my experience / for me, being nitpicky on accuracy is worth it. But, if you prefer, for vike-vue we can eagerly merge things and fix bugs as they arise.

brillout commented 1 month ago

Also, if you're busy feel free to let me know; I'm happy to give a hand wherever you think makes sense.

4350pChris commented 4 weeks ago

@brillout @pdanpdan I think this is ready. Lmk what you think.

brillout commented 4 weeks ago

Now that we use cumulative hooks, how about we make bodyHtml{Start,End} cumulative as well?

brillout commented 4 weeks ago

I wonder, from a DX perspective, could it be nice to also support bodyHtmlStart: string | (pageContext: PageContext) => string for simple use cases?

// /pages/some-page/+config.js

export default {
  // No need to create +bodyHtmlStart.js (because `string` is serializable whereas `Function` isn't).
  bodyHtmlStart: '<div id="toast"></div>'
}

Everything else LGTM!

vikejs / vike-vue

feat: support teleports, add bodyHtml{Start,End} hooks (#88, fix #87)) #88