let TVariable generics extend OperationVariables

kamilkisiela / apollo-angular

A fully-featured, production ready caching GraphQL client for Angular and every GraphQL server 🎁

https://apollo-angular.com

MIT License

1.5k stars 309 forks source link

let TVariable generics extend OperationVariables #1910

Closed phryneas closed 1 year ago

phryneas commented 1 year ago

to accommodate an upstream type change in @apollo/client@3.7.6

This addresses https://github.com/kamilkisiela/apollo-angular/issues/1907

Checklist:

[ ] If this PR is a new feature, please reference a discussion where a consensus about the design was reached (not necessary for small changes)
[ ] Make sure all the significant new logic is covered by tests
[x] Try to include the Pull Request inside of CHANGELOG.md

changeset-bot[bot] commented 1 year ago

🦋 Changeset detected

Latest commit: ad5340fbd82d781bf477a3f5dae41a3f89efd04a

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

| Name | Type | | -------------- | ----- | | apollo-angular | Patch |

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

phryneas commented 1 year ago

Oh, sorry! I had only run tsc in the package folder and assumed that would be enough to catch everything.

I already finished up for the day - I'll get back to this and fix up the rest tomorrow.

phryneas commented 1 year ago

Okay, next turn in the CI :)

Generally, these are hard to catch, since they don't turn up in the repo itself (which has strict: false), but only if used in other projects (as the CI does) with a different tsconfig and skipLibChecks: false. Let's hope I did get all usages now.

I didn't turn on strict: true for the repo since that results in a big batch of unrelated strictNullCheck-caused errors, but personally I would really want to encourage you to make that change in the future, since this might reoccur - consuming projects with a more strict tsconfig and skipLibChecks: false might run into issues with this. (Also, strict: true is how the TS team intends TS to be used)

PowerKiKi commented 1 year ago

100% agree we must switch to strict typescript as soon as possible. I was not aware we were not strict yet.

phryneas commented 1 year ago

Looks like it's al green now :) Yay!

PowerKiKi commented 1 year ago

Thank you taking the time to fix this :heart: