Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Define the public API of the SDK #7

Closed
lawrence-forooghian opened this issue Aug 14, 2024 · 0 comments · Fixed by #9
Closed

Define the public API of the SDK #7

lawrence-forooghian opened this issue Aug 14, 2024 · 0 comments · Fixed by #9
Assignees

Comments

@lawrence-forooghian
Copy link
Collaborator

lawrence-forooghian commented Aug 14, 2024

Created this issue to replace internal Jira issue ECO-4900 so that I can refer to it in subsequent issues. Description of that issue:

Agree and document a public interface for Chat (Swift) which is consistent with Chat (Kotlin). Account for any platform differences decided in the architecture task.

┆Issue is synchronized with this Jira Task by Unito

@lawrence-forooghian lawrence-forooghian self-assigned this Aug 14, 2024
lawrence-forooghian added a commit that referenced this issue Aug 15, 2024
Based on JS repo at 0b4b0f8.

There are some things that I’m unsure about regarding the usability and
implementability of this API. We’ll be able to validate the usability
when we do #4, which will create a mock implementation of this API and
then build the example app around the mock. Implementability we’ll
discover as we try to build the SDK.

TypeScript-to-Swift decisions:

- I’ve named the entry point class DefaultChatClient instead of their
  ChatClient; this is so that I can have a public protocol named
  ChatClient.

- My `throws` annotations are based on the JS docstrings (or in a couple
  of cases my assumptions). The Rooms accessors that throw in JS if a
  feature is not enabled instead call fatalError in Swift, which is the
  idiomatic thing to do for programmer errors [1].

- Skipped copying the docstrings from JS; created #1 to do this later.
  Turned off missing_docs for now.

- The listener pattern in JS is instead implemented by returning an
  AsyncSequence. I believe that we do not need an equivalent of the `off*`
  / `unsubscribe*` methods since iterating over an AsyncSequence is a pull
  rather than a push. And I believe (but am still quite shaky about the
  details, so may be wrong) that there are AsyncSequence lifecycle events
  (e.g. end of iteration, task cancellation) that we can use to manage the
  underlying ably-cocoa listeners.

Swift decisions and thoughts:

My decision on what should be a protocol and what should be a concrete
type was fairly arbitrary; I’ve made everything a protocol (for
mockability) except structs that are basically just containers for data
(but this line is blurry; for example, this might introduce issues for
somebody who wants to be able to mock Message’s isBefore(_:) method).

I’ve annotated all of the protocols that feel like they represent some
sort of client with AnyObject; don’t have a great explanation of why but
intuitively it felt right (seemed like they should be reference types).

Having not yet used Swift concurrency much, I didn’t have a good
intuition for what things should be Sendable, so I’ve put it on pretty
much everything. Similarly, I don’t have a good sense of what should be
annotated as `async` (for some of this our hand will probably also end
up being forced by the implementation). I also am not sure whether the
`current` / `error` properties for connection and room status make sense
in a world where most things are async (especially if the intention is
that, for example, the user check `current` before deciding whether to
call a certain method, and this method will throw an error if they get
it wrong, but the state of the world might have changed since they
checked it and that’s not their fault), but I’ve kept them for now.

Chose to use existential types when one protocol returns another (i.e.
`-> any MyProtocol`) instead of associated types, because it’s more
readable (you can’t use opaque types in a protocol declaration) and I
don’t think that we need to worry about the performance implications.

turned on explicit_acl, but just for the library — it's a handy thing to
have to make sure you've not missed making anything public, but we don’t
need it for BuildTool

Some stuff is up in the air until we start trying to implement, too.

Resolves #7.

[1] https://www.douggregor.net/posts/swift-for-cxx-practitioners-error-handling/
lawrence-forooghian added a commit that referenced this issue Aug 15, 2024
Based on JS repo at 0b4b0f8. I have not performed any review or critique
of the JS API, and simply copied it and mildly adapted to Swift.

There are some things that I’m unsure about regarding the usability and
implementability of this Swift API. We’ll be able to validate the
usability when we do #4, which will create a mock implementation of this
API and then build the example app around the mock. Implementability
we’ll discover as we try to build the SDK.

This is just a first attempt and all of the decisions can be revisited.

TypeScript-to-Swift decisions:

- I’ve named the entry point class DefaultChatClient instead of their
  ChatClient; this is so that I can have a public protocol named
  ChatClient.

- My `throws` annotations are based on the JS docstrings (or in a couple
  of cases my assumptions). The Rooms accessors that throw in JS if a
  feature is not enabled instead call fatalError in Swift, which is the
  idiomatic thing to do for programmer errors [1].

- Skipped copying the docstrings from JS to avoid churn; created #1 to do
  this later. Turned off missing_docs for now.

