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

Example: Add with-relay #33892

Closed
wants to merge 2 commits into from
Closed

Conversation

jesstelford
Copy link
Contributor

@jesstelford jesstelford commented Feb 2, 2022

Adds examples/with-relay which uses the latest version of Relay known as "Relay Hooks".

Relay Modern is now (ironically?) out of date. I have removed the existing examples/with-relay-modern example as per @alunyov's suggestion to avoid confusion.

Note that I have used the SWAPI GraphQL API in with-relay. This differs from the other examples (with-apollo, with-relay-modern, etc) which use the Prisma example GraphQL API because I was getting errors when attempting to query it: prisma-labs/nextjs-graphql-api-examples#4

Documentation / Examples

  • Make sure the linting passes by running yarn lint

@alunyov
Copy link
Contributor

alunyov commented Feb 2, 2022

This looks cool!

What do you think about making just one example: with-relay? (Renaming old with-relay-modern) and adding the changes form this PR to it?

@dylanOshima
Copy link

Just to note if this ever gets shipped: I pulled your PR and was getting hydration errors in the initial launch. These errors go away when I go to another page and comes back, thus I suspect it has something to do with some mismatch in the initial launch. Not familiar enough with Relay to give more help unfortunately :(.

Other side note is that the relay-modern example seems to be busted, I'm getting missing fragment errors on initial start-up. So I think deleting it and replacing it with this one would be a much cleaner approach for n00bs like me.

@jesstelford jesstelford force-pushed the with-relay-hooks branch 2 times, most recently from 08e5977 to cb4fe99 Compare April 26, 2022 06:32
@jesstelford
Copy link
Contributor Author

@dylanOshima I've fixed those hydration errors: It was my fault for rendering a <p> inside a <li>. I've simplified the HTML so those errors don't happen anymore 👍️

@alunyov Good idea! I've removed with-relay-modern, and added this Relay Hooks example as with-relay. Much cleaner :)

I've also expanded the example to include lazy loading & render-as-you-fetch with usePreloadedQuery 🚀

@jesstelford jesstelford changed the title Example: Add with-relay-hooks Example: Add with-relay Apr 27, 2022
@MoOx
Copy link
Contributor

MoOx commented May 31, 2022

Would be nice to have this merged as a reference, it's confusing out there between outdated with-relay-modern and relay-nextjs not being straight forward to setup (and probably at the same time "too complicated" and not flexible enough to work easily with something like next-auth).

@@ -1,49 +0,0 @@
# Relay Modern Example
Copy link
Member

@balazsorban44 balazsorban44 Jun 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's not delete this file and rather mention that this example is now moved.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find this counter productive. It's will only add more confusion to new comers now that "relay modern" is just "relay".

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason we don't want to remove old examples is that there might exist a lot of links out there that still point to that location. We want the developer to know that the example has moved.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So remove all the example code, but leave a single README with a link to the new example?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct 👍

@leerob
Copy link
Member

leerob commented Jan 4, 2023

Closing as stale – if you would like to continue this, please open a new PR and we can take a look. Thank you so much! 🙏

@leerob leerob closed this Jan 4, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 4, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
examples Issue/PR related to examples
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants