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

Preserve defaultValue literals #3810

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

yaacovCR
Copy link
Contributor

@yaacovCR yaacovCR commented Dec 31, 2022

#3074 rebased on main.

Depends on #3809

@leebyron comments from original PR (edited, hopefully correctly):

Fixes #3051

This change solves the problem of default values defined via SDL not always resolving correctly through introspection by preserving the original GraphQL literal in the schema definition. This changes argument and input field definitions defaultValue field from just the "value" to a new GraphQLDefaultValueUsage type which contains either (EDIT: but not both!) "value" and "literal" fields.

Here is the flow for how a default value defined in an SDL would be converted into a functional schema and back to an SDL:

Before this change:

(SDL) --parse-> (AST) --coerceInputLiteral--> (defaultValue config) --valueToAST--> (AST) --print --> (SDL)

coerceInputLiteral performs coercion which is a one-way function, and valueToAST is unsafe and set to be deprecated in #3049.

After this change:

(SDL) --parse-> (defaultValue literal config) --print --> (SDL)

@netlify
Copy link

netlify bot commented Dec 31, 2022

Deploy Preview for compassionate-pike-271cb3 ready!

Name Link
🔨 Latest commit fdce4b2
🔍 Latest deploy log https://app.netlify.com/sites/compassionate-pike-271cb3/deploys/66e98a739b38c800088888d2
😎 Deploy Preview https://deploy-preview-3810--compassionate-pike-271cb3.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@yaacovCR yaacovCR added the PR: bug fix 🐞 requires increase of "patch" version number label Dec 31, 2022
@github-actions
Copy link

Hi @yaacovCR, 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

@yaacovCR yaacovCR force-pushed the default-value-literals-rebased branch from f7d4e56 to 4883f29 Compare December 31, 2022 19:52
@yaacovCR yaacovCR force-pushed the default-value-literals-rebased branch from 4883f29 to 023cd9c Compare January 1, 2023 19:36
@yaacovCR yaacovCR requested a review from leebyron January 2, 2023 12:21
@yaacovCR yaacovCR force-pushed the default-value-literals-rebased branch 5 times, most recently from cafef7b to 7a927c5 Compare February 6, 2023 12:52
@yaacovCR yaacovCR force-pushed the default-value-literals-rebased branch 2 times, most recently from c3fc46d to 0376313 Compare May 31, 2023 11:36
@yaacovCR yaacovCR force-pushed the default-value-literals-rebased branch from 0376313 to 619c450 Compare March 20, 2024 11:01
@yaacovCR yaacovCR force-pushed the default-value-literals-rebased branch from 619c450 to fd87704 Compare September 14, 2024 20:54
@yaacovCR yaacovCR requested a review from a team as a code owner September 14, 2024 20:54
@yaacovCR yaacovCR force-pushed the default-value-literals-rebased branch 8 times, most recently from 7c8cc2c to 47081cb Compare September 15, 2024 11:42
@yaacovCR yaacovCR force-pushed the default-value-literals-rebased branch from 47081cb to f6feca9 Compare September 16, 2024 17:27
@yaacovCR yaacovCR force-pushed the default-value-literals-rebased branch 4 times, most recently from 0439857 to 52aa101 Compare September 17, 2024 12:20
Fixes graphql#3051

This change solves the problem of default values defined via SDL not always resolving correctly through introspection by preserving the original GraphQL literal in the schema definition. This changes argument and input field definitions `defaultValue` field from just the "value" to a new `GraphQLDefaultValueUsage` type which contains either -- but not both -- "value" and "literal" fields.

Here is the flow for how a default value defined in an SDL would be converted into a functional schema and back to an SDL:

**Before this change:**

```
(SDL) --parse-> (AST) --coerceInputLiteral--> (defaultValue config) --valueToAST--> (AST) --print --> (SDL)
```

`coerceInputLiteral` performs coercion which is a one-way function, and `valueToAST` is unsafe and set to be deprecated in graphql#3049.

**After this change:**

```
(SDL) --parse-> (defaultValue literal config) --print --> (SDL)
```
@yaacovCR yaacovCR force-pushed the default-value-literals-rebased branch from 52aa101 to fdce4b2 Compare September 17, 2024 13:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: bug fix 🐞 requires increase of "patch" version number
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Default value declared via SDL may not introspect correctly
3 participants