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

Provide people with tabs so they can use classes as well #4284

Merged
merged 2 commits into from
Nov 7, 2024

Conversation

JoviDeCroock
Copy link
Member

@JoviDeCroock JoviDeCroock commented Nov 4, 2024

This provides people with the option to choose between the template approach or the classes approach. This is a proposal to tackle #1368

Preview

This has been applied throughout the codebase now, however one of the things I am uncertain about is how we offer buildSchema with the GraphQLDefer/... directives? Should we add an option to buildSchema? The exports defined in that chapter seem to only exist in v17 so we should explicitly flag that.

Copy link

github-actions bot commented Nov 4, 2024

Hi @JoviDeCroock, I'm @github-actions bot happy to help you with this PR 👋

Supported commands

Please post this commands in separate comments and only one per comment:

  • @github-actions run-benchmark - Run benchmark comparing base and merge commits for this PR
  • @github-actions publish-pr-on-npm - Build package from this PR and publish it on NPM

@JoviDeCroock JoviDeCroock marked this pull request as draft November 4, 2024 19:53
@JoviDeCroock JoviDeCroock force-pushed the classes-alternative branch 2 times, most recently from 076abdb to 53dae86 Compare November 6, 2024 05:55
@JoviDeCroock JoviDeCroock marked this pull request as ready for review November 6, 2024 06:41
Copy link
Contributor

@yaacovCR yaacovCR left a comment

Choose a reason for hiding this comment

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

I haven’t found the time yet to get through every line, but I love the idea of tabs!

One question I have is whether we want to call these SDL-first vs code-first …. Our SDL-first currently requires using the root value/parent method of providing resolvers but the eventual plan would be to allow resolvers as additional arguments to buildschema just like graphql tools makeExecutableSchema…

@JoviDeCroock JoviDeCroock merged commit 577a9ad into 16.x.x Nov 7, 2024
31 checks passed
@JoviDeCroock JoviDeCroock deleted the classes-alternative branch November 7, 2024 17:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants