Limit ParentType to only contain fields from the @key and @requires directives

[jakeblaxon]

This PR seeks to address #4089. It modifies the ParentType for resolvers of federated types to only contain fields from the @key and @requires directives, since these fields are the only ones guaranteed to be available by federation.

This is my first PR with this project, so it's a little rough. My goal was to lay a foundation to solve this issue, but certain edge cases may not be fully addressed (such as unwrapped types and type mappings) and may break backwards compatibility. I furthermore only focused on adding this feature to Typescript, so it remains unimplemented in Flow for now. I think it would be best if we make this an experimental feature, but I'm unsure of the best way to do so. Please feel free to make any recommendations/modifications you see necessary. Thanks!

[jakeblaxon]

I don't intend for this PR to be merged yet, but I do want to open it up for discussion. Since this has a high risk of breaking backward compatibility, I think it'd be best to offer it as an experimental feature for now. What would be the best way to do this? Should I add it as a config option or maybe move this feature to its own plugin altogether?

[kamilkisiela]

One type is too complex to read so we either need to add comments or break it into multiple types.

[dotansimha]

This sounds like a very nice solution for that. @jakeblaxon can you please fix @kamilkisiela 's suggestions? And do you think we need more test cases for this change? maybe add tests with GraphQL interface and type to make sure it's compatible with that?

[dotansimha]

@jakeblaxon looks like a great progress :) What are the remaining tasks for this PR?

@kamilkisiela can you please take a look again?

[jakeblaxon]

I apologize for the delay on this. I've updated the types to pascal case. The last few remaining tasks I want to add are tests for interface and union types (and any other cases that you suggest) and documentation for how to use the GraphQLRecursivePick type (in case you want to use it as a param or return type in a custom method). The only other thing I would want to consider is adding a flag to enable/disable this specific feature as experimental. I'm fine with not including that, though, if you don't think it's necessary. @dotansimha what are your thoughts?

[dotansimha]

Sounds good, thanks @jakeblaxon !

Not sure about the need for a feature flag, because this is actually a bug fix and anyway it's better than what we have now. @kamilkisiela what do you think?

[jakeblaxon]

@dotansimha I think this PR is in a good state so I've opened it up. I added an interface test, but I'm skipping it for now since this PR doesn't support them. However, I think it's better to just get this in now since it works for types and is non-breaking for interfaces. I ended up not adding a test for unions since they don't really make sense in this context. I also didn't add documentation for the GraphQLRecursivePick type, but I think it's easy enough to figure out by looking at the generated types that use it. I wasn't sure where the best place was to put it. If you think there's a good spot where I can add it though, please let me know and I can get that in too!

[jakeblaxon]

@dotansimha is there anything preventing us from merging this PR? I noticed this has been open for a while.

[eurostar-fennec-cooper]

I could really use this fix, too. Would love to see this merged 👍

[dotansimha]

@jakeblaxon No reason. You got @kamilkisiela 's approval so we are good to go :)
Thank you so much!

[theguild-bot]

The latest changes of this PR are available as alpha in npm: 1.17.4-alpha-48e09323.0

Quickly update your package.json by running:

npx match-version @graphql-codegen 1.17.4-alpha-48e09323.0

[dotansimha]

@jakeblaxon @eurostar-fennec-cooper can you please try the latest alpha? 1.17.4-alpha-48e09323.0

[eurostar-fennec-cooper]

@dotansimha 1.17.4-alpha-48e09323.0 works perfectly/resolves the issue for me.

[dotansimha]

Great! Thank you for the feedback, @eurostar-fennec-cooper !
We will have a stable release soon :)