-
Notifications
You must be signed in to change notification settings - Fork 56
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
The current type of `any` for an outgoing message is overly permissive and doesn’t help users understand the shape of the object they need to provide. So, we: 1. change the Message class to an interface, which represents a Message-shaped object; 2. make Message’s `id` and `timestamp` properties optional (since we’re going to also use this interface for outgoing messages, which don’t necessarily have these properties), and compensate for this loosening of the Message type by introducing an InboundMessage type to represent messages received from Ably; 3. update the signature publishing methods to accept a Message object. Note that we deviate from the feature spec in that, in the feature spec, the publishing methods accept a Message instance. There are a couple of reasons for this deviation: 1. Accepting a Message-shaped object instead of a Message instance is consistent with our usage of the library in all of our existing example code and our tests, and is, I think, how things tend to be done in JavaScript. 2. The types in the feature spec are wrong; as things stand there, a user needs to provide a Message to the publishing methods, but Message has non-optional `id` and `timestamp` properties even though a user is not expected to populate them. We haven’t yet figured out how to address this error in the feature spec (i.e. do we introduce an InboundMessage type like we have here, or do we add some comments and leave it for library authors to figure out?); I started a dicussion about it in [1], but we’ve decided that we’d like to proceed with this ably-js change (which, since it’s a breaking change, we want to get into v2) without waiting for it to be addressed in the feature spec. Resolves #1472. [1] ably/specification#156
- Loading branch information
1 parent
4e3733f
commit 19a8d47
Showing
3 changed files
with
60 additions
and
40 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters