fix(typescript): add correct type safety to zone#create#307
fix(typescript): add correct type safety to zone#create#307bcoe merged 2 commits intogoogleapis:masterfrom callmehiphop:create-zone-request
Conversation
googlebot
added
the
cla: yes
| (err: Error | null): void; | ||
| } | ||
|
|
||
| type Without<T, K> = { |
There was a problem hiding this comment.
This made my brain melt. Can you add a quick comment letting the unsuspecting javascript developer know what's happening here?
| ): void | Promise<CreateZoneResponse> { | ||
| // tslint:disable-next-line no-any | ||
| const args = [config, callback!] as any; | ||
| ServiceObject.prototype.create.apply(this, args); |
There was a problem hiding this comment.
Any reason why this couldn't be:
return super.create(config, callback);There was a problem hiding this comment.
My approach here was to make TypeScript think there wasn't a create method in the prototype chain so that we could override it with the correct signature. As a result we can't call super here because it thinks that method doesn't exist.
Codecov Report
@@ Coverage Diff @@
## master #307 +/- ##
==========================================
- Coverage 92.65% 91.93% -0.72%
==========================================
Files 4 4
Lines 245 248 +3
Branches 47 47
==========================================
+ Hits 227 228 +1
- Misses 9 11 +2
Partials 9 9
Continue to review full report at Codecov.
|
|
Nicely done @callmehiphop! This is both a.) elegant and b.) gross. |
bcoe
left a comment
bcoe
left a comment
There was a problem hiding this comment.
this is looking good to me, a few clarifying questions 😄
bcoe
added
the
kokoro:force-run
kokoro-team
removed
the
kokoro:force-run
5 participants
So this isn't exactly the right way to fix this issue. A better solution would be to use generics in the common library, however that might be tricky/result in a sweeping breaking change. I'd like to offer this up as a temporary fix until we feel comfortable giving the types in the common lib some love.
Fixes #126 🦕