Skip to content
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

Use server-only package instead of custom environment detection #90

Merged
merged 2 commits into from
Aug 31, 2023

Conversation

leonchabbey
Copy link
Contributor

I saw this issue when trying to fetch data from the sitemap.ts file using the getClient / registerApolloClient (like it's done for server only code).
Apparently the "client version" of React is somehow available when the sitemap is getting generated which was triggering the custom detection currently implemented.

I tried using import 'server-only instead as it is the recommended way to make sure something isn't imported from a client component and all cases seem to work:

  • From CC -> crash (✅ )
  • From RSC -> ✅
  • From sitemap.ts -> ✅

@phryneas
Copy link
Member

Apparently the "client version" of React is somehow available when the sitemap is getting generated which was triggering the custom detection currently implemented.

That would be very worrying as that client version has no working version of React.cache 🤔

Apart form that: this is a great change, and will also play nicely with apollographql/apollo-client#11175 - thank you!

I just wonder why CI didn't kick off?

@leonchabbey
Copy link
Contributor Author

leonchabbey commented Aug 30, 2023

That would be very worrying as that client version has no working version of React.cache 🤔

Yep it's really weird. I didn't investigate much more what was happening on NextJS' side but it looks like it might be a bug on their side. We obviously can't (and also don't want to) run React to generate a sitemap 😄

I will open an issue on their side about this React context issue

@phryneas phryneas merged commit b7db32f into apollographql:main Aug 31, 2023
@phryneas
Copy link
Member

I'll see that I get this out as a new patch release - thanks a ton for the PR! :)

@phryneas
Copy link
Member

And released in https://github.com/apollographql/apollo-client-nextjs/releases/tag/v.0.4.2 :)

@leonchabbey
Copy link
Contributor Author

Great! Thanks a lot for being so quick getting this merged 👏

@jerelmiller jerelmiller mentioned this pull request Sep 9, 2023
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

Successfully merging this pull request may close these issues.

2 participants