- The listener pattern in JS is instead implemented by returning an
  AsyncSequence. I believe that we do not need an equivalent of the `off*`
  / `unsubscribe*` methods since iterating over an AsyncSequence is a pull
  rather than a push. And I believe (but am still quite shaky about the
  details, so may be wrong) that there are AsyncSequence lifecycle events
  (e.g. end of iteration, task cancellation) that we can use to manage the
  underlying ably-cocoa listeners.

- RoomOptionsDefaults in JS is instead implemented here by giving the
  *Options types a no-args initializer that populates the default values.

- I’ve copied the logging interface more or less from JS (but with
  LogHandler a protocol so that we can have argument labels). Will think
  about something more idiomatic in #8.

Swift decisions and thoughts:

- My decision on what should be a protocol and what should be a concrete
  type was fairly arbitrary; I’ve made everything a protocol (for
  mockability) except structs that are basically just containers for data
  (but this line is blurry; for example, this might introduce issues for
  somebody who wants to be able to mock Message’s isBefore(_:) method).
  One downside of using protocols is that you can’t nest types inside them
  (this would be nice for e.g. related enums) but it’s alright.

- I’ve annotated all of the protocols that feel like they represent some
  sort of client with AnyObject; I don’t have a great explanation of why
  but intuitively it felt right (seemed like they should be reference
  types).

- Having not yet used Swift concurrency much, I didn’t have a good
  intuition for what things should be Sendable, so I’ve put it on pretty
  much everything. Similarly, I don’t have a good sense of what should be
  annotated as `async` (for some of this our hand will probably also end
  up being forced by the implementation). I also am not sure whether the
  `current` / `error` properties for connection and room status make sense
  in a world where most things are async (especially if the intention is
  that, for example, the user check `current` before deciding whether to
  call a certain method, and this method will throw an error if they get
  it wrong, but the state of the world might have changed since they
  checked it and that’s not their fault), but I’ve kept them for now.

- Chose to use existential types when one protocol returns another (i.e.
  `-> any MyProtocol`) instead of associated types, because it’s more
  readable (you can’t use opaque types in a protocol declaration) and I
  don’t think that we need to worry about the performance implications.

- I’ve deferred adding helpful protocol conformances (Equatable, Hashable,
  Codable etc) to #10.

- I’ve turned on the explicit_acl SwiftLint rule, which forces us to add
  an access control modifier to all declarations. This makes it less
  likely that we forget to make something `public` instead of the default
  `internal`.

Resolves #7.

[1] https://www.douggregor.net/posts/swift-for-cxx-practitioners-error-handling/
lawrence-forooghian added a commit that referenced this issue Aug 15, 2024
Based on JS repo at 0b4b0f8. I have not performed any review or critique
of the JS API, and simply copied it and mildly adapted to Swift.

There are some things that I’m unsure about regarding the usability and
implementability of this Swift API. We’ll be able to validate the
usability when we do #4, which will create a mock implementation of this
API and then build the example app around the mock. Implementability
we’ll discover as we try to build the SDK.

This is just a first attempt and all of the decisions can be revisited.

TypeScript-to-Swift decisions:

- I’ve named the entry point class DefaultChatClient instead of their
  ChatClient; this is so that I can have a public protocol named
  ChatClient.

- My `throws` annotations are based on the JS docstrings (or in a couple
  of cases my assumptions). The Rooms accessors that throw in JS if a
  feature is not enabled instead call fatalError in Swift, which is the
  idiomatic thing to do for programmer errors [1].

- Skipped copying the docstrings from JS to avoid churn; created #1 to do
  this later. Turned off missing_docs for now.

- The listener pattern in JS is instead implemented by returning an
  AsyncSequence. I believe that we do not need an equivalent of the `off*`
  / `unsubscribe*` methods since iterating over an AsyncSequence is a pull
  rather than a push. And I believe (but am still quite shaky about the
  details, so may be wrong) that there are AsyncSequence lifecycle events
  (e.g. end of iteration, task cancellation) that we can use to manage the
  underlying ably-cocoa listeners.

- RoomOptionsDefaults in JS is instead implemented here by giving the
  *Options types a no-args initializer that populates the default values.

- I’ve copied the logging interface more or less from JS (but with
  LogHandler a protocol so that we can have argument labels). Will think
  about something more idiomatic in #8.

Swift decisions and thoughts:

- My decision on what should be a protocol and what should be a concrete
  type was fairly arbitrary; I’ve made everything a protocol (for
  mockability) except structs that are basically just containers for data
  (but this line is blurry; for example, this might introduce issues for
  somebody who wants to be able to mock Message’s isBefore(_:) method).
  One downside of using protocols is that you can’t nest types inside them
  (this would be nice for e.g. related enums) but it’s alright.

