fix: document body undefined #518
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
| server.transformIndexHtml(req.url || '', html).then((content) => { | ||
| res.statusCode = 200; |
There was a problem hiding this comment.
This is not related to the PR, but it's a minor change, since we used to set the status 200 even though the .then might never work and the promise might reject.
So we just bring it when we're sure the promise is getting resolved.
| // enqueue `mainJs` first in the body to avoid the document.body undefined error, before we used to inject it in the head which sometimes made it load before the document.body was loaded | ||
| controller.enqueue( | ||
| encoder.encode( | ||
| `<script src="${config.basePath}${config.srcDir}/${config.mainJs}" async type="module"></script>`, | ||
| ), | ||
| ); | ||
|
|
There was a problem hiding this comment.
Here, this adds <script ...> multiple times.
npm run examples:dev:04_promise can reproduce it.
| @@ -102,6 +104,16 @@ globalThis.__WAKU_PREFETCHED__ = { | |||
| if (scriptsClosed) { | |||
| return; | |||
| } | |||
| if (!mainJsSent) { | |||
| // enqueue `mainJs` first in the body to avoid the document.body undefined error, before we used to inject it in the head which sometimes made it load before the document.body was loaded | |||
There was a problem hiding this comment.
My oversight, this should be DEV only, right?
npm run examples:prd:04_promise
There was a problem hiding this comment.
This should in general send the mainJs one time, weird error, do you know why that happens?
There was a problem hiding this comment.
Here's the fix #528
main.tsx doesn't exist on PRD, right?
There was a problem hiding this comment.
Yes, what exists then if that does not exist? because as far as I checked the website, there was an alternative. And we need that to be in the body too, so the behaviour is normalized between two environments.
There was a problem hiding this comment.
it's bundled by vite.
it sounds like you just miss a point. do you understand why we get 404?
There was a problem hiding this comment.
But the error that this PR fixed was reproduced with async too, so I don't think that can always be true and we need to be sure about that.
There was a problem hiding this comment.
My understanding is that the issue this PR fixed is for HMR in dev mode. I never see such an issue in PRD.
There was a problem hiding this comment.
Yes it fixed the HMR issue after the refresh in DEV, but I was just concerned how it'd be handled in PRD. That's it. sorry for the confusion.
There was a problem hiding this comment.
So, my question for you is how Vite+React does it. My guess is that all scripts are in <head>.
There was a problem hiding this comment.
Yep, they're in the head! because the body is always sent in the initial template (index.html)
|
Hmm, interesting, the goal is to have it in the body so we always sure that
document.body is available. wdyt?
…On Fri, 23 Feb 2024, 16:00 Daishi Kato, ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In packages/waku/src/lib/renderers/html-renderer.ts
<#518 (comment)>:
> @@ -102,6 +104,16 @@ globalThis.__WAKU_PREFETCHED__ = {
if (scriptsClosed) {
return;
}
+ if (!mainJsSent) {
+ // enqueue `mainJs` first in the body to avoid the document.body undefined error, before we used to inject it in the head which sometimes made it load before the document.body was loaded
See #528 <#528> for the fix. Yes,
it's already in the head in PRD. It's already done in the build step, so we
don't need to do anything in the runtime process.
—
Reply to this email directly, view it on GitHub
<#518 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AJBMICBLW7TWSRNBA4VCSR3YVCDXVAVCNFSM6AAAAABDTBSRVOVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTQOJYGAYDCMRZG4>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
|
I think usually document.body is available with "async" script. |
Partially resolves #344
When refreshing a waku page (website), the hydration failed and as result, hmr would not work.
The reason behind the failure, is that for the next refreshes, the
/main.tsxfile is optimized and warm so it gets resolved before the<body>is getting streamed to the browser since it was in the<head>and that loads before the<body>, what that mean is thedocument.bodywould be undefined during the hydration and React would throw an error since that is undefined. One solution was to restart the server so we get/main.tsxcold again.What this PR does is that it puts the script of
/main.tsxin the body so it gets resolved after the body is being streamed and as a result, document.body would be never beundefined.This also fixed the hmr issue after the page manual refresh.