fixing async loader for v3 - #952
Conversation
| const CLI_DATA = { preRenderData }; | ||
| root = render(h(app, { CLI_DATA }), document.body, root); | ||
| const doRender = | ||
| process.env.NODE_ENV !== 'production' || root.tagName !== 'script' |
There was a problem hiding this comment.
wondering if we need to recommend folks use id="app" here. We could have L35 do:
let root = document.getElementById('preact_root') || document.body.firstElementChild;Thoughts? It would help us avoid browser extensions potentially changing the value of firstElementChild on us.
There was a problem hiding this comment.
yes! I really really want to do this. This stream lines everything for us
There was a problem hiding this comment.
totally agree with it. :) Always wanted to do this one
|
Looks perfect aside from the one question about assuming firstElementChild. I think we can move forward with this regardless. @prateekbh Only other thing would be if we want to consider having something for Preact 8? Or are we officially dropping support for it as of this PR? |
I guess lets keep it simple |
|
@prateekbh right, just currently CLI V3 supports Preact 8 and X at the same time. |
|
Does this handle route transitions as well now? |
|
Ahh good point. This needs to only throw during hydration! |
4 participants
TO BE MERGED after: preactjs/preact#2259
What kind of change does this PR introduce?
Bug fix
Did you add tests for your changes?
No
Summary
Does this PR introduce a breaking change?
No