- I’ve annotated all of the protocols that feel like they represent some
  sort of client with AnyObject; I don’t have a great explanation of why
  but intuitively it felt right (seemed like they should be reference
  types).

- Having not yet used Swift concurrency much, I didn’t have a good
  intuition for what things should be Sendable, so I’ve put it on pretty
  much everything. Similarly, I don’t have a good sense of what should be
  annotated as `async` (for some of this our hand will probably also end
  up being forced by the implementation). I also am not sure whether the
  `current` / `error` properties for connection and room status make sense
  in a world where most things are async (especially if the intention is
  that, for example, the user check `current` before deciding whether to
  call a certain method, and this method will throw an error if they get
  it wrong, but the state of the world might have changed since they
  checked it and that’s not their fault), but I’ve kept them for now.

- Chose to use existential types when one protocol returns another (i.e.
  `-> any MyProtocol`) instead of associated types, because it’s more
  readable (you can’t use opaque types in a protocol declaration) and I
  don’t think that we need to worry about the performance implications.

- I’ve deferred adding helpful protocol conformances (Equatable, Hashable,
  Codable etc) to #10.

- I’ve turned on the explicit_acl SwiftLint rule, which forces us to add
  an access control modifier to all declarations. This makes it less
  likely that we forget to make something `public` instead of the default
  `internal`.

Resolves #7.

[1] https://www.douggregor.net/posts/swift-for-cxx-practitioners-error-handling/
lawrence-forooghian added a commit that referenced this issue Aug 15, 2024
Based on JS repo at 0b4b0f8. I have not performed any review or critique
of the JS API, and simply copied it and mildly adapted to Swift.

There are some things that I’m unsure about regarding the usability and
implementability of this Swift API. We’ll be able to validate the
usability when we do #4, which will create a mock implementation of this
API and then build the example app around the mock. Implementability
we’ll discover as we try to build the SDK.

This is just a first attempt and all of the decisions can be revisited.

TypeScript-to-Swift decisions:

- I’ve named the entry point class DefaultChatClient instead of their
  ChatClient; this is so that I can have a public protocol named
  ChatClient.

- My `throws` annotations are based on the JS docstrings (or in a few
  cases my assumptions). The Rooms accessors that throw in JS if a feature
  is not enabled instead call fatalError in Swift, which is the idiomatic
  thing to do for programmer errors [1].

- Skipped copying the docstrings from JS to avoid churn; created #1 to do
  this later. Turned off missing_docs for now.

- The listener pattern in JS is instead implemented by returning an
  AsyncSequence. I believe that we do not need an equivalent of the `off*`
  / `unsubscribe*` methods since iterating over an AsyncSequence is a pull
  rather than a push. And I believe (but am still quite shaky about the
  details, so may be wrong) that there are AsyncSequence lifecycle events
  (e.g. end of iteration, task cancellation) that we can use to manage the
  underlying ably-cocoa listeners.

- RoomOptionsDefaults in JS is instead implemented here by giving the
  *Options types a no-args initializer that populates the default values.

- I’ve copied the logging interface more or less from JS (but with
  LogHandler a protocol so that we can have argument labels). Will think
  about something more idiomatic in #8.

Swift decisions and thoughts:

- My decision on what should be a protocol and what should be a concrete
  type was fairly arbitrary; I’ve made everything a protocol (for
  mockability) except structs that are basically just containers for data
  (but this line is blurry; for example, this might introduce issues for
  somebody who wants to be able to mock Message’s isBefore(_:) method).
  One downside of using protocols is that you can’t nest types inside them
  (this would be nice for e.g. related enums) but it’s alright.

- I’ve annotated all of the protocols that feel like they represent some
  sort of client with AnyObject; I don’t have a great explanation of why
  but intuitively it felt right (seemed like they should be reference
  types).

- Having not yet used Swift concurrency much, I didn’t have a good
  intuition for what things should be Sendable, so I’ve put it on pretty
  much everything. Similarly, I don’t have a good sense of what should be
  annotated as `async` (for some of this our hand will probably also end
  up being forced by the implementation). I also am not sure whether the
  `current` / `error` properties for connection and room status make sense
  in a world where most things are async (especially if the intention is
  that, for example, the user check `current` before deciding whether to
  call a certain method, and this method will throw an error if they get
  it wrong, but the state of the world might have changed since they
  checked it and that’s not their fault), but I’ve kept them for now.

- Chose to use existential types when one protocol returns another (i.e.
  `-> any MyProtocol`) instead of associated types, because it’s more
  readable (you can’t use opaque types in a protocol declaration) and I
  don’t think that we need to worry about the performance implications.

- I’ve deferred adding helpful protocol conformances (Equatable, Hashable,
  Codable etc) to #10.

