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

Bind directly to AST types, instead of copying out random bits #490

Merged
merged 6 commits into from
Jan 9, 2019

Conversation

vektah
Copy link
Collaborator

@vektah vektah commented Jan 8, 2019

😅 This took a bit of work.

The core of this change is to go from

// TypeReference represents the type of a field or arg, referencing an underlying 
// TypeDefinition (type, input, scalar)
type TypeReference struct {
	*TypeDefinition

	Modifiers   []string
	ASTType     *ast.Type
	AliasedType *TypeImplementation
}
// TypeDefinition is the static reference to a graphql type. It can be referenced by 
// many TypeReferences, and has one or more backing implementations in go.
type TypeDefinition struct {
	TypeImplementation
	IsScalar    bool
	IsInterface bool
	IsInput     bool
	GQLType     string              // Name of the graphql type
	Marshaler   *TypeImplementation // If this type has an external marshaler this will be set
}

// TypeImplementation is a reference to exisiting golang code that either meets
// the graphql.Marshaler interfaceor points to the root of a pair of external 
// Marshal[TYPE] and Unmarshal[TYPE] functions.
type TypeImplementation struct {
	GoType        string // Name of the go type
	Package       string // the package the go type lives in
	IsUserDefined bool   // does the type exist in the typemap
}

to

// TypeReference represents the type of a field or arg, referencing an underlying 
// TypeDefinition (type, input, scalar)
type TypeReference struct {
	Definition *TypeDefinition

	GoType  types.Type
	ASTType *ast.Type
}
// TypeDefinition is the static reference to a graphql type. It can be referenced by 
// many TypeReferences, and has one or more backing implementations in go.
type TypeDefinition struct {
	GQLDefinition *ast.Definition
	GoType        types.Type  // The backing go type, may be nil until after model generation
	Marshaler     *types.Func // When using external marshalling functions this will point to the Marshal function
	Unmarshaler   *types.Func // When using external marshalling functions this will point to the Unmarshal function
}

This refactor is preparing for a few upcoming changes:

  • Binding to multiple go types for a single GraphQL type (eg Int can be int64, int, int32 or ID can be string or int)
  • Making all structs pointers by default - this was getting really messy because the graphql nonnullability was being conflated with go pointers.

This PR breaks a fairly subtle old behaviour, automatically treating wrapped scalar types as their primitives by automatically casting. This will be replaced by being able to define multiple custom scalars for the primitives, and I will make sure that lands before we tag a release.

Aside from that, pretty confident in this change. Only generated code changes were minor whitespace issues.

@vektah vektah merged commit 82b1917 into next Jan 9, 2019
@vektah vektah deleted the bind-directly-to-types branch January 9, 2019 07:51
}

// todo @vektah: This should probably go away, its too easy to conflate gql required vs go pointer
Copy link
Contributor

Choose a reason for hiding this comment

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

This goes away with the implementation of #375?

cgxxv pushed a commit to cgxxv/gqlgen that referenced this pull request Mar 25, 2022
Bind directly to AST types, instead of copying out random bits
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.

2 participants