Validate against conflicting options on the same type
- Dominant language
- Go
- Stars
- 1.3k
- Forks
- 143
- Avg merge
- 4h 50m
- Merged PRs (30d)
- 1
Description
Consider the following queries:
```graphql
# @genqlient(omitempty: true)
query Q1(input1: MyInput) { ... }
query Q2(input2: MyInput) { ... }
```
In this case, we generate a type `MyInput` which we use for both `input1` and `input2`. But there's actually a conflict here: `omitempty` is set to true on `input1` and false (by default) on `input2`. (This is because directives on operation cascade down to all child fields.) The same problem is even more likely to happen with different queries in a package, and also applies to options set via the forthcoming input-field options setting (see #124), or output type-names reused via `typename`.
Right now we don't check for that; we just use whichever one we see first. Instead, similar to how we validate that if you use `typename` on several fields, they all have the same selections, we should validate that in any case where we use the same type-name in two places, they have the same options. So in this case, we would error, and advise the user to either set `omitempty: true` on `input2`, or set a different typename for either `input1` or `input2`.
In principle we could do this by extending the existing naming-collision validation (`selectionsMatch`). But doing that in the obvious way will require some annoying plumbing as well as parsing directives twice (once in type-generation then again in validation). The best thing to do is probably to parse all the directives in a preprocessing step upfront rather than as we generate types, and somehow attach them to nodes (perhaps just via a global map of node to genqlient directive). Then we can just read from that map in both type-generation and validation. (It would probably simplify the conversion code, too, since we could skip plumbing the options everywhere inline.)
Note that this will be a breaking change, but it will be going from silent confusing behavior to an explicit error, so I think that's totally fine and worthwhile.
Contributor guide
Research direction
Start with the existing naming-collision validation, especially selectionsMatch, and trace where directives are parsed during type generation. Determine how directives can be preprocessed and attached to nodes for both generation and validation. Done means reused type names with conflicting options produce an explicit error that suggests matching the options or choosing a different typename.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- tooling
- Issue type
- Feature
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100