- I’ve turned on the explicit_acl SwiftLint rule, which forces us to add
  an access control modifier to all declarations. This makes it less
  likely that we forget to make something `public` instead of the default
  `internal`.

Resolves #7.

[1] https://www.douggregor.net/posts/swift-for-cxx-practitioners-error-handling/
lawrence-forooghian added a commit that referenced this issue Aug 15, 2024
Based on JS repo at 0b4b0f8. I have not performed any review or critique
of the JS API, and simply copied it and mildly adapted to Swift.

There are some things that I’m unsure about regarding the usability and
implementability of this Swift API. We’ll be able to validate the
usability when we do #4, which will create a mock implementation of this
API and then build the example app around the mock. Implementability
we’ll discover as we try to build the SDK.

This is just a first attempt and all of the decisions can be revisited.

TypeScript-to-Swift decisions:

- I’ve named the entry point class DefaultChatClient instead of their
  ChatClient; this is so that I can have a public protocol named
  ChatClient.

- My `throws` annotations are based on the JS docstrings (or in a few
  cases my assumptions). The Rooms accessors that throw in JS if a feature
  is not enabled instead call fatalError in Swift, which is the idiomatic
  thing to do for programmer errors [1].

- Skipped copying the docstrings from JS to avoid churn; created #1 to do
  this later. Turned off missing_docs for now.

- The listener pattern in JS is instead implemented by returning an
  AsyncSequence. I believe that we do not need an equivalent of the `off*`
  / `unsubscribe*` methods since iterating over an AsyncSequence is a pull
  rather than a push. And I believe (but am still quite shaky about the
  details, so may be wrong) that there are AsyncSequence lifecycle events
  (e.g. end of iteration, task cancellation) that we can use to manage the
  underlying ably-cocoa listeners.

- RoomOptionsDefaults in JS is instead implemented here by giving the
  *Options types a no-args initializer that populates the default values.

- I’ve copied the logging interface more or less from JS (but with
  LogHandler a protocol so that we can have argument labels). Will think
  about something more idiomatic in #8.

Swift decisions and thoughts:

- My decision on what should be a protocol and what should be a concrete
  type was fairly arbitrary; I’ve made everything a protocol (for
  mockability) except structs that are basically just containers for data
  (but this line is blurry; for example, this might introduce issues for
  somebody who wants to be able to mock Message’s isBefore(_:) method).
  One downside of using protocols is that you can’t nest types inside them
  (this would be nice for e.g. related enums) but it’s alright.

- I’ve annotated all of the protocols that feel like they represent some
  sort of client with AnyObject; I don’t have a great explanation of why
  but intuitively it felt right (seemed like they should be reference
  types).

- Having not yet used Swift concurrency much, I didn’t have a good
  intuition for what things should be Sendable, so I’ve put it on pretty
  much everything. Similarly, I don’t have a good sense of what should be
  annotated as `async` (for some of this our hand will probably also end
  up being forced by the implementation). I also am not sure whether the
  `current` / `error` properties for connection and room status make sense
  in a world where most things are async (especially if the intention is
  that, for example, the user check `current` before deciding whether to
  call a certain method, and this method will throw an error if they get
  it wrong, but the state of the world might have changed since they
  checked it and that’s not their fault), but I’ve kept them for now.

- Chose to use existential types when one protocol returns another (i.e.
  `-> any MyProtocol`) instead of associated types, because it’s more
  readable (you can’t use opaque types in a protocol declaration) and I
  don’t think that we need to worry about the performance implications.

- I’ve deferred adding helpful protocol conformances (Equatable, Hashable,
  Codable etc) to #10.

- I’ve turned on the explicit_acl SwiftLint rule, which forces us to add
  an access control modifier to all declarations. This makes it less
  likely that we forget to make something `public` instead of the default
  `internal`.

Resolves #7.

[1] https://www.douggregor.net/posts/swift-for-cxx-practitioners-error-handling/
lawrence-forooghian added a commit that referenced this issue Aug 19, 2024
Based on JS repo at 0b4b0f8. I have not performed any review or critique
of the JS API, and simply copied it and mildly adapted to Swift.

There are some things that I’m unsure about regarding the usability and
implementability of this Swift API. We’ll be able to validate the
usability when we do #4, which will create a mock implementation of this
API and then build the example app around the mock. Implementability
we’ll discover as we try to build the SDK.

This is just a first attempt and all of the decisions can be revisited.

TypeScript-to-Swift decisions:

- I’ve named the entry point class DefaultChatClient instead of their
  ChatClient; this is so that I can have a public protocol named
  ChatClient.

- My `throws` annotations are based on the JS docstrings (or in a few
  cases my assumptions). The Rooms accessors that throw in JS if a feature
  is not enabled instead call fatalError in Swift, which is the idiomatic
  thing to do for programmer errors [1].

