Skip to content

Not sending data to the client. #325

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
nitedani opened this issue Jun 30, 2024 · 16 comments
Closed

Not sending data to the client. #325

nitedani opened this issue Jun 30, 2024 · 16 comments

Comments

@nitedani
Copy link

I am trying to integrate this with

It's almost working, my code looks like this:

import { WrapApolloProvider } from '@apollo/client-react-streaming'
import { buildManualDataTransport } from '@apollo/client-react-streaming/manual-transport'
import { useStream } from 'react-streaming'
import { renderToString } from 'react-dom/server'
import React from 'react'

export const WrappedApolloProvider = WrapApolloProvider(
  buildManualDataTransport({
    useInsertHtml() {
      const stream = useStream()
      if (!stream) {
        return () => {}
      }
      return (callback: () => React.ReactNode) => {
        const reactNode = callback()
        const injectionsString = renderToString(reactNode)
        console.log({ reactNode, injectionsString })
        stream.injectToStream(injectionsString)
      }
    }
  })
)

Issue: the data is not sent to the client. The output of the console.log above is the following:

{
  reactNode: {
    '$$typeof': Symbol(react.transitional.element),
    type: [Function: RehydrateOnClient],
    key: null,
    props: {},
    _owner: null,
    _store: {}
  },
  injectionsString: ''
}
{
  reactNode: {
    '$$typeof': Symbol(react.transitional.element),
    type: [Function: RehydrateOnClient],
    key: null,
    props: {},
    _owner: null,
    _store: {}
  },
  injectionsString: '<script>(window[Symbol.for("ApolloSSRDataTransport")] ??= []).push({"rehydrate":{},"events":[{"type":"started","options":{"fetchPolicy":"cache-first","query":"query Dragons{dragons{name first_flight diameter{feet}launch_payload_mass{lb}}}","notifyOnNetworkStatusChange":false,"nextFetchPolicy":undefined},"id":"1"}]})</script>'
}
{
  reactNode: {
    '$$typeof': Symbol(react.transitional.element),
    type: [Function: RehydrateOnClient],
    key: null,
    props: {},
    _owner: null,
    _store: {}
  },
  injectionsString: ''
}
{
  reactNode: {
    '$$typeof': Symbol(react.transitional.element),
    type: [Function: RehydrateOnClient],
    key: null,
    props: {},
    _owner: null,
    _store: {}
  },
  injectionsString: '<script>(window[Symbol.for("ApolloSSRDataTransport")] ??= []).push({"rehydrate":{},"events":[{"type":"complete","id":"1"}]})</script>'
}

As you can see, the callback is called 4 times, but it produces only 2 script tags, with no data included.
image

@nitedani
Copy link
Author

I can prepare a reproduction if it would help.

@nitedani
Copy link
Author

nitedani commented Jun 30, 2024

It works in your vite example because renderInjectedHtml is called with await, so it executes after Reflect.set(...args).
https://github.com/apollographql/apollo-client-nextjs/blob/e7a59cb26716a77bbfac659f435f89f5af8eff61/packages/client-react-streaming/src/stream-utils/createInjectionTransformStream.tsx#L91

This is the cause:
https://github.com/apollographql/apollo-client-nextjs/blob/e7a59cb26716a77bbfac659f435f89f5af8eff61/packages/client-react-streaming/src/ManualDataTransport/RehydrationContext.tsx#L102

I see that renderInjectedHtml needs to be delayed to send the data, because ensureInserted() is called before Reflect.set(...args). You are calling renderInjectedHtml with await, that delays it just enough. Is this intentional?

@nitedani
Copy link
Author

I figured it out, the following works:
Just needed to add await in front of the callback. 😄 Had to dig a little.

import { WrapApolloProvider } from '@apollo/client-react-streaming'
import { buildManualDataTransport } from '@apollo/client-react-streaming/manual-transport'
import { useStream } from 'react-streaming'
import { renderToString } from 'react-dom/server'
import React from 'react'

export const WrappedApolloProvider = WrapApolloProvider(
  buildManualDataTransport({
    useInsertHtml() {
      const stream = useStream()
      if (!stream) {
        return () => {}
      }
      return async (callback: () => React.ReactNode) => {
        const reactNode = await callback()  // <--- added await
        const injectionsString = renderToString(reactNode)
        stream.injectToStream(injectionsString)
      }
    }
  })
)

