[ECO-5577] Get rid of existential type usage in the public API#364
[ECO-5577] Get rid of existential type usage in the public API#364lawrence-forooghian merged 2 commits intomainfrom
Conversation
WalkthroughReplaces many public-api existentials with associated types or opaque Changes
Sequence Diagram(s)(Skipped — changes are type/signature-level and do not introduce new runtime control-flow interactions.) Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (28)
🚧 Files skipped from review as they are similar to previous changes (10)
🧰 Additional context used🧬 Code graph analysis (15)Sources/AblyChat/Connection.swift (4)
Tests/AblyChatTests/MessageSubscriptionAsyncSequenceTests.swift (2)
Sources/AblyChat/Presence.swift (2)
Sources/AblyChat/Messages.swift (3)
Sources/AblyChat/DefaultConnection.swift (2)
Sources/AblyChat/RoomReactions.swift (2)
Example/AblyChatExample/Mocks/Misc.swift (1)
Example/AblyChatExample/Mocks/MockSubscriptionStorage.swift (1)
Sources/AblyChat/SubscriptionStorage.swift (1)
Sources/AblyChat/Subscription.swift (1)
Tests/AblyChatTests/Mocks/MockRoom.swift (4)
Sources/AblyChat/Room.swift (5)
Sources/AblyChat/MessageReactions.swift (2)
Sources/AblyChat/Typing.swift (5)
Example/AblyChatExample/Mocks/MockClients.swift (9)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
🔇 Additional comments (39)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
Sources/AblyChat/DefaultOccupancy.swift (1)
21-26: Fix message: “presence events” → “occupancy events”.User-facing fatalError text mentions presence; this is occupancy.
- fatalError("In order to be able to subscribe to presence events, please set enableEvents to true in the room's occupancy options.") + fatalError("In order to be able to subscribe to occupancy events, please set enableEvents to true in the room's occupancy options.")Sources/AblyChat/Messages.swift (1)
11-24: Avoid exposingMessagesas a protocol existential
Room.swift:30: changenonisolated var messages: Messages { get }to returnsome Messagesor introduce a generic type parameter.
Audit all other public APIs returning or acceptingMessagesand update them similarly.Sources/AblyChat/DefaultMessageReactions.swift (1)
104-111: Enforce spec CHA-MR7a: throwARTErrorInfoinstead of crashing
- In Sources/AblyChat/DefaultMessageReactions.swift (around line 106), replace
fatalError("Room is not configured to support raw message reactions")
with
throw ARTErrorInfo(chatError: .badRequest(message: "Room is not configured to support raw message reactions", code: 40000)).- Change the protocol definition in Sources/AblyChat/MessageReactions.swift from
func subscribeRaw(_ callback: …) -> Subscription
to
func subscribeRaw(_ callback: …) throws -> Subscription
and update bothsubscribeRaw(bufferingPolicy:)and the no-argsubscribeRaw()AsyncSequence variants to propagate the thrown error.Sources/AblyChat/DefaultMessages.swift (1)
150-185: Filteronceor callunsubscribeon your listener to avoid hanging
resolveSubscriptionStart()useschannel.once { … }, which can fire on an intermediate state (e.g..attaching) and never resume. Two fixes:
Use the overload that listens only for
.attached:- _ = channel.once { [weak self] stateChange in + _ = channel.once(.attached) { [weak self] stateChange in guard let self else { return } let serial = stateChange.resumed ? channel.properties.channelSerial : channel.properties.attachSerial if let serial { continuation.resume(returning: serial) } else { continuation.resume(throwing: ARTErrorInfo(chatError: .attachSerialIsNotDefined).toInternalError()) } }Or subscribe with
onand callchannel.unsubscribe(listener)inside terminal branches:return try await withCheckedContinuation { continuation in let listener = channel.on { [weak self] stateChange in guard let self else { return } switch stateChange.current { case .attached: channel.unsubscribe(listener) continuation.resume(returning: /* serial */) case .failed, .suspended: channel.unsubscribe(listener) continuation.resume(throwing: ARTErrorInfo(chatError: .channelFailedToAttach(cause: stateChange.reason)).toInternalError()) default: break } } }.get()
🧹 Nitpick comments (14)
CONTRIBUTING.md (1)
43-44: Good clarification on avoiding public existentials; add one nuance for “some” usage.Consider explicitly noting that “some Protocol” is only allowed in return positions; for stored properties/surfaces prefer associated types, generics, or concrete wrappers. This will prevent misuse when replacing
anywithsome.Sources/AblyChat/DefaultOccupancy.swift (2)
20-22: Consider marking the callback @sendable (consistency with other subscribe APIs).Other subscribe methods annotate callbacks as @sendable to pacify concurrency diagnostics noted in CONTRIBUTING.
- internal func subscribe(_ callback: @escaping @MainActor (OccupancyEvent) -> Void) -> some SubscriptionProtocol { + internal func subscribe(_ callback: @escaping @MainActor @Sendable (OccupancyEvent) -> Void) -> some SubscriptionProtocol {
52-57: Useunsubscribefor message listener removal
In DefaultOccupancy.swift’s DefaultSubscription closure, replace:-self.channel.off(eventListener) +self.channel.unsubscribe(eventListener)Sources/AblyChat/Messages.swift (2)
11-14: Tie associated types together to prevent mismatches.Ensure
SubscribeResponse.HistoryResultmatchesHistoryResultto keep API coherent.-public protocol Messages: AnyObject, Sendable { - associatedtype Reactions: MessageReactions - associatedtype SubscribeResponse: MessageSubscriptionResponseProtocol - associatedtype HistoryResult: PaginatedResult<Message> +public protocol Messages: AnyObject, Sendable { + associatedtype Reactions: MessageReactions + associatedtype HistoryResult: PaginatedResult<Message> + associatedtype SubscribeResponse: MessageSubscriptionResponseProtocol where SubscribeResponse.HistoryResult == HistoryResult
23-24: Consider marking the callback @sendable.Matches other subscribe APIs and avoids concurrency diagnostics noted in CONTRIBUTING.
- func subscribe(_ callback: @escaping @MainActor (ChatMessageEvent) -> Void) -> SubscribeResponse + func subscribe(_ callback: @escaping @MainActor @Sendable (ChatMessageEvent) -> Void) -> SubscribeResponseExample/AblyChatExample/Mocks/MockSubscriptionStorage.swift (1)
133-133: Annotate onTerminate with @mainactor for consistencyOther storages mark onTerminate as
@MainActor. This one doesn’t. For consistency and to avoid accidental cross-actor hops, annotate it.- onTerminate: @escaping () -> Void, + onTerminate: @escaping @MainActor () -> Void,Sources/AblyChat/ChatClient.swift (1)
74-77: Mark connection accessor as nonisolated to match protocolProtocol requires
nonisolated var connection. Consider annotating the witness for clarity and to avoid unintended actor hops.- public var connection: some Connection { + public nonisolated var connection: some Connection { _connection }Sources/AblyChat/Subscription.swift (1)
59-69: Preserve actor/sendable annotations on stored closuresStore closure properties as @mainactor @sendable to retain concurrency guarantees.
-internal struct DefaultSubscription: SubscriptionProtocol, Sendable { - private let _unsubscribe: () -> Void +internal struct DefaultSubscription: SubscriptionProtocol, Sendable { + private let _unsubscribe: @MainActor @Sendable () -> Void @@ - internal init(unsubscribe: @MainActor @Sendable @escaping () -> Void) { + internal init(unsubscribe: @MainActor @Sendable @escaping () -> Void) { _unsubscribe = unsubscribe } } -internal struct DefaultStatusSubscription: StatusSubscriptionProtocol, Sendable { - private let _off: () -> Void +internal struct DefaultStatusSubscription: StatusSubscriptionProtocol, Sendable { + private let _off: @MainActor @Sendable () -> Void @@ - internal init(off: @MainActor @Sendable @escaping () -> Void) { + internal init(off: @MainActor @Sendable @escaping () -> Void) { _off = off } }Also applies to: 71-81
Example/AblyChatExample/Mocks/MockClients.swift (2)
108-110: Avoid fatalError in onDiscontinuity mock.Implement a no-op subscription to prevent crashes in example code paths.
- func onDiscontinuity(_: @escaping @MainActor (DiscontinuityEvent) -> Void) -> DefaultStatusSubscription { - fatalError("Not yet implemented") - } + func onDiscontinuity(_: @escaping @MainActor (DiscontinuityEvent) -> Void) -> DefaultStatusSubscription { + var active = true + return DefaultStatusSubscription { active = false } + }
306-308: Avoid fatalError in subscribeRaw mock.Provide a no-op subscription like other mocks to keep examples robust.
- func subscribeRaw(_: @escaping @MainActor @Sendable (MessageReactionRawEvent) -> Void) -> MockSubscription { - fatalError("Not implemented") - } + func subscribeRaw(_: @escaping @MainActor @Sendable (MessageReactionRawEvent) -> Void) -> MockSubscription { + MockSubscription { /* no-op */ } + }Sources/AblyChat/SubscriptionStorage.swift (2)
15-24: Unify return type with concreteDefaultSubscription.
createstill returnsany SubscriptionProtocolwhile the storage now constructs and storesDefaultSubscription(and the status storage returnsDefaultStatusSubscription). To reduce existential usage and keep types consistent, returnDefaultSubscription.Apply:
- internal func create(_ callback: @escaping @MainActor (Element) -> Void) -> any SubscriptionProtocol { + internal func create(_ callback: @escaping @MainActor (Element) -> Void) -> DefaultSubscription { let id = UUID() let subscription = DefaultSubscription { [weak self] in self?.subscriptionDidTerminate(id: id) } let subscriptionItem = SubscriptionItem(callback: callback, subscription: subscription) subscriptions[id] = subscriptionItem return subscription }
36-41: Doc nit: “receiver” spelling.“reciever’s” → “receiver’s”. Optional cleanup.
Also applies to: 77-83
Sources/AblyChat/DefaultMessages.swift (2)
39-90: Ensurecallbackruns on MainActor.
callbackis@MainActor. Confirmchannel.subscribeinvokes its closure on MainActor. If not guaranteed, hop explicitly to MainActor when callingcallback.Example:
- let event = ChatMessageEvent(message: message) - callback(event) + let event = ChatMessageEvent(message: message) + Task { @MainActor in + callback(event) + }Also consider renaming the inner
messagemodel var to avoid shadowing the parameter (readability).
95-115: Closure signature likely needsasync(usesawait).
subscriptionStartSerialexplicitly annotatesthrows(InternalError)but callstry await. If the initializer expects anasync throwsclosure, either:
- remove the explicit signature and let the compiler infer
async throws, or- add
asyncexplicitly.Apply:
- subscriptionStartSerial: { [weak self] () throws(InternalError) in + subscriptionStartSerial: { [weak self] () async throws(InternalError) in guard let self else { throw MessagesError.noReferenceToSelf.toInternalError() } if channel.state == .attached, let startSerial = subscriptionPoints[uuid] { return startSerial } let startSerial = try await resolveSubscriptionStart() subscriptionPoints[uuid] = startSerial return startSerial },If the initializer already infers
async throws, prefer:- subscriptionStartSerial: { [weak self] () throws(InternalError) in + subscriptionStartSerial: { [weak self] in
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (28)
CONTRIBUTING.md(1 hunks)Example/AblyChatExample/Mocks/Misc.swift(1 hunks)Example/AblyChatExample/Mocks/MockClients.swift(15 hunks)Example/AblyChatExample/Mocks/MockSubscriptionStorage.swift(8 hunks)Sources/AblyChat/ChatAPI.swift(2 hunks)Sources/AblyChat/ChatClient.swift(4 hunks)Sources/AblyChat/Connection.swift(2 hunks)Sources/AblyChat/DefaultConnection.swift(2 hunks)Sources/AblyChat/DefaultMessageReactions.swift(3 hunks)Sources/AblyChat/DefaultMessages.swift(4 hunks)Sources/AblyChat/DefaultOccupancy.swift(2 hunks)Sources/AblyChat/DefaultPresence.swift(3 hunks)Sources/AblyChat/DefaultRoomReactions.swift(2 hunks)Sources/AblyChat/DefaultTyping.swift(2 hunks)Sources/AblyChat/MessageReactions.swift(3 hunks)Sources/AblyChat/Messages.swift(8 hunks)Sources/AblyChat/Occupancy.swift(2 hunks)Sources/AblyChat/PaginatedResult.swift(4 hunks)Sources/AblyChat/Presence.swift(3 hunks)Sources/AblyChat/Room.swift(13 hunks)Sources/AblyChat/RoomLifecycleManager.swift(4 hunks)Sources/AblyChat/RoomReactions.swift(2 hunks)Sources/AblyChat/Subscription.swift(5 hunks)Sources/AblyChat/SubscriptionStorage.swift(2 hunks)Sources/AblyChat/Typing.swift(2 hunks)Tests/AblyChatTests/MessageSubscriptionAsyncSequenceTests.swift(2 hunks)Tests/AblyChatTests/Mocks/MockRoom.swift(2 hunks)Tests/AblyChatTests/Mocks/MockRoomLifecycleManager.swift(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (24)
Sources/AblyChat/DefaultConnection.swift (2)
Example/AblyChatExample/Mocks/MockClients.swift (2)
onStatusChange(90-105)onStatusChange(587-600)Sources/AblyChat/Connection.swift (2)
onStatusChange(43-57)onStatusChange(60-62)
Sources/AblyChat/Connection.swift (2)
Example/AblyChatExample/Mocks/MockClients.swift (2)
onStatusChange(90-105)onStatusChange(587-600)Sources/AblyChat/DefaultConnection.swift (1)
onStatusChange(23-82)
Sources/AblyChat/DefaultOccupancy.swift (4)
Sources/AblyChat/DefaultMessageReactions.swift (1)
subscribe(62-100)Sources/AblyChat/DefaultPresence.swift (2)
subscribe(202-222)subscribe(224-247)Sources/AblyChat/DefaultRoomReactions.swift (1)
subscribe(36-79)Sources/AblyChat/Occupancy.swift (2)
subscribe(55-69)subscribe(72-74)
Example/AblyChatExample/Mocks/Misc.swift (1)
Tests/AblyChatTests/Mocks/MockHTTPPaginatedResponse.swift (2)
next(46-48)first(50-52)
Sources/AblyChat/DefaultMessages.swift (2)
Example/AblyChatExample/Mocks/MockClients.swift (8)
subscribe(127-155)subscribe(280-304)subscribe(335-352)subscribe(366-382)subscribe(534-536)subscribe(538-540)subscribe(554-565)history(157-159)Sources/AblyChat/Messages.swift (2)
subscribe(95-116)subscribe(119-121)
Sources/AblyChat/RoomLifecycleManager.swift (5)
Tests/AblyChatTests/Mocks/MockRoomLifecycleManager.swift (2)
onRoomStatusChange(77-80)onDiscontinuity(82-85)Tests/AblyChatTests/Helpers/RoomLifecycleManager+AsyncSequence.swift (3)
onRoomStatusChange(6-20)onRoomStatusChange(22-24)onDiscontinuity(26-39)Example/AblyChatExample/Mocks/MockClients.swift (1)
onDiscontinuity(107-110)Sources/AblyChat/Room.swift (3)
onDiscontinuity(172-185)onDiscontinuity(188-190)onDiscontinuity(397-400)Tests/AblyChatTests/Mocks/MockRoom.swift (1)
onDiscontinuity(72-75)
Sources/AblyChat/Occupancy.swift (2)
Example/AblyChatExample/Mocks/MockClients.swift (7)
subscribe(127-155)subscribe(280-304)subscribe(335-352)subscribe(366-382)subscribe(534-536)subscribe(538-540)subscribe(554-565)Sources/AblyChat/DefaultOccupancy.swift (1)
subscribe(20-58)
Tests/AblyChatTests/MessageSubscriptionAsyncSequenceTests.swift (2)
Sources/AblyChat/SubscriptionAsyncSequence.swift (4)
next(44-46)next(119-130)next(139-141)emit(72-79)Tests/AblyChatTests/SubscriptionAsyncSequenceTests.swift (1)
emit(13-23)
Sources/AblyChat/MessageReactions.swift (2)
Example/AblyChatExample/Mocks/MockClients.swift (8)
subscribe(127-155)subscribe(280-304)subscribe(335-352)subscribe(366-382)subscribe(534-536)subscribe(538-540)subscribe(554-565)subscribeRaw(306-308)Sources/AblyChat/DefaultMessageReactions.swift (2)
subscribe(62-100)subscribeRaw(103-149)
Sources/AblyChat/Typing.swift (5)
Example/AblyChatExample/Mocks/MockClients.swift (6)
subscribe(127-155)subscribe(280-304)subscribe(335-352)subscribe(366-382)subscribe(534-536)subscribe(538-540)Sources/AblyChat/Messages.swift (2)
subscribe(95-116)subscribe(119-121)Sources/AblyChat/Occupancy.swift (2)
subscribe(55-69)subscribe(72-74)Sources/AblyChat/Presence.swift (4)
subscribe(139-153)subscribe(166-180)subscribe(183-185)subscribe(188-190)Sources/AblyChat/RoomReactions.swift (2)
subscribe(44-58)subscribe(61-63)
Sources/AblyChat/Presence.swift (2)
Example/AblyChatExample/Mocks/MockClients.swift (7)
subscribe(127-155)subscribe(280-304)subscribe(335-352)subscribe(366-382)subscribe(534-536)subscribe(538-540)subscribe(554-565)Sources/AblyChat/DefaultPresence.swift (2)
subscribe(202-222)subscribe(224-247)
Sources/AblyChat/RoomReactions.swift (6)
Example/AblyChatExample/Mocks/MockClients.swift (5)
subscribe(127-155)subscribe(280-304)subscribe(335-352)subscribe(366-382)subscribe(534-536)Sources/AblyChat/DefaultRoomReactions.swift (1)
subscribe(36-79)Sources/AblyChat/Messages.swift (2)
subscribe(95-116)subscribe(119-121)Sources/AblyChat/Occupancy.swift (2)
subscribe(55-69)subscribe(72-74)Sources/AblyChat/Presence.swift (4)
subscribe(139-153)subscribe(166-180)subscribe(183-185)subscribe(188-190)Sources/AblyChat/Typing.swift (2)
subscribe(63-77)subscribe(80-82)
Tests/AblyChatTests/Mocks/MockRoom.swift (4)
Example/AblyChatExample/Mocks/MockClients.swift (3)
onStatusChange(90-105)onStatusChange(587-600)onDiscontinuity(107-110)Sources/AblyChat/Room.swift (6)
onStatusChange(143-157)onStatusChange(160-162)onStatusChange(386-389)onDiscontinuity(172-185)onDiscontinuity(188-190)onDiscontinuity(397-400)Sources/AblyChat/RoomLifecycleManager.swift (1)
onDiscontinuity(226-229)Tests/AblyChatTests/Mocks/MockRoomLifecycleManager.swift (1)
onDiscontinuity(82-85)
Sources/AblyChat/Messages.swift (3)
Example/AblyChatExample/Mocks/MockClients.swift (8)
subscribe(127-155)subscribe(280-304)subscribe(335-352)subscribe(366-382)subscribe(534-536)subscribe(538-540)subscribe(554-565)history(157-159)Sources/AblyChat/DefaultMessages.swift (2)
subscribe(39-115)history(118-124)Tests/AblyChatTests/MessageSubscriptionAsyncSequenceTests.swift (1)
mockGetPreviousMessages(49-58)
Tests/AblyChatTests/Mocks/MockRoomLifecycleManager.swift (5)
Sources/AblyChat/RoomLifecycleManager.swift (2)
onRoomStatusChange(182-185)onDiscontinuity(226-229)Tests/AblyChatTests/Helpers/RoomLifecycleManager+AsyncSequence.swift (3)
onRoomStatusChange(6-20)onRoomStatusChange(22-24)onDiscontinuity(26-39)Sources/AblyChat/SubscriptionStorage.swift (2)
create(16-24)create(57-65)Sources/AblyChat/Room.swift (3)
onDiscontinuity(172-185)onDiscontinuity(188-190)onDiscontinuity(397-400)Tests/AblyChatTests/DefaultRoomTests.swift (1)
onDiscontinuity(296-318)
Example/AblyChatExample/Mocks/MockClients.swift (12)
Sources/AblyChat/Connection.swift (2)
onStatusChange(43-57)onStatusChange(60-62)Sources/AblyChat/DefaultConnection.swift (1)
onStatusChange(23-82)Sources/AblyChat/Room.swift (6)
onStatusChange(143-157)onStatusChange(160-162)onStatusChange(386-389)onDiscontinuity(172-185)onDiscontinuity(188-190)onDiscontinuity(397-400)Sources/AblyChat/RoomLifecycleManager.swift (1)
onDiscontinuity(226-229)Sources/AblyChat/DefaultMessages.swift (2)
subscribe(39-115)history(118-124)Sources/AblyChat/DefaultOccupancy.swift (1)
subscribe(20-58)Sources/AblyChat/DefaultPresence.swift (2)
subscribe(202-222)subscribe(224-247)Sources/AblyChat/DefaultRoomReactions.swift (1)
subscribe(36-79)Sources/AblyChat/DefaultTyping.swift (1)
subscribe(37-108)Sources/AblyChat/Messages.swift (2)
subscribe(95-116)subscribe(119-121)Sources/AblyChat/DefaultMessageReactions.swift (1)
subscribeRaw(103-149)Sources/AblyChat/MessageReactions.swift (2)
subscribeRaw(96-110)subscribeRaw(113-115)
Sources/AblyChat/DefaultPresence.swift (2)
Sources/AblyChat/Presence.swift (4)
subscribe(139-153)subscribe(166-180)subscribe(183-185)subscribe(188-190)Sources/AblyChat/Subscription.swift (2)
unsubscribe(62-64)unsubscribe(89-91)
Sources/AblyChat/DefaultRoomReactions.swift (5)
Example/AblyChatExample/Mocks/MockClients.swift (7)
subscribe(127-155)subscribe(280-304)subscribe(335-352)subscribe(366-382)subscribe(534-536)subscribe(538-540)subscribe(554-565)Sources/AblyChat/DefaultOccupancy.swift (1)
subscribe(20-58)Sources/AblyChat/DefaultPresence.swift (2)
subscribe(202-222)subscribe(224-247)Sources/AblyChat/DefaultTyping.swift (1)
subscribe(37-108)Sources/AblyChat/RoomReactions.swift (2)
subscribe(44-58)subscribe(61-63)
Sources/AblyChat/SubscriptionStorage.swift (1)
Example/AblyChatExample/Mocks/MockSubscriptionStorage.swift (3)
create(38-49)create(96-107)create(153-170)
Example/AblyChatExample/Mocks/MockSubscriptionStorage.swift (1)
Sources/AblyChat/Subscription.swift (1)
historyBeforeSubscribe(93-108)
Sources/AblyChat/Subscription.swift (1)
Example/AblyChatExample/Mocks/MockSubscriptionStorage.swift (1)
historyBeforeSubscribe(211-213)
Sources/AblyChat/Room.swift (7)
Example/AblyChatExample/Mocks/MockClients.swift (3)
onStatusChange(90-105)onStatusChange(587-600)onDiscontinuity(107-110)Sources/AblyChat/Connection.swift (2)
onStatusChange(43-57)onStatusChange(60-62)Sources/AblyChat/DefaultConnection.swift (1)
onStatusChange(23-82)Tests/AblyChatTests/Mocks/MockRoom.swift (2)
onStatusChange(67-70)onDiscontinuity(72-75)Tests/AblyChatTests/DefaultRoomTests.swift (2)
onStatusChange(269-291)onDiscontinuity(296-318)Sources/AblyChat/RoomLifecycleManager.swift (1)
onDiscontinuity(226-229)Tests/AblyChatTests/Mocks/MockRoomLifecycleManager.swift (1)
onDiscontinuity(82-85)
Sources/AblyChat/DefaultMessageReactions.swift (6)
Sources/AblyChat/DefaultOccupancy.swift (1)
subscribe(20-58)Sources/AblyChat/DefaultPresence.swift (2)
subscribe(202-222)subscribe(224-247)Sources/AblyChat/DefaultRoomReactions.swift (1)
subscribe(36-79)Sources/AblyChat/MessageReactions.swift (4)
subscribe(67-81)subscribe(84-86)subscribeRaw(96-110)subscribeRaw(113-115)Sources/AblyChat/Subscription.swift (2)
unsubscribe(62-64)unsubscribe(89-91)Sources/AblyChat/AblyCocoaExtensions/InternalAblyCocoaTypes.swift (3)
unsubscribe(212-214)unsubscribe(316-318)unsubscribe(430-432)
Sources/AblyChat/DefaultTyping.swift (2)
Example/AblyChatExample/Mocks/MockClients.swift (7)
subscribe(127-155)subscribe(280-304)subscribe(335-352)subscribe(366-382)subscribe(534-536)subscribe(538-540)subscribe(554-565)Sources/AblyChat/Typing.swift (2)
subscribe(63-77)subscribe(80-82)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: Xcode, iOS (Xcode 26.0)
- GitHub Check: Xcode, tvOS (Xcode 26.0)
- GitHub Check: SPM,
releaseconfiguration (Xcode 26.0) - GitHub Check: Example app, macOS (Xcode 26.0)
- GitHub Check: Example app, tvOS (Xcode 26.0)
- GitHub Check: Xcode,
releaseconfiguration, tvOS (Xcode 26.0) - GitHub Check: Xcode,
releaseconfiguration, iOS (Xcode 26.0) - GitHub Check: Example app, iOS (Xcode 26.0)
- GitHub Check: SPM (Xcode 26.0)
- GitHub Check: Xcode,
releaseconfiguration, macOS (Xcode 26.0)
🔇 Additional comments (47)
Sources/AblyChat/DefaultMessageReactions.swift (2)
63-100: LGTM on returning a concrete DefaultSubscription.Removes the existential and keeps unsubscribe symmetrical.
146-148: LGTM on unsubscribe symmetry.Sources/AblyChat/PaginatedResult.swift (3)
11-13: Good: navigation returns Self, eliminating public existentials.This aligns with the PR goal.
65-98: Wrapper conforms correctly to Self-returning API.The Self-typed next/first/current are coherent.
1-14: No remainingany PaginatedResultexistentials
Search returned no matches.Sources/AblyChat/Messages.swift (2)
95-121: AsyncSequence convenience correctly tracks SubscribeResponse.HistoryResult.Unsubscribe on termination and event piping look sound.
338-371: Generic MessageSubscriptionAsyncSequence wiring is consistent.getPreviousMessages closure typing and public API match the new associated types.
Example/AblyChatExample/Mocks/MockSubscriptionStorage.swift (6)
9-9: Concrete MockSubscription in storage — LGTMSwitching from protocol existential to concrete mock reduces dynamic dispatch and clarifies mock wiring.
42-42: Factory now returns MockSubscription — LGTMConsistent with the storage item type and eases use in tests.
67-67: Concrete MockStatusSubscription — LGTMMatches the existential reduction strategy across mocks.
100-100: Status factory return type updated — LGTMReturn type aligns with stored mock and usage.
122-131: Generic Message subscription storage — LGTMPropagating PaginatedResult as a generic greatly clarifies types and mirrors the production flow. Returning
some MessageSubscriptionResponseProtocolkeeps call sites stable.Also applies to: 155-159
207-221: MockMessageSubscriptionResponse generic result — LGTMGeneric
PaginatedResultplumbs through correctly;historyBeforeSubscribereturns the concrete associated type expected by the async-sequence wrapper.Sources/AblyChat/ChatAPI.swift (2)
19-22: Opaque getMessages return — LGTMReturning
some PaginatedResult<Message>removes existential use while keeping call sites unchanged.
245-252: Opaque makePaginatedRequest return — LGTMThe opaque return paired with
toPaginatedResultis appropriate; generic constraints ensure Sendable/Equatable safety.Sources/AblyChat/ChatClient.swift (2)
6-6: Introduce Connection associated type and nonisolated accessor — LGTMThis removes the public existential and aligns with the broader associated-type pattern.
Also applies to: 22-22
108-109: Wire DefaultConnection into client — LGTMConcrete storage with an opaque public accessor preserves API while removing existentials internally.
Sources/AblyChat/Connection.swift (1)
8-8: Associated StatusSubscription and concrete return — LGTMEliminates the existential in
onStatusChange. The async-sequence helper remains valid viaoff()on the associated type.Also applies to: 30-30
Sources/AblyChat/RoomReactions.swift (1)
10-10: Associated Subscription and typed subscribe — LGTMRemoves existential return from the public API and keeps the async-sequence adapter intact.
Also applies to: 31-31
Example/AblyChatExample/Mocks/Misc.swift (1)
33-33: Self-referential PaginatedResult properties — LGTMAligned with the new Self-based PaginatedResult. No issues.
Also applies to: 35-35, 37-37
Tests/AblyChatTests/MessageSubscriptionAsyncSequenceTests.swift (1)
13-17: Tests updated for self-referential PaginatedResult and explicit generic — LGTMMocks and instantiation align with the new typing.
Also applies to: 36-36, 42-42
Sources/AblyChat/DefaultRoomReactions.swift (1)
37-37: Opaque Subscription return — LGTMSwitch to returning DefaultSubscription via some SubscriptionProtocol is consistent and safe.
Also applies to: 76-76
Sources/AblyChat/DefaultTyping.swift (1)
38-38: Opaque Subscription return — LGTMReturn type updated and unsubscribe handled correctly.
Also applies to: 100-100
Sources/AblyChat/DefaultPresence.swift (1)
202-202: Concrete DefaultSubscription returns — LGTMGood move away from existentials; unsubscribe closures are safe.
Also applies to: 217-217, 224-224, 240-240
Sources/AblyChat/DefaultConnection.swift (1)
24-24: Opaque StatusSubscription return — LGTMExistential removed; unsubscribe closure handles both timer and listener cleanly.
Also applies to: 75-75
Sources/AblyChat/Occupancy.swift (1)
11-11: [scratchpad]
[tasks_1/2 verify_conformers]
[observations]
- Protocol
Occupancydefinesassociatedtype Subscription: SubscriptionProtocol.DefaultOccupancy: Occupancyimplementsfunc subscribe(...) -> some SubscriptionProtocolwithout explicittypealias Subscription.MockOccupancy: Occupancyexists but no evidence oftypealias Subscription.
[analysis]- Conformers must satisfy
Subscriptionassociatedtype: returningsome SubscriptionProtocolimplicitly infers associatedtype. Swift inference supports opaque return types for associatedtype conformance.- Both
DefaultOccupancyandMockOccupancyuse opaque (some SubscriptionProtocol) andMockSubscriptionrespectively, satisfying requirement.
[pending]- Confirm no missing conformers for
Occupancy.- Check if any conformer fails to implement required
func subscribe(_:) -> Subscription.
[actions]- Run search for
DefaultOccupancydefinition to inspect conformance.- Run search for
MockOccupancydefinition to inspect conformance.
[done]
[/scratchpad]All Occupancy conformers return a consistent concrete
Subscriptiontype viasome SubscriptionProtocolor a specificMockSubscription, satisfying the associatedtype requirement.Sources/AblyChat/Subscription.swift (2)
37-38: HistoryResult associated type — LGTM Swift tools version 6.2 supports opaque types satisfying associated type requirements.
93-108: Opaque history return in concrete type — OK; aligns with protocol associated type
Ensure minimum Swift version supports opaque return types (some).Sources/AblyChat/Presence.swift (1)
86-100: Subscribe API return type change looks good; AsyncSequence adapter correctly unsubscribes.Signatures now return the associated
Subscriptionand the adapters callunsubscribe()on termination. Aligns with PR goals.Sources/AblyChat/RoomLifecycleManager.swift (2)
5-5: Associated StatusSubscription is appropriate for internal API.Conforms to the PR’s direction; keeps existentials out of returns while allowing internal existential use where needed.
12-12: Return types switched to concrete/associated subscription; LGTM.
onRoomStatusChange/onDiscontinuitynow returnStatusSubscription/DefaultStatusSubscription. AsyncSequence helpers upstream correctly calloff().Also applies to: 27-27, 183-183, 227-227
Example/AblyChatExample/Mocks/MockClients.swift (9)
12-12: Switch to MockConnection is fine in examples.Assuming
ChatClient.connectionsurface accepts concrete/opaque connection. No issues spotted here.
56-60: Concrete mock component properties align with associated type changes.Using
Mock*concretes avoids existential usage in examples. Looks good.
91-105: Status subscription mock returns DefaultStatusSubscription; good parity with production.Periodic emitter + proper cancellation closure. LGTM.
127-155: Messages.subscribe mock updated to opaque response; state tracking changes LGTM.Reactions bookkeeping updates (Lines 144,146) and
some MessageSubscriptionResponseProtocolreturn are consistent with new API.
157-159: Opaque PaginatedResult return in history mock: good.
280-304: MessageReactions.subscribe mock returning MockSubscription matches protocol constraints.Fits the new
associatedtype Subscription. LGTM.
534-541: Presence.subscribe mocks returning MockSubscription align with new Presence PAT.Consistent with
MockSubscriptionStorage. LGTM.
555-565: Occupancy.subscribe mock now returns MockSubscription; consistent with API.No issues.
588-600: Connection.onStatusChange mock uses opaque status subscription; good.Matches updated Connection surface across the repo.
Sources/AblyChat/MessageReactions.swift (2)
41-41: Subscribe returns associated Subscription; AsyncSequence adapters correctly unsubscribe.Matches implementations (DefaultMessageReactions). Good.
Also applies to: 54-54
10-11: Reactions exposures use associated types
Room.reactionsandMessages.reactionsreturn generic associated types rather thanany MessageReactions, so no changes needed.Tests/AblyChatTests/Mocks/MockRoomLifecycleManager.swift (1)
78-80: Mocks updated to return DefaultStatusSubscription; matches production API.Simple, correct change.
Also applies to: 83-85
Sources/AblyChat/DefaultMessages.swift (1)
118-124: Opaquesome PaginatedResultpassthrough.Forwarding an opaque result is fine as long as
chatAPI.getMessagesconsistently returns the same concrete type. Please confirm it does.Tests/AblyChatTests/Mocks/MockRoom.swift (1)
19-37: Mocks aligned with concrete Default types and subscriptions.*Switch to
DefaultMessages/Presence/Reactions/Typing/Occupancyand returningDefaultStatusSubscriptionmatches the new API. LGTM.Also applies to: 67-75
Sources/AblyChat/Room.swift (2)
10-17: Associated types migration looks consistent.Public
Roomnow uses associated types and returnsStatusSubscriptionfor status APIs. Extension helpers still useoff()correctly. Looks good.Confirm
StatusSubscriptionProtocolexposesoff()(and not e.g.unsubscribe()), since both styles exist elsewhere.Also applies to: 30-67, 83-96
229-238: DefaultRoom generic + concrete Default composition reads well.*Factory return type and stored properties align with the associated-type protocol. Status methods returning
LifecycleManager.StatusSubscriptionare coherent.Also applies to: 241-256, 271-330, 386-400
lawrence-forooghian
force-pushed
the
2025-10-03-enable-upcoming-language-features
branch
from
f59a5b1 to
a43b818
Compare
This was started in f44e533; here we address the remainder. The only remaining usage is in ChatClientOptions.logHandler, to avoid making the options type generic. (There's still a fair bit of usage of existential types internally but we don't need to sort that out pre-v1 release.)
lawrence-forooghian
force-pushed
the
remove-existential-from-public-api
branch
from
7ac378d to
5009845
Compare
lawrence-forooghian
changed the title
2 participants
Note: This PR is based on top of #363; please review that one first.
This was started in #362; here we address the remainder.
The only remaining usage is in
ChatClientOptions.logHandler, to avoid making the options type generic.(There's still a fair bit of usage of existential types internally but we don't need to sort that out pre-v1 release.)
Summary by CodeRabbit
Refactor
Documentation
Tests