- Skipped copying the docstrings from JS to avoid churn; created #1 to do
  this later. Turned off missing_docs for now.

- The listener pattern in JS is instead implemented by returning an
  AsyncSequence. I believe that we do not need an equivalent of the `off*`
  / `unsubscribe*` methods since iterating over an AsyncSequence is a pull
  rather than a push. And I believe (but am still quite shaky about the
  details, so may be wrong) that there are AsyncSequence lifecycle events
  (e.g. end of iteration, task cancellation) that we can use to manage the
  underlying ably-cocoa listeners.

- RoomOptionsDefaults in JS is instead implemented here by giving the
  *Options types a no-args initializer that populates the default values.

- I’ve copied the logging interface more or less from JS (but with
  LogHandler a protocol so that we can have argument labels). Will think
  about something more idiomatic in #8.

Swift decisions and thoughts:

- My decision on what should be a protocol and what should be a concrete
  type was fairly arbitrary; I’ve made everything a protocol (for
  mockability) except structs that are basically just containers for data
  (but this line is blurry; for example, this might introduce issues for
  somebody who wants to be able to mock Message’s isBefore(_:) method).
  One downside of using protocols is that you can’t nest types inside them
  (this would be nice for e.g. related enums) but it’s alright.

- I’ve annotated all of the protocols that feel like they represent some
  sort of client with AnyObject; I don’t have a great explanation of why
  but intuitively it felt right (seemed like they should be reference
  types).

- Having not yet used Swift concurrency much, I didn’t have a good
  intuition for what things should be Sendable, so I’ve put it on pretty
  much everything. Similarly, I don’t have a good sense of what should be
  annotated as `async` (for some of this our hand will probably also end
  up being forced by the implementation). I also am not sure whether the
  `current` / `error` properties for connection and room status make sense
  in a world where most things are async (especially if the intention is
  that, for example, the user check `current` before deciding whether to
  call a certain method, and this method will throw an error if they get
  it wrong, but the state of the world might have changed since they
  checked it and that’s not their fault), but I’ve kept them for now.

- Chose to use existential types when one protocol returns another (i.e.
  `-> any MyProtocol`) instead of associated types, because it’s more
  readable (you can’t use opaque types in a protocol declaration) and I
  don’t think that we need to worry about the performance implications.

- I’ve deferred adding helpful protocol conformances (Equatable, Hashable,
  Codable etc) to #10.

- I’ve turned on the explicit_acl SwiftLint rule, which forces us to add
  an access control modifier to all declarations. This makes it less
  likely that we forget to make something `public` instead of the default
  `internal`.

Resolves #7.

[1] https://www.douggregor.net/posts/swift-for-cxx-practitioners-error-handling/
lawrence-forooghian added a commit that referenced this issue Aug 19, 2024
Based on JS repo at 0b4b0f8. I have not performed any review or critique
of the JS API, and simply copied it and mildly adapted to Swift.

There are some things that I’m unsure about regarding the usability and
implementability of this Swift API. We’ll be able to validate the
usability when we do #4, which will create a mock implementation of this
API and then build the example app around the mock. Implementability
we’ll discover as we try to build the SDK.

This is just a first attempt and all of the decisions can be revisited.

TypeScript-to-Swift decisions:

- I’ve named the entry point class DefaultChatClient instead of their
  ChatClient; this is so that I can have a public protocol named
  ChatClient.

- My `throws` annotations are based on the JS docstrings (or in a few
  cases my assumptions). The Rooms accessors that throw in JS if a feature
  is not enabled instead call fatalError in Swift, which is the idiomatic
  thing to do for programmer errors [1].

- Skipped copying the docstrings from JS to avoid churn; created #1 to do
  this later. Turned off missing_docs for now.

- The listener pattern in JS is instead implemented by returning an
  AsyncSequence. This was partly because of Umair’s architecture thoughts
  [2] which — at least my reading of it — indicated a desire to use some
  sort of reactive API, and partly because I was curious to try it out,
  having not used it before. I believe that we do not need an equivalent
  of the `off*` / `unsubscribe*` methods since iterating over an
  AsyncSequence is a pull rather than a push. And I believe (but am still
  quite shaky about the details, so may be wrong) that there are
  AsyncSequence lifecycle events (e.g. end of iteration, task
  cancellation) that we can use to manage the underlying ably-cocoa
  listeners. And, I’m sure that there will be things we have to consider
  about how to make sure that we don’t have leaks of MessageSubscriptions
  which cause messages to start accumulating in buffer that the user
  forgot exists.

- RoomOptionsDefaults in JS is instead implemented here by giving the
  *Options types a no-args initializer that populates the default values.

- I’ve copied the logging interface more or less from JS (but with
  LogHandler a protocol so that we can have argument labels). Will think
  about something more idiomatic in #8.

