brillout / react-streaming

React Streaming. Full-fledged & Easy.
MIT License
216 stars 14 forks source link

Allow passing async function to injectToStream #40

Closed nitedani closed 3 months ago

nitedani commented 3 months ago

Example:

stream.injectToStream(async () => {
  const injectionsString = await something()
  return injectionsString
})

It would "block" the stream until the async function is resolved, while queuing other chunks in the meantime. When the async function is resolved, it would write the resolved value to the stream, then continue with the queued chunks.

The following is not the same, the stream can close while we are waiting for something

const injectionsString = await something()
stream.injectToStream(injectionsString)

It would be nice for https://github.com/vikejs/vike-react/pull/125, I think this is the only missing piece for the PR. It's working as-is, but can't send the last chunk for hydration, because the stream is closed.

With this implemented, I could mimic this await behavior: https://github.com/apollographql/apollo-client-nextjs/blob/e7a59cb26716a77bbfac659f435f89f5af8eff61/packages/client-react-streaming/src/stream-utils/createInjectionTransformStream.tsx#L104

Why await is needed in the linked code, is a different issue: https://github.com/apollographql/apollo-client-nextjs/issues/325

brillout commented 3 months ago

This is actually something I've been wondering. As soon as the stream provided by React closes, the wrapper stream (created by react-streaming) is closed, causing a race condition when things are injected at the end of the stream.

Passing a promise or an async function to injectToStream() works only if injectToStream() is called before React finishes. Otherwise there is still a race condition.

I wonder if there is another (more reliable?) way to tell react-streaming to not close the wrapper stream too early?

brillout commented 3 months ago

How about this:

const stream = useStream()
const makeClosableAgain = stream.doNotClose()
// ...
stream.injectToStream(someChunk)
// ...
makeClosableAgain()
nitedani commented 3 months ago

I think it's not good and unpredictable for my use case because it can send the chunk too late. We need to "pause" the stream and respect the order of injectToStream calls. https://github.com/apollographql/apollo-client-nextjs/issues/325#issuecomment-2199664143

Example issue with this: Call 1:

const stream = useStream()
const makeClosableAgain = stream.doNotClose()
const someChunk = await something() 
stream.injectToStream(someChunk)
makeClosableAgain()

Call 2:

const stream = useStream()
stream.injectToStream("something that depends on Call 1")

Output: "something that depends on Call 1" someChunk

Passing an async function allows us to pause the stream until it resolves and respect the order of injectToStream calls: Call 1:

stream.injectToStream(async () => {
    const someChunk = await something() 
    return someChunk;
})

Call 2:

stream.injectToStream("something that depends on Call 1")

Output: someChunk "something that depends on Call 1"

Passing a promise or an async function to injectToStream() works only if injectToStream() is called before React finishes.

I think this is ok.

I think passing an async function and doNotClose have separate use cases, and both can be good for different things.

brillout commented 3 months ago

Makes sense, let me see if I can implement chunks as async functions.

brillout commented 3 months ago

Both implemented and released.

brillout commented 3 months ago

You can now pass a chunk promise. It isn't exactly what you wanted (you cannot pass an async function) because I think passing chunk promises is more accurate, but let me know if it doesn't work out.