Problem with 2.0 migration

I didn't test if the example migration works, but I simply cannot get 2.0.1 to work unless I remove this line in my project:

relay-nextjs/example/src/lib/relay_client_environment.ts

Line 25 in 9cb7898

if (typeof window === 'undefined') return null;

Otherwise I'll only get this error: TypeError: Cannot read properties of null (reading 'commitPayload').

If you have any ideas please let me know. I don't have a repro right now but I'll look into making one if you think there's some problem with just deleting that line.

Removing that line removes the safeguard around creating a new client environment on the server, so it looks like the server environment isn't being initialized correctly, or the page isn't passing along the preloaded query correctly. Can you paste the withRelay function call?

We have utility function we use to add the withRelay, translations etc, but even when I reduced it to this:

export default withRelay(HomePage, IndexQuery, {
  createClientEnvironment: () => getClientEnvironment()!,
  createServerEnvironment: async (
    ctx,
  ) => {
    const { createServerEnvironment } = await import('src/util/relay/createServerEnvironment');
    return createServerEnvironment(ctx.req,ctx.res,'en');
  },
});

It's still the same error. I'll look a bit more into this when I have time. Something to note is that createServerEnvironment does return a RelayModernEnvironment and the data is fetched correctly and is available in the
pageProps. Also, the error happens during SSR, not client side. Obviously could also happen in client side as well, I just haven't gotten that far yet 😄

Okay I finally found the problem. pageProps has preloadedQuery field which is not enumerable meaning that it's lost when we do {...pageProps} in one of our _app.tsx wrapper components. This behaviour has changed in v2 since it wasn't a problem earlier 🤔

Not sure if it's the only problem I have as adding our proper withRelay util call back I'm back at the same error. I'll give up for now, might look into it later 😅

Okay one more update. I finally managed to get rid of the error by finding all the places where I expanded pageProps and manually copying preloadedQuery to the new props object. But then I ran into the next error on client side: TypeError: environment.retain is not a function

@FINDarkside Any more updates? Any way we can help?

I'll still have to get back to updating it probably this week. I assume my current client side issues are possibly caused by the same thing. Assuming that's the case then the only real "issue" would be that preloadedQuery and possibly some other fields are non-enumerable. It's a bit unexpected since it's common for people to do stuff like {...props} down the line, which in this case would lose preloadedQuery and possibly some other fields. Is there some reason why these fields must be non-enumerable?

Is there some reason why these fields must be non-enumerable?

The field are non-enumerable to prevent Next from attempting to serialize data isn't supposed to. It lets us initialize a server-only store, get the data out, and carefully control what Next actually sends over in initial props. I might be possible to workaround it in another way, but would be a large refactor.

Unfortunately we have the same issue and therefore we can't upgrade from 1.x

However for us there might be another way to get it to work: we are able to remove all the _app customization that causes the same errors @FINDarkside ran into.

But if we do that, it seems Next.js tries to do Automatic Static Optimization and calls the page query at compile time, which fails because our build system isn't designed to have the backend available during compile time and our pages aren't meant to be statically generated at compile time anyway.

I tried turning this behavior off but I found no way to force it off. If there was a way, maybe we could get past the environment.retain is not a function error.

Ok so I finally gave it another go and got it working when I manually assigned preloadedQuery inside _app in our wrapper function which is needed for translations. Also added code like

Object.defineProperty(newProps, 'preloadedQuery', {
  enumerable: false,
  value: originalProps.preloadedQuery,
});

to our function which modifies Page.getInitialProps due to translations.

Note that it wasn't possible to simply change the order of which wraps the page first, translations wrapper or withRelay, since it seems like withRelay just overrides the getInitialProps instead of calling the potentially already existing one and keeping the props from that.