Skip to content

Default value declared via SDL may not introspect correctly #3051

Closed
@leebyron

Description

@leebyron

One interesting aspect here is that a default value declared via SDL is coerced twice:
Once via valueFromAST and then again via coerceInputValueImpl. Is my reading here correct?

I am not really sure this is a problem, but I wanted to call that out.

Originally posted by @andimarek in #3049 (comment)

That's a good call out. Coercing more than once should be safe, assuming that coerce(X) == coerce(serialize(coerce(X)))

However I think this is likely exposing a real (but maybe low-impact) bug, even before this diff. Default values are exposed via introspection and are not expected to include nested default values:

input A {
  a: B = {}
}

input B {
  c: Int = 0
}

When introspecting A.a, we should see "defaultValue": "{}" but if I read correctly likely will see "defaultValue": "{c:0}"

If you were to re-print this SDL after introspection, you'd have:

input A {
  a: B = { c: 0 }
}

which broke the original intent.

Executed results would be correct, however schema representation would be off.

Notably, constructing the schema programmatically would not encounter this bug - only using AST creation would because of how valueFromAST works.

Activity

andimarek

andimarek commented on Apr 26, 2021

@andimarek
Contributor

I think that ties back to the missing functionality of printing an Ast value: #1817

andimarek

andimarek commented on Apr 26, 2021

@andimarek
Contributor

@leebyron I think in that case specifically we are looking at parseValue(parseLiteral(x)) == parseLiteral(x) as the first coercion is parsing the Literal and then it gets coerced again via parseValue (treated like a variable value lets say).

andimarek

andimarek commented on May 9, 2021

@andimarek
Contributor

Looking at GraphQL Java we have the same challenge around applied Directives in a Schema.
e.g.:

type Query {
  foo: String @bar(json:  {a: "a", b = "b"})
}

cc @IvanGoncharov How does GraphQL.js handle that? I didn't see any specific code to add an applied directive to a schema element. Is this possible programmatically without defining a SDL?

added a commit that references this issue on May 10, 2021
29971d0
added 9 commits that reference this issue on May 10, 2021
4b4cb69
3ab211e
222caaf
0a91e63
2ee1da8
330f01a
05db9bd
6c79c53
297f2c3

43 remaining items

Loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      Participants

      @leebyron@andimarek@yaacovCR

      Issue actions

        Default value declared via SDL may not introspect correctly · Issue #3051 · graphql/graphql-js