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

Write about @oneOf in the graphql-js documentation #4290

Merged
merged 2 commits into from
Nov 8, 2024
Merged

Conversation

JoviDeCroock
Copy link
Member

Currently input-unions and by extension the @oneOf directive aren't present in the documentation. I have opted to put this into the advanced section. The copy might be up for improvement, honestly fire away if there's more cases to cover, just wanted to get the ball rolling here.

CC @benjie

@JoviDeCroock JoviDeCroock requested a review from a team as a code owner November 7, 2024 19:53
Copy link

github-actions bot commented Nov 7, 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 changed the title Write about @oneOf Write about @oneOf in the graphql-js documentation Nov 7, 2024
benjie
benjie previously requested changes Nov 8, 2024
Copy link
Member

@benjie benjie left a comment

Choose a reason for hiding this comment

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

Really great to add some documentation about @oneOf!

@@ -0,0 +1,72 @@
---
title: Input Unions
Copy link
Member

Choose a reason for hiding this comment

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

@oneOf is not equivalent to "input union". You can use it to form an input union, but its use is broader than that. I would title it "OneOf input objects" or similar.

(Example: input UserSpecifier @oneOf { databaseId: Int, username: String, hospitalNumber: Int } - note that both databaseId and hospitalNumber are Int and can uniquely identify a user in different ways - so this isn't simply a union.)

Copy link
Member Author

Choose a reason for hiding this comment

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

My hesitancy was mainly that it's less of a clickthrough as the name is opaque to people while a general known word helps. I do agree that they are not the same

Copy link
Member

Choose a reason for hiding this comment

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

I agree; perhaps the URL would be oneof and the title would be "OneOf and input unions" or similar?

website/pages/input-unions.mdx Outdated Show resolved Hide resolved
website/pages/input-unions.mdx Outdated Show resolved Hide resolved
website/pages/input-unions.mdx Outdated Show resolved Hide resolved
website/pages/input-unions.mdx Outdated Show resolved Hide resolved
@benjie
Copy link
Member

benjie commented Nov 8, 2024

The SDL changes need reflection in the code version I think?

Co-authored-by: Benjie <benjie@jemjie.com>
@JoviDeCroock JoviDeCroock merged commit 273fc84 into 16.x.x Nov 8, 2024
31 checks passed
@JoviDeCroock JoviDeCroock deleted the input-unions branch November 8, 2024 15:33
yaacovCR pushed a commit to yaacovCR/graphql-js that referenced this pull request Nov 26, 2024
Currently input-unions and by extension the `@oneOf` directive aren't
present in the documentation. I have opted to put this into the advanced
section. The copy might be up for improvement, honestly fire away if
there's more cases to cover, just wanted to get the ball rolling here.

CC @benjie

---------

Co-authored-by: Benjie <benjie@jemjie.com>
yaacovCR pushed a commit to yaacovCR/graphql-js that referenced this pull request Dec 3, 2024
Currently input-unions and by extension the `@oneOf` directive aren't
present in the documentation. I have opted to put this into the advanced
section. The copy might be up for improvement, honestly fire away if
there's more cases to cover, just wanted to get the ball rolling here.

CC @benjie

---------

Co-authored-by: Benjie <benjie@jemjie.com>
yaacovCR pushed a commit to yaacovCR/graphql-js that referenced this pull request Dec 3, 2024
Currently input-unions and by extension the `@oneOf` directive aren't
present in the documentation. I have opted to put this into the advanced
section. The copy might be up for improvement, honestly fire away if
there's more cases to cover, just wanted to get the ball rolling here.

CC @benjie

---------

Co-authored-by: Benjie <benjie@jemjie.com>
yaacovCR pushed a commit to yaacovCR/graphql-js that referenced this pull request Dec 3, 2024
Currently input-unions and by extension the `@oneOf` directive aren't
present in the documentation. I have opted to put this into the advanced
section. The copy might be up for improvement, honestly fire away if
there's more cases to cover, just wanted to get the ball rolling here.

CC @benjie

---------

Co-authored-by: Benjie <benjie@jemjie.com>
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