Swift decisions and thoughts:

- My decision on what should be a protocol and what should be a concrete
  type was fairly arbitrary; I’ve made everything a protocol (for
  mockability) except structs that are basically just containers for data
  (but this line is blurry; for example, this might introduce issues for
  somebody who wants to be able to mock Message’s isBefore(_:) method).
  One downside of using protocols is that you can’t nest types inside them
  (this would be nice for e.g. related enums) but it’s alright.

- I’ve annotated all of the protocols that feel like they represent some
  sort of client with AnyObject; I don’t have a great explanation of why
  but intuitively it felt right (seemed like they should be reference
  types).

- Having not yet used Swift concurrency much, I didn’t have a good
  intuition for what things should be Sendable, so I’ve put it on pretty
  much everything. Similarly, I don’t have a good sense of what should be
  annotated as `async` (for some of this our hand will probably also end
  up being forced by the implementation). I also am not sure whether the
  `current` / `error` properties for connection and room status make sense
  in a world where most things are async (especially if the intention is
  that, for example, the user check `current` before deciding whether to
  call a certain method, and this method will throw an error if they get
  it wrong, but the state of the world might have changed since they
  checked it and that’s not their fault), but I’ve kept them for now.

- Chose to use existential types when one protocol returns another (i.e.
  `-> any MyProtocol`) instead of associated types, because it’s more
  readable (you can’t use opaque types in a protocol declaration) and I
  don’t think that we need to worry about the performance implications.

- I’ve deferred adding helpful protocol conformances (Equatable, Hashable,
  Codable etc) to #10.

- I’ve turned on the explicit_acl SwiftLint rule, which forces us to add
  an access control modifier to all declarations. This makes it less
  likely that we forget to make something `public` instead of the default
  `internal`.

Resolves #7.

[1] https://www.douggregor.net/posts/swift-for-cxx-practitioners-error-handling/
[2] https://ably.atlassian.net/wiki/spaces/SDK/pages/3261202438/Swift+Architecture+Thoughts
lawrence-forooghian added a commit that referenced this issue Aug 19, 2024
Based on JS repo at 0b4b0f8. I have not performed any review or critique
of the JS API, and simply copied it and mildly adapted to Swift.

There are some things that I’m unsure about regarding the usability and
implementability of this Swift API. We’ll be able to validate the
usability when we do #4, which will create a mock implementation of this
API and then build the example app around the mock. Implementability
we’ll discover as we try to build the SDK.

This is just a first attempt and all of the decisions can be revisited.

TypeScript-to-Swift decisions:

- I’ve named the entry point class DefaultChatClient instead of their
  ChatClient; this is so that I can have a public protocol named
  ChatClient.

- My `throws` annotations are based on the JS docstrings (or in a few
  cases my assumptions). The Rooms accessors that throw in JS if a feature
  is not enabled instead call fatalError in Swift, which is the idiomatic
  thing to do for programmer errors [1].

- Skipped copying the docstrings from JS to avoid churn; created #1 to do
  this later. Turned off missing_docs for now.

- The listener pattern in JS is instead implemented by returning an
  AsyncSequence. This was partly because of Umair’s architecture thoughts
  [2] which — at least my reading of it — indicated a desire to use some
  sort of reactive API, and partly because I was curious to try it out,
  having not used it before. I believe that we do not need an equivalent
  of the `off*` / `unsubscribe*` methods since iterating over an
  AsyncSequence is a pull rather than a push. And I believe (but am still
  quite shaky about the details, so may be wrong) that there are
  AsyncSequence lifecycle events (e.g. end of iteration, task
  cancellation) that we can use to manage the underlying ably-cocoa
  listeners. And, I’m sure that there will be things we have to consider
  about how to make sure that we don’t have leaks of MessageSubscriptions
  which cause messages to start accumulating in buffer that the user
  forgot exists.

- RoomOptionsDefaults in JS is instead implemented here by giving the
  *Options types a no-args initializer that populates the default values.

- I’ve copied the logging interface more or less from JS (but with
  LogHandler a protocol so that we can have argument labels). Will think
  about something more idiomatic in #8.

Swift decisions and thoughts:

- My decision on what should be a protocol and what should be a concrete
  type was fairly arbitrary; I’ve made everything a protocol (for
  mockability) except structs that are basically just containers for data
  (but this line is blurry; for example, this might introduce issues for
  somebody who wants to be able to mock Message’s isBefore(_:) method).
  One downside of using protocols is that you can’t nest types inside them
  (this would be nice for e.g. related enums) but it’s alright.

- I’ve annotated all of the protocols that feel like they represent some
  sort of client with AnyObject; I don’t have a great explanation of why
  but intuitively it felt right (seemed like they should be reference
  types).

