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

Updates Apollo Server Examples to use Apollo Server 4 & @as-integrations/next #42771

Merged
merged 4 commits into from Nov 14, 2022
Merged

Conversation

idoob
Copy link
Contributor

@idoob idoob commented Nov 11, 2022

Closes #42769

Description

This PR address #42769 by updating the api-routes-apollo-server, api-routes-apollo-server-and-client and api-routes-apollo-server-and-client-auth examples to use Apollo Server 4 and @as-integrations/next, which is the Apollo Server Next integration package. The PR also updates the three examples to use Typescript. The functionality of the examples is the same.

Documentation / Examples

  • Make sure the linting passes by running pnpm build && pnpm lint
  • The "examples guidelines" are followed from our contributing doc

closes #33545
closes #30082
closes #21984
closes #10413

@ijjk ijjk added the example bug Issue was opened via the example bug report template. label Nov 11, 2022
@balazsorban44
Copy link
Member

balazsorban44 commented Nov 11, 2022

Looks like this would also close these:

#33545
#30082
#21984

Likely #10413 too.

We should probably merge most Apollo examples into a single one, as there are currently 6 different versions.

@leerob
Copy link
Member

leerob commented Nov 11, 2022

Code seems okay, but yeah agree on merging into one.

@idoob
Copy link
Contributor Author

idoob commented Nov 11, 2022

The api-routes-apollo-server-and-client and api-routes-apollo-server-and-client-auth are very similar with the only major difference being the inclusion of auth. They both focus on supporting the Apollo Client with SSR and Apollo Client rehydration. On the other hand, the api-routes-apollo-server, is a barebones example that only uses Apollo Server and not the client. This is also an example for building static pages.

Would it be prefered to have a single Apollo Server + Apollo Client example? Based on the content of the examples, maybe only the api-routes-apollo-server-and-client and api-routes-apollo-server-and-client-auth examples should be merged?

Copy link
Member

@balazsorban44 balazsorban44 left a comment

Choose a reason for hiding this comment

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

Next.js apps are usually hybrid (meaning they have both static and dynamic pages), so I think we could really merge everything into a single example, showcasing anything we need in one project. For now, I think we can merge this PR, as it resolves a bunch of issues, thank you! 💚

If you are interested in helping out with the merging of the examples, you can open a new PR with your suggested changes and we can continue the discussion there. 👍

@kodiakhq kodiakhq bot merged commit 081b8fb into vercel:canary Nov 14, 2022
@glasser
Copy link

glasser commented Nov 16, 2022

Thanks @idoob and @kodiakhq!

@glasser
Copy link

glasser commented Nov 16, 2022

FWIW it looks like there's still an Apollo Server 2 example in examples/with-apollo-neo4j-graphql (which also uses neo4j-graphql-js which also appears to be deprecated). Maybe it would be reasonable to just delete that one?

@leerob
Copy link
Member

leerob commented Nov 16, 2022

+1 I would just delete it if deprecated.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
example bug Issue was opened via the example bug report template.
Projects
None yet
5 participants