-
-
Notifications
You must be signed in to change notification settings - Fork 313
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
Fixed type definition for subscribe #1290
Fixed type definition for subscribe #1290
Conversation
1f65e95
to
3520a64
Compare
@kamilkisiela any idea why the above 2 checks aren't finishing after days? |
@intellix could you rebase? |
3520a64
to
017d6d8
Compare
Rebased @kamilkisiela |
Thank you @intellix !! 🚀 🥇 |
I think this is a breaking change that should have been release as a major version. It would typically break the following code: export interface Login {
login: boolean;
}
this.apollo.mutate<Login>({
mutation: loginMutation,
variables: loginData,
}).pipe(map(({data: {login}}) => login)); With an error similar to:
This is because ultimately I am not sure why data is nullable though, I believe in the context of apollo-angular (not graphql), a mutation either succeeds and its data is exactly as defined in the mutation itself (probably often non-nullable), or the mutation fails and it goes through the observable error mechanism. But we should never force a mutation result to be nullable... or should we ? |
@intellix @kamilkisiela do you have any thoughts on my comment ? As is it is pretty much impossible to use |
Except apollo family because of kamilkisiela/apollo-angular#1290 (comment)
I digged deeper in this issue and finally figured out the issue is in |
Checklist:
According to apollo-client it returns a FetchResult whereas apollo-angular says it's an any: https://github.com/apollographql/apollo-client/blob/7eaf4132cd2cd6244260777799406aaa03fcf377/packages/apollo-client/src/ApolloClient.ts#L355