- Having not yet used Swift concurrency much, I didn’t have a good
  intuition for what things should be Sendable, so I’ve put it on pretty
  much everything. Similarly, I don’t have a good sense of what should be
  annotated as `async` (for some of this our hand will probably also end
  up being forced by the implementation). I also am not sure whether the
  `current` / `error` properties for connection and room status make sense
  in a world where most things are async (especially if the intention is
  that, for example, the user check `current` before deciding whether to
  call a certain method, and this method will throw an error if they get
  it wrong, but the state of the world might have changed since they
  checked it and that’s not their fault), but I’ve kept them for now.

- Chose to use existential types when one protocol returns another (i.e.
  `-> any MyProtocol`) instead of associated types, because it’s more
  readable (you can’t use opaque types in a protocol declaration) and I
  don’t think that we need to worry about the performance implications.

- I’ve deferred adding helpful protocol conformances (Equatable, Hashable,
  Codable etc) to #10.

- I’ve turned on the explicit_acl SwiftLint rule, which forces us to add
  an access control modifier to all declarations. This makes it less
  likely that we forget to make something `public` instead of the default
  `internal`.

Resolves #7.

[1] https://www.douggregor.net/posts/swift-for-cxx-practitioners-error-handling/
[2] https://ably.atlassian.net/wiki/spaces/SDK/pages/3261202438/Swift+Architecture+Thoughts
lawrence-forooghian added a commit that referenced this issue Aug 20, 2024
Based on JS repo at 0b4b0f8. I have not performed any review or critique
of the JS API, and simply copied it and mildly adapted to Swift.

There are some things that I’m unsure about regarding the usability and
implementability of this Swift API. We’ll be able to validate the
usability when we do #4, which will create a mock implementation of this
API and then build the example app around the mock. Implementability
we’ll discover as we try to build the SDK.

This is just a first attempt and all of the decisions can be revisited.

TypeScript-to-Swift decisions:

- I’ve named the entry point class DefaultChatClient instead of their
  ChatClient; this is so that I can have a public protocol named
  ChatClient.

- My `throws` annotations are based on the JS docstrings (or in a few
  cases my assumptions). The Rooms accessors that throw in JS if a feature
  is not enabled instead call fatalError in Swift, which is the idiomatic
  thing to do for programmer errors [1].

- Skipped copying the docstrings from JS to avoid churn; created #1 to do
  this later. Turned off missing_docs for now.

- The listener pattern in JS is instead implemented by returning an
  AsyncSequence. This was partly because of Umair’s architecture thoughts
  [2] which — at least my reading of it — indicated a desire to use some
  sort of reactive API, and partly because I was curious to try it out,
  having not used it before. I believe that we do not need an equivalent
  of the `off*` / `unsubscribe*` methods since iterating over an
  AsyncSequence is a pull rather than a push. And I believe (but am still
  quite shaky about the details, so may be wrong) that there are
  AsyncSequence lifecycle events (e.g. end of iteration, task
  cancellation) that we can use to manage the underlying ably-cocoa
  listeners. And, I’m sure that there will be things we have to consider
  about how to make sure that we don’t have leaks of MessageSubscriptions
  which cause messages to start accumulating in buffer that the user
  forgot exists.

- RoomOptionsDefaults in JS is instead implemented here by giving the
  *Options types a no-args initializer that populates the default values.

- I’ve copied the logging interface more or less from JS (but with
  LogHandler a protocol so that we can have argument labels). Will think
  about something more idiomatic in #8.

Swift decisions and thoughts:

- My decision on what should be a protocol and what should be a concrete
  type was fairly arbitrary; I’ve made everything a protocol (for
  mockability) except structs that are basically just containers for data
  (but this line is blurry; for example, this might introduce issues for
  somebody who wants to be able to mock Message’s isBefore(_:) method).
  One downside of using protocols is that you can’t nest types inside them
  (this would be nice for e.g. related enums) but it’s alright.

- I’ve annotated all of the protocols that feel like they represent some
  sort of client with AnyObject; I don’t have a great explanation of why
  but intuitively it felt right (seemed like they should be reference
  types).

- Having not yet used Swift concurrency much, I didn’t have a good
  intuition for what things should be Sendable, so I’ve put it on pretty
  much everything. Similarly, I don’t have a good sense of what should be
  annotated as `async` (for some of this our hand will probably also end
  up being forced by the implementation). I also am not sure whether the
  `current` / `error` properties for connection and room status make sense
  in a world where most things are async (especially if the intention is
  that, for example, the user check `current` before deciding whether to
  call a certain method, and this method will throw an error if they get
  it wrong, but the state of the world might have changed since they
  checked it and that’s not their fault), but I’ve kept them for now.

