Make the function signature overloads of SubtleCrypto#exportKey more flexible
#1593
Conversation
…e flexible # Problem This test case _fails_ with the current state of the `exportKey` overloads ([playground](https://www.typescriptlang.org/play?#code/NocgVg7g1iA0AEIBOBDCdEAcoGMDOAHBiHtgJYgC6AdAGYD2SAoijgBYAUDSAtigC7wAvAD54AbwBQ8GfBxIAnpn71qeAK4AjfgBsAptT0APTI34BpPQq6M+-BOIC+8FHngBhRcvqWFASgBuaVkAehDZCMiAPQB+SUdAoA)). ```ts ['jwk', 'raw', 'pkcs8', 'spki'].forEach(format => { crypto.subtle.exportKey( format, // ERROR {} as CryptoKey, ); }); ``` # Change This PR makes it so that the `"jwk"` overload works as expected by producing `Promise<JsonWebKey>` as the return type, and the remaining keys in the `KeyFormat` union produce `Promise<ArrayBuffer>` as the return type, _and_ the example code above typechecks correctly.
|
Thanks for the PR! This section of the codebase is owned by @saschanaz - if they write a comment saying "LGTM" then it will be merged. |
|
@microsoft-github-policy-service agree |
steveluscher
changed the title
SubtleCrypto#exportKey more flexibleSubtleCrypto#exportKey and SubtleCrypto#importKey more flexible
|
Mmm… hold on. I see why this doesn't work for |
steveluscher
changed the title
SubtleCrypto#exportKey and SubtleCrypto#importKey more flexibleSubtleCrypto#exportKeymore flexible
steveluscher
changed the title
SubtleCrypto#exportKeymore flexibleSubtleCrypto#exportKey more flexible
|
I'm pretty sure this is done to fix the |
|
Check out the second playground link; |
|
I mean: playground |
unittests/files/keyusage.ts
Outdated
| function assertType<T>(_x: T) {} | ||
|
|
||
| const mockKey = {} as CryptoKey; | ||
| assertType<Promise<JsonWebKey>>(crypto.subtle.exportKey('jwk', mockKey)); |
There was a problem hiding this comment.
Please also check what happens if the input is 'jwk' | 'pkcs8'.
There was a problem hiding this comment.
100%. I'm on it.
|
I think we are blocked by microsoft/TypeScript#14107 which is still open. |
|
There we go! Updated the playground link in the description. |
|
Ah, that works better 👍 |
unittests/files/keyusage.ts
Outdated
| function assertType<T>(_x: T) {} | ||
|
|
||
| const mockKey = {} as CryptoKey; | ||
| assertType<Promise<ArrayBuffer | JsonWebKey>>(crypto.subtle.exportKey('' as KeyFormat, mockKey)); |
There was a problem hiding this comment.
This is too loose as either Promise<ArrayBuffer> or Promise<JsonWebKey> will match the type. But not sure how to make it strict enough 🤔
There was a problem hiding this comment.
Maybe we can test whether it works for both as Promise<ArrayBuffer> and as Promise<JsonWebKey>, like:
crypto.subtle.exportKey('' as KeyFormat, mockKey) as Promise<ArrayBuffer>; // type includes ArrayBuffer
crypto.subtle.exportKey('' as KeyFormat, mockKey) as Promise<JsonWebKey>; // type also includes JsonWebKeyThere was a problem hiding this comment.
This is too loose as either Promise or Promise will match the type
Isn't that exactly the point when format is not known to yield one or the other?
How about this?
assertType<Promise<ArrayBuffer | JsonWebKey>>(
crypto.subtle
.exportKey("" as KeyFormat, mockKey)
.then((ambiguousExportedKeyData) =>
ambiguousExportedKeyData instanceof ArrayBuffer
? (ambiguousExportedKeyData satisfies ArrayBuffer)
: (ambiguousExportedKeyData satisfies JsonWebKey)
)
);There was a problem hiding this comment.
Isn't that exactly the point when
formatis not known to yield one or the other?
Yes but that'll never fail when the return type regresses to either ArrayBuffer or JsonWebKey.
How about this?
Sounds good!
|
This is technically a breaking change but I think it's reasonable. LGTM |
|
Merging because @saschanaz is a code-owner of all the changes - thanks! |
Problem
This test case fails with the current state of the
exportKeyoverloads (playground).Change
This PR makes it so that the
"jwk"overload works as expected by producingPromise<JsonWebKey>as the return type, and the remaining keys in theKeyFormatunion producePromise<ArrayBuffer>as the return type, and the example code above typechecks correctly (playground).