vikejs
Fallback to default injectToStream when react-streaming stream is closed
Fixes https://github.com/batijs/bati/issues/283
PS: I'm not sure why imports are reordered, it's disabled in my IDE (perhaps biome does that?)
This would await for the stream to completely end though, see docs about streamEnd at https://github.com/brillout/react-streaming.
Thus, this will probably break progressive rendering. You can test that at /examples/react-streaming. (We should probably add some progressive rendering tests at some point.)
PS: I'm not sure why imports are reordered, it's disabled in my IDE (perhaps biome does that?)
There isn't any automatic formatting, so I'm inclined to think it's something on your side.
Yup I noticed that's not the fix we need, we would need a way to check if the underlying streamOriginal is closed, but I'm not sure we have that available
I wonder why and who is trying to push to the stream even though it's already closed.
On Thu, Jun 27, 2024 at 3:58 PM Joël Charles @.***> wrote:
Yup I noticed that's not the fix we need, we would need a way to check if the underlying streamOriginal is closed, but I'm not sure we have that available
— Reply to this email directly, view it on GitHub https://github.com/vikejs/vike/pull/1718#issuecomment-2194758181, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAHVQRV53OBWK4IBSMVNGDDZJQK63AVCNFSM6AAAAABJ76XYEKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCOJUG42TQMJYGE . You are receiving this because your review was requested.Message ID: @.***>
Looking at the log, I would say Vike itself or Vite:
Error: [[email protected]][Wrong Usage] Cannot inject following chunk after stream has ended: `<script id="vike_pageContext" type="application/json">{"$$typeof":"!undefined","abortReason":"!undefined","_urlRewrite":null,"_urlRedirect":"!undefined","abortStatusCode":"!undefined","_abortCall":"!undefined","_pageContextInitIsPassedToClient":"!undefined","_pageId":"/pages/_error","routeParams":{},"data":"!undefined","pageProps":{"is404":false},"is404":false,"_isServerSideError":"!undefined"}</script><script type="module" async>
import RefreshRuntime from "/@react-refresh"
RefreshRuntime.injectIntoGlobalHook(window)
window.$RefreshReg$ = () => {}
window.$RefreshSig$ = () => (type) => type
window.__vite_plugin_react_preamble_installed__ = true
import "/@vite/client";
import "/@fs/tmp/bati-app/node_modules/.pnpm/[email protected][email protected][email protected][email protected][email protected][email protected]_@[email protected]_/node_modules/vike/dist/esm/client/client-routing-runtime/entry.js";
</script>`
at injectToStream (/tmp/bati-app/node_modules/.pnpm/[email protected][email protected][email protected][email protected]/node_modules/react-streaming/dist/cjs/server/renderToStream/createBuffer.js:12:33)
at injectHtmlFragment (file:///tmp/bati-app/node_modules/.pnpm/[email protected][email protected][email protected][email protected][email protected][email protected]_@[email protected]_/node_modules/vike/dist/esm/node/runtime/html/injectAssets/injectHtmlTags.js:48:9)
at file:///tmp/bati-app/node_modules/.pnpm/[email protected][email protected][email protected][email protected][email protected][email protected]_@[email protected]_/node_modules/vike/dist/esm/node/runtime/html/injectAssets/injectHtmlTags.js:16:26
at Array.forEach (<anonymous>)
at injectHtmlTags (file:///tmp/bati-app/node_modules/.pnpm/[email protected][email protected][email protected][email protected][email protected][email protected]_@[email protected]_/node_modules/vike/dist/esm/node/runtime/html/injectAssets/injectHtmlTags.js:10:15)
at injectToHtmlBegin (file:///tmp/bati-app/node_modules/.pnpm/[email protected][email protected][email protected][email protected][email protected][email protected]_@[email protected]_/node_modules/vike/dist/esm/node/runtime/html/injectAssets.js:41:17)
at injectAtStreamBegin (file:///tmp/bati-app/node_modules/.pnpm/[email protected][email protected][email protected][email protected][email protected][email protected]_@[email protected]_/node_modules/vike/dist/esm/node/runtime/html/injectAssets.js:25:21)
at injectStringAtBegin (file:///tmp/bati-app/node_modules/.pnpm/[email protected][email protected][email protected][email protected][email protected][email protected]_@[email protected]_/node_modules/vike/dist/esm/node/runtime/html/renderHtml.js:68:24)
This chunk is indeed authored by Vike (not Vite).
Both Vike (DEBUG=vike:stream) and react-streaming (https://github.com/brillout/react-streaming/commit/c98b1a58119e2aa917d010dfdb31e15c66d1743a) have debug logs to see what happens when. It's usually an effective way to get to the bottom of these kind of issues. But let me know if you want me to have a look at it.
But let me know if you want me to have a look at it.
Could be great yes.
I'm not sure if the stream created by react-streaming in createReadableWrapper should be kept open until further notice (but what does it even mean in this case? and how does this then apply to other frameworks supporting streaming?).
Intuitively, I would have injectToStream be a wrapper around any underlying Stream, but it would ignore close (non-error) events from underlying stream (i.e. React stream), and only listen for close events coming from "above" (i.e. Vike or Vite, I'm confused here).
For now I was testing with those instructions https://github.com/batijs/bati/issues/283#issuecomment-2194448517 (and fiddling directly in node_modules)
I tried with stream: 'node' and:
// vike-handler.ts
import { Writable } from 'node:stream';
...
const { readable, writable } = new TransformStream();
response?.pipe(Writable.fromWeb(writable));
return new Response(readable, {
status: response?.statusCode,
headers: response?.headers,
});
...
and no error, so at least it's contained to Web streams mode.
For now I was testing with those instructions batijs/bati#283 (comment) (and fiddling directly in node_modules)
I just disabled stream mode by default in Bati, so you perhaps need to manually add stream: 'web' back to +config.ts
Weirdly enough, if I try to copy the reproduction repo in vike/examples I have no issue (but streaming seems disabled) :confused:
Weirdly enough, if I try to copy the reproduction repo in
vike/examplesI have no issue (but streaming seems disabled) 😕
:face_with_head_bandage: I was testing on this branch with my "fix"...
Anyway, I removed the fix, and pushed the repo in vike/examples/bati-app for testing purpose.
Actually, I cannot reproduce anymore.
Neither on this PR nor on https://github.com/brillout/vike-stream-bug-2024-06. Can you systematically reproduce or is it random?
I just pushed this tentative fix on brillout/dev. I'm shooting in the dark as I cannot reproduce; let me know if it fixes the issue (you can test the fix by rebasing this PR onto brillout/dev).
Actually, I cannot reproduce anymore.
Neither on this PR nor on https://github.com/brillout/vike-stream-bug-2024-06. Can you systematically reproduce or is it random?
Yup still reproduce, even managed get the issue on a second machine
I just pushed this tentative fix on brillout/dev. I'm shooting in the dark as I cannot reproduce; let me know if it fixes the issue (you can test the fix by rebasing this PR onto brillout/dev).
I'll try that
After a quick pnpm dedupe, it at first seems to have fixed the issue (and at least server-side no more error). But it generates an interesting side effect on the client:
The added code after the stream closes makes React confused when trying to rehydrate on the client.
That's actually something I was expecting, let me try to reproduce on my machine.
I believe https://github.com/vikejs/vike/pull/1722 fixes it.
I ain't sure because I couldn't reproduce the error without the fix. But we can be confident this works if: you can reproduce the error by reverting the fix, then re-apply the fix and then not be able to reproduce the error.
I do not have issues anymore on #1722 but how can I make sure that streaming is actually working?
With working do you mean that it doesn't throw the error anymore or that progressive rendering works?
With working do you mean that it doesn't throw the error anymore or that progressive rendering works?
The I have no error, but I'm not sure I have progressive rendering, what's the best way to test it?
@brillout progressive rendering works both with Node Stream and Web Stream with latest vike-react and react-streaming :rocket:
Yes, that is what I was testing with my latest push https://github.com/vikejs/vike/pull/1718/commits/0cc3cbca61c5d6ab88eb0336c36453fe7593a9ec