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

Add an errors.Location field to all GraphQL type definitions #444

Closed
ssko1 opened this issue Mar 31, 2021 · 0 comments
Closed

Add an errors.Location field to all GraphQL type definitions #444

ssko1 opened this issue Mar 31, 2021 · 0 comments

Comments

@ssko1
Copy link

ssko1 commented Mar 31, 2021

Problem Statement

For some context on the problem statement, please refer to the following comment!

#437 (comment)

We wrote a linter to catch common errors made by contributors to our shared codebase. It checks for things like:

  • do mutations and their inputs have similar names?
  • does the type and its fields repeat words (eg avoid: FriendRequests { friendRequestID })
  • is pagination implemented correctly
    and so on.

One of the goals of our linter is to show meaningful error messages to our contributors whenever a GraphQL schema change produces errors. An important component of a good error message is the source location of the problem. The source location allows contributors to quickly identify issues in their schema so they can correct it. Additionally, IDEs can parse the source location to provide quick links to the affected code.

Currently, GraphQL type definitions (Object for example) have no fields that provide the location of the type in the source schema.

Recommended Approach

All GraphQL type definitions should provide a Loc errors.Location field.

Examples:

type DirectiveDecl struct {
	Name string
	Desc string
	Locs []string
	Args common.InputValueList
+       Loc errors.Location
}
type Object struct {
	Name       string
	Interfaces []*Interface
	Fields     FieldList
	Desc       string
	Directives common.DirectiveList

	interfaceNames []string
+       Loc errors.Location
}

Other potential solutions

replace Name: string with Name: Ident

Ident is primarily used in query fields, e.g. Field and Fragment Spreads.

We could extend this functionality and replace type names with Ident instead.

However, this change introduces a significant amount of stuttering, particularly where the string variant of name is needed (field.Name.Name, directive.Name.Name). This is especially true in the construction of maps: map[string]SomeType.

Alternative approaches

I currently see no alternatives to providing location other than rolling out a completely separate parser that provides the location for GraphQL type definitions.

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

No branches or pull requests

2 participants