Skip to content

Unify InputValueConfig in schema definition #3067

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

Draft
wants to merge 4 commits into
base: next
Choose a base branch
from
Draft

Conversation

leebyron
Copy link
Contributor

@leebyron leebyron commented May 7, 2021

Depends on #3059

Defines a central GraphQLInputValueConfig and GraphQLInputValue as well as single definitions for converting between them, unifying this common functionality between input values and arguments.

This is a pre-req for #3049, as it means we now have a single authority for how defaultValue is defined, as that is a property of InputValue

@leebyron leebyron requested a review from IvanGoncharov May 7, 2021 19:52
leebyron added 4 commits May 12, 2021 09:38
Defines a central `GraphQLInputValueConfig` and `GraphQLInputValue` as well as single definitions for converting between them, unifying this common functionality between input values and arguments.

This is a pre-req for #3049
@leebyron leebyron force-pushed the input-value-config branch from 1d87c02 to 9830fda Compare May 12, 2021 16:38
leebyron added a commit that referenced this pull request May 15, 2021
Depends on #3067

Removes `valueFromAST()` and adds `coerceInputLiteral()` as an additional export from `coerceInputValue`.

The implementation is almost exactly the same as `valueFromAST()` with a slightly more strict type signature and refactored tests to improve coverage (the file unit test has 100% coverage)

While this does not change any behavior, it could be breaking if you rely directly on the valueFromAST() method. Use `coerceInputLiteral()` as a direct replacement.
leebyron added a commit that referenced this pull request May 15, 2021
Depends on #3067

Removes `valueFromAST()` and adds `coerceInputLiteral()` as an additional export from `coerceInputValue`.

The implementation is almost exactly the same as `valueFromAST()` with a slightly more strict type signature and refactored tests to improve coverage (the file unit test has 100% coverage)

While this does not change any behavior, it could be breaking if you rely directly on the valueFromAST() method. Use `coerceInputLiteral()` as a direct replacement.
leebyron added a commit that referenced this pull request May 15, 2021
Depends on #3067

Removes `valueFromAST()` and adds `coerceInputLiteral()` as an additional export from `coerceInputValue`.

The implementation is almost exactly the same as `valueFromAST()` with a slightly more strict type signature and refactored tests to improve coverage (the file unit test has 100% coverage)

While this does not change any behavior, it could be breaking if you rely directly on the valueFromAST() method. Use `coerceInputLiteral()` as a direct replacement.
leebyron added a commit that referenced this pull request May 15, 2021
Depends on #3067

Removes `valueFromAST()` and adds `coerceInputLiteral()` as an additional export from `coerceInputValue`.

The implementation is almost exactly the same as `valueFromAST()` with a slightly more strict type signature and refactored tests to improve coverage (the file unit test has 100% coverage)

While this does not change any behavior, it could be breaking if you rely directly on the valueFromAST() method. Use `coerceInputLiteral()` as a direct replacement.
@leebyron leebyron marked this pull request as draft June 2, 2021 00:10
@leebyron
Copy link
Contributor Author

leebyron commented Jun 2, 2021

Taking this out of the stack for now. When working on rebasing, I found it not necessarily needed.

@yaacovCR
Copy link
Contributor

yaacovCR commented Oct 1, 2024

Noting that this PR stills adds a bit more sanity to the code base, and can still be considered separately.

When rebasing this PR on main, we will have to consider how/whether to generalize GraphQLInputValue to include GraphQLVariableSignature, which was added to support fragment spread argument/fragment definition variables.

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.

3 participants