Use correct return type for RTCSession encryption keys #4423
Conversation
This changes the return type of some public functions
hughns
changed the title
As per matrix-org/matrix-js-sdk#4423 the key can be undefined and so we should handle this rather than waiting for SubtleCrypto.importKey() to fail.
| @@ -483,6 +486,7 @@ export class MatrixRTCSession extends TypedEventEmitter<MatrixRTCSessionEvent, M | |||
| } | |||
| } | |||
|
|
|||
| // n.b. this will extend the array if necessary and fill in the gaps with undefined | |||
There was a problem hiding this comment.
It will actually make it a sparse array with empty slots that are skipped by iteration methods.
Which gets me thinking, maybe callers can hook into that...
There was a problem hiding this comment.
maybe callers can hook into that...
That is indeed the case; see element-hq/element-call#2646 (comment) which makes the updated typehints of this PR unnecessary (though the comments & updated unit test remain useful).
There was a problem hiding this comment.
Well, TypeScript doesn't seem to model sparse arrays at all. So, as far as I can see, there is no compile time hint that a sparse array can contain empty values.
I don't find this very satisfactory. /me ponders
There was a problem hiding this comment.
There is --noUncheckedIndexedAccess, but that would require updating many other parts of the codebase to comply with it.
|
I've made #4427 as an alternative which cleans up the API entirely. I prefer that as an approach to this. |
* Handle case of encryption key for an index to be undefined As per matrix-org/matrix-js-sdk#4423 the key can be undefined and so we should handle this rather than waiting for SubtleCrypto.importKey() to fail. * Use release version of matrix-js-sdk Diff is matrix-org/matrix-js-sdk@baa6d13...v34.7.0 * Use RTCSession. reemitEncryptionKeys() * Add some test coverage whilst we are here * Add some test coverage whilst we are here * Lint
The functions have always possibly returned
undefinedwhere the key at a particular index was not known. This changes the return type to match this reality.Checklist
public/exportedsymbols have accurate TSDoc documentation.