- Chose to use existential types when one protocol returns another (i.e.
  `-> any MyProtocol`) instead of associated types, because it’s more
  readable (you can’t use opaque types in a protocol declaration) and I
  don’t think that we need to worry about the performance implications.

- I’ve deferred adding helpful protocol conformances (Equatable, Hashable,
  Codable etc) to #10.

- I’ve turned on the explicit_acl SwiftLint rule, which forces us to add
  an access control modifier to all declarations. This makes it less
  likely that we forget to make something `public` instead of the default
  `internal`.

Resolves #7.

[1] https://www.douggregor.net/posts/swift-for-cxx-practitioners-error-handling/
[2] https://ably.atlassian.net/wiki/spaces/SDK/pages/3261202438/Swift+Architecture+Thoughts
lawrence-forooghian added a commit that referenced this issue Aug 24, 2024
Based on JS repo at 0b4b0f8. I have not performed any review or critique
of the JS API, and simply copied it and mildly adapted to Swift.

There are some things that I’m unsure about regarding the usability and
implementability of this Swift API. We’ll be able to validate the
usability when we do #4, which will create a mock implementation of this
API and then build the example app around the mock. Implementability
we’ll discover as we try to build the SDK.

This is just a first attempt and all of the decisions can be revisited.

TypeScript-to-Swift decisions:

- I’ve named the entry point class DefaultChatClient instead of their
  ChatClient; this is so that I can have a public protocol named
  ChatClient.

- My `throws` annotations are based on the JS docstrings (or in a few
  cases my assumptions). The Rooms accessors that throw in JS if a feature
  is not enabled instead call fatalError in Swift, which is the idiomatic
  thing to do for programmer errors [1].

- Skipped copying the docstrings from JS to avoid churn; created #1 to do
  this later. Turned off missing_docs for now.

- The listener pattern in JS is instead implemented by returning an
  AsyncSequence. This was partly because of Umair’s architecture thoughts
  [2] which — at least my reading of it — indicated a desire to use some
  sort of reactive API, and partly because I was curious to try it out,
  having not used it before. I believe that we do not need an equivalent
  of the `off*` / `unsubscribe*` methods since iterating over an
  AsyncSequence is a pull rather than a push. And I believe (but am still
  quite shaky about the details, so may be wrong) that there are
  AsyncSequence lifecycle events (e.g. end of iteration, task
  cancellation) that we can use to manage the underlying ably-cocoa
  listeners. And, I’m sure that there will be things we have to consider
  about how to make sure that we don’t have leaks of MessageSubscriptions
  which cause messages to start accumulating in buffer that the user
  forgot exists.

- RoomOptionsDefaults in JS is instead implemented here by giving the
  *Options types a no-args initializer that populates the default values.

- I’ve copied the logging interface more or less from JS (but with
  LogHandler a protocol so that we can have argument labels). Will think
  about something more idiomatic in #8.

Swift decisions and thoughts:

- My decision on what should be a protocol and what should be a concrete
  type was fairly arbitrary; I’ve made everything a protocol (for
  mockability) except structs that are basically just containers for data
  (but this line is blurry; for example, this might introduce issues for
  somebody who wants to be able to mock Message’s isBefore(_:) method).
  One downside of using protocols is that you can’t nest types inside them
  (this would be nice for e.g. related enums) but it’s alright.

- I’ve annotated all of the protocols that feel like they represent some
  sort of client with AnyObject; I don’t have a great explanation of why
  but intuitively it felt right (seemed like they should be reference
  types).

- Having not yet used Swift concurrency much, I didn’t have a good
  intuition for what things should be Sendable, so I’ve put it on pretty
  much everything. Similarly, I don’t have a good sense of what should be
  annotated as `async` (for some of this our hand will probably also end
  up being forced by the implementation). I also am not sure whether the
  `current` / `error` properties for connection and room status make sense
  in a world where most things are async (especially if the intention is
  that, for example, the user check `current` before deciding whether to
  call a certain method, and this method will throw an error if they get
  it wrong, but the state of the world might have changed since they
  checked it and that’s not their fault), but I’ve kept them for now.

- Chose to use existential types when one protocol returns another (i.e.
  `-> any MyProtocol`) instead of associated types, because it’s more
  readable (you can’t use opaque types in a protocol declaration) and I
  don’t think that we need to worry about the performance implications.

- I’ve deferred adding helpful protocol conformances (Equatable, Hashable,
  Codable etc) to #10.

- I’ve turned on the explicit_acl SwiftLint rule, which forces us to add
  an access control modifier to all declarations. This makes it less
  likely that we forget to make something `public` instead of the default
  `internal`.

Resolves #7.

[1] https://www.douggregor.net/posts/swift-for-cxx-practitioners-error-handling/
[2] https://ably.atlassian.net/wiki/spaces/SDK/pages/3261202438/Swift+Architecture+Thoughts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant