TS: remove TS-specific TData from ExecutionResult

[IvanGoncharov]

Reverts DefinitelyTyped/DefinitelyTyped#26763
Since we can't gurantee that response match types it was basically
glorified type cast.

[sgoll]

apollographql/apollo-link#1261 (comment)

@IvanGoncharov The generic parameter TData from ExecutionResult is (was) also used

• via FetchResult<TData> in apollo-client's MutationUpdaterFn<T>
• via MutationUpdaterFn<T> in @apollo/react's BaseMutationOptions<TData>
• via BaseMutationOptions<TData> in @apollo/react-hooks's MutationHookOptions<TData>
• via MutationHookOptions<TData> in code generated by dotansimha/graphql-code-generator

For our project, this effectively renders type-checking of the result from mutation hooks defunct since data now always is { [key: string]: any } instead of the actual type derived via code generation from the underlying schema/typedefs.

I am not sure how to best work around this regression caused by graphql/graphql-js version 15 within the GraphQL ecosystem.

Is this something that needs to be worked around now by higher-order libraries such as apollographql/apollo-link or dotansimha/graphql-code-generator?

Where best should I file a bug report?

[tomdale]

This is a pretty significant breaking change that has cascading ecosystem effects. Is it possible to revert this change until a more thorough rationale is provided and more discussion can happen?

[erik-beus]

We use graphql-code-generator for codegen, which means that the "glorified type cast" was a pretty qualified guess for the response type in our case @IvanGoncharov
Any chance the generic could be reintroduced? This is stopping us from using v. 15

[IvanGoncharov]

We use graphql-code-generator for codegen, which means that the "glorified type cast" was a pretty qualified guess for the response type in our case

@erik-beus Totally agree that since graphql-code-generator can see you query in build time and thus can guarantee type-safety.
graphql-js on the other hand doesn't have access to your query in compile time so it can't provide guarantees about data value.
So type cast should be done in graphql-code-generator.

Any chance the generic could be reintroduced? This is stopping us from using v. 15

We adopted TS typings from DefinetlyTyped and referred from breaking changes until v15. This change is clearly marked as breaking so it's normal that some TS libraries need some time to adapt to this change.

At the same time, if you can explain why graphql-code-generator or any other TS library can't adapt to this change I'm totally open to discussion.

[davewasmer]

@IvanGoncharov what about accepted a generic arg but defaulting to the current type? I.e. type ExecutionResult<TData = { [key: string]: any }> = ...

I see two possibilities among consumers of this library:

1. They plan to cast the types at some point (whether through build time tooling or by hand or something else). A generic arg is helpful here to avoid having to create wrapper types that do the cast in every project.

2. They don't plan to cast the types, in which case the default arg puts them in today's situation.

[IvanGoncharov]

@davewasmer Ok it's actually reasonable to add a template argument on ExecutionResult since graphql-js wouldn't use it but instead provide it as a convenience for other libraries.
But I'm 100% percent against adding template argument to execute or graphql since we can't guarantee that their result will match return type.
Please create PR for that.

[IvanGoncharov]

@davewasmer Released in https://github.com/graphql/graphql-js/releases/tag/v15.1.0
Can you please provide feedback if it what you wanted or not?

[davewasmer]

Yep, I think that does it! Thanks @IvanGoncharov

[GreenGremlin]

With this change, it's possible to specify a data type for ExecutionResult, but not for the execute function. Here's a PR to update execute and executeSync to take an optional generic to override the data type.

https://github.com/graphql/graphql-js/pull/2862/files