@nitedani nitedani closed this as completed Jul 1, 2024
Copy link
Contributor

github-actions bot commented Jul 1, 2024

Do you have any feedback for the maintainers? Please tell us by taking a one-minute survey. Your responses will help us understand Apollo Client usage and allow us to serve you better.

@phryneas
Copy link
Member

phryneas commented Jul 1, 2024

Hmm... I believe what you have here doesn't work:

return async (callback: () => React.ReactNode) => {
        const reactNode = await callback()  // <--- added await
        const injectionsString = renderToString(reactNode)
        stream.injectToStream(injectionsString)
      }

This delays the injection into the stream - but without blocking the stream, so React will already start to stream content that depends on the value that will only be injected later.

You'd need to pause the stream, await, inject, and then resume normal stream functionality.

PS: super cool to see more people playing with this! Please share your final result - maybe we can even add it as another integration test?

@brillout
Copy link

brillout commented Jul 2, 2024

Thanks for your answer, that makes sense.

I'm currently working on fixing it (I'm very looking forward to vike-react-apollo) but I'm having issues because the injected chunks are being inserted in the wrong place.

Is there a rule of when it's allowed to inject to the stream? I've yet to read an official guide from React about this.

PS: super cool to see more people playing with this! Please share your final result - maybe we can even add it as another integration test?

Apollo has quite a lot of goodies to make integration easy, it's quite neat. And, yes, we'd be up for an integration test!

@phryneas
Copy link
Member

phryneas commented Jul 3, 2024

There's no official guidance on when it is safe to inject - our createInjectionTransformStream is based on the implementation from Next.js in the hopes that they know what they are doing.

Generally: it seems that React might inject multiple chunks that belong together, but always does so synchronously. So waiting for the next Promise.resolve().then, setImmediate or similar should always give you a safe point in time to inject.

@phryneas
Copy link
Member

phryneas commented Jul 3, 2024

@brillout
Copy link

brillout commented Jul 3, 2024

it seems that React might inject multiple chunks that belong together, but always does so synchronously

That was my guess as well. (Funny coincidence.)

Good to know that I ain't alone with my guess.

Generally, did you see https://github.com/apollographql/apollo-client/pull/11807/files#diff-bd3d926d0da12f95cf1980b882dc8bb6f70b4e542f3673ea96f0bb21c281118f ?

That looks quite neat CC @nitedani.

@brillout
Copy link

brillout commented Jul 3, 2024

Generally: it seems that React might inject multiple chunks that belong together, but always does so synchronously. So waiting for the next Promise.resolve().then, setImmediate or similar should always give you a safe point in time to inject.

FYI that's what react-streaming originally did, but I moved away from this approach. Instead, whenever React writes, react-streaming synchronously adds React's chunk to its internal buffer (an array of chunks), so that injection order is respected. I find this approach a lot more reliable and flexible, and easier to reason about.

@phryneas
Copy link
Member

phryneas commented Jul 3, 2024

FYI that's what react-streaming originally did, but I moved away from this approach. Instead, whenever React writes, react-streaming synchronously adds React's chunk to its internal buffer (an array of chunks), so that injection order is respected. I find this approach a lot more reliable and flexible, and easier to reason about.

Interesting!

Tbh., I don't really want to overengineer that part.
For what we are doing, I see it as a crutch we have to use until React gets their story on stream injection of values from SSR to the Browser in place. I still hope that we'll get an early React 19.x minor that has that functionality natively and then we can drop it.

@brillout
Copy link

brillout commented Jul 3, 2024

I still hope that we'll get an early React 19.x minor that has that functionality natively and then we can drop it.

Me too, although I haven't heard any news about this nor injectToStream.

@phryneas
Copy link
Member

phryneas commented Jul 3, 2024

Last I heard from the team they were trying to find more granular things than an injectToStream helper, but I have no idea on their current stance.

I tried to hack together my "optimal" api by just editing React sources, and have multiple attempts, if you're interested in the exploration :)

@brillout
Copy link

I guess we can close this (I can't do it as I ain't OP), thank you @phryneas for your help!

@phryneas
Copy link
Member

Good call - I believe the release of this was just announced at https://x.com/vike_js/status/1813474021201027129, so I'm going to close this here :)

Copy link
Contributor

Do you have any feedback for the maintainers? Please tell us by taking a one-minute survey. Your responses will help us understand Apollo Client usage and allow us to serve you better.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants