feat(react): return history function from useChannel hook#2177
Merged
owenpearson merged 2 commits intomainfrom Mar 12, 2026
Merged
feat(react): return history function from useChannel hook#2177owenpearson merged 2 commits intomainfrom
history function from useChannel hook#2177owenpearson merged 2 commits intomainfrom
Conversation
WalkthroughAdds a history API surface to the useChannel hook: mock Ably connections expose a history method, the hook returns a history function that delegates to channel.history, and test coverage is added to validate behavior and stability. Changes
Sequence Diagram(s)sequenceDiagram
participant Consumer
participant useChannelHook as "useChannel Hook"
participant Channel as "Ably Channel"
participant AblyMock as "Ably Mock Connection"
Consumer->>useChannelHook: call history(params)
useChannelHook->>Channel: channel.history(params)
Channel->>AblyMock: delegate history call
AblyMock-->>Channel: return { items, hasNext, isLast, first, current, next }
Channel-->>useChannelHook: return paginated result
useChannelHook-->>Consumer: resolved history result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
github-actions
bot
temporarily deployed
to
staging/pull/2177/bundle-report
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
src/platform/react-hooks/src/hooks/useChannel.test.tsx (1)
155-172: Consider wrapping the asynchistory()call inact()for consistency.The
history()call on line 170 is an async operation that may trigger state updates but isn't wrapped inact(). While this may work in practice since it's just reading data from the mock, wrapping async operations inact()is a React Testing Library best practice to ensure all updates are processed.♻️ Suggested improvement
- const paginatedResult = await result.current.history(); + let paginatedResult; + await act(async () => { + paginatedResult = await result.current.history(); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/platform/react-hooks/src/hooks/useChannel.test.tsx` around lines 155 - 172, The test calls result.current.history() which is an async operation that can trigger state updates but isn't wrapped in act(); update the test so the history() call is wrapped inside an await act(async () => { ... }) block (similar to how the otherClient.publish calls are wrapped) to ensure React processes all updates — modify the it('history returns previously published messages'...) test to call await act(async () => { const paginatedResult = await result.current.history(); expect(paginatedResult.items.length).toBe(2); }); while keeping the existing renderHook wrapper (AblyProvider/ChannelProvider) and otherClient.publish steps intact.src/platform/react-hooks/src/fakes/ably.ts (2)
218-227: Duplicatehistoryimplementation - consider extracting to a shared helper.This
historymethod is identical to the one inClientSingleChannelConnection(lines 167-176). Consider extracting the pagination result creation into a shared helper function to reduce duplication.♻️ Suggested refactor to reduce duplication
// Add a helper function at file level function createPaginatedHistoryResult(channel: Channel) { const items = channel.publishedMessages.map((m: any) => m.messageEnvelope); const result = { items, hasNext: () => false, isLast: () => true, first: () => Promise.resolve(createPaginatedHistoryResult(channel)), current: () => Promise.resolve(createPaginatedHistoryResult(channel)), next: () => Promise.resolve(null), }; return result; } // Then in both classes: public async history(_params?: any) { return createPaginatedHistoryResult(this.channel); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/platform/react-hooks/src/fakes/ably.ts` around lines 218 - 227, Both classes implement an identical async history method; extract the pagination-result construction into a shared helper (e.g., createPaginatedHistoryResult) to avoid duplication. Implement a file-level function createPaginatedHistoryResult(channel) that builds the object using channel.publishedMessages.map(m => m.messageEnvelope) and the hasNext/isLast/first/current/next behaviors, then replace the history implementations in both ClientSingleChannelConnection and the other class to simply return createPaginatedHistoryResult(this.channel). Ensure the helper returns the same Promise-returning first/current references to itself and next resolves to null.
167-176: MockmessageEnvelopeshape doesn't matchAbly.InboundMessagetype.The
history()method returns items mapped frommessageEnvelopewhich only contains{ data: message }(see line 322-324). However,Ably.RealtimeChannel['history']returnsPaginatedResult<InboundMessage>, andInboundMessagerequiresid,timestamp,action,version, andannotationsas non-optional fields.While this works for the current tests (which only access
.data.text), it may cause type mismatches or test failures if future tests rely on other message properties.♻️ Consider enriching the messageEnvelope structure
In the
Channel.publishmethod around line 322:const messageEnvelope = { data: message, + id: `${Date.now()}-${Math.random().toString(36).substr(2, 9)}`, + timestamp: Date.now(), + action: 1, // MESSAGE_CREATE + version: '1.0', + annotations: {}, };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/platform/react-hooks/src/fakes/ably.ts` around lines 167 - 176, The mock history() returns items from channel.publishedMessages.map(m => m.messageEnvelope) but messageEnvelope is too minimal; update Channel.publish (where publishedMessages are pushed) to construct messageEnvelope objects that match Ably.InboundMessage by including id, timestamp, action, version, annotations (and existing data) so history() returns PaginatedResult<InboundMessage>-shaped items; ensure the shape is applied to channel.publishedMessages and referenced by history(), and keep values sensible (e.g., generate unique id, ISO timestamp, numeric action/version, and empty annotations) to satisfy typings and future tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/platform/react-hooks/src/fakes/ably.ts`:
- Around line 218-227: Both classes implement an identical async history method;
extract the pagination-result construction into a shared helper (e.g.,
createPaginatedHistoryResult) to avoid duplication. Implement a file-level
function createPaginatedHistoryResult(channel) that builds the object using
channel.publishedMessages.map(m => m.messageEnvelope) and the
hasNext/isLast/first/current/next behaviors, then replace the history
implementations in both ClientSingleChannelConnection and the other class to
simply return createPaginatedHistoryResult(this.channel). Ensure the helper
returns the same Promise-returning first/current references to itself and next
resolves to null.
- Around line 167-176: The mock history() returns items from
channel.publishedMessages.map(m => m.messageEnvelope) but messageEnvelope is too
minimal; update Channel.publish (where publishedMessages are pushed) to
construct messageEnvelope objects that match Ably.InboundMessage by including
id, timestamp, action, version, annotations (and existing data) so history()
returns PaginatedResult<InboundMessage>-shaped items; ensure the shape is
applied to channel.publishedMessages and referenced by history(), and keep
values sensible (e.g., generate unique id, ISO timestamp, numeric
action/version, and empty annotations) to satisfy typings and future tests.
In `@src/platform/react-hooks/src/hooks/useChannel.test.tsx`:
- Around line 155-172: The test calls result.current.history() which is an async
operation that can trigger state updates but isn't wrapped in act(); update the
test so the history() call is wrapped inside an await act(async () => { ... })
block (similar to how the otherClient.publish calls are wrapped) to ensure React
processes all updates — modify the it('history returns previously published
messages'...) test to call await act(async () => { const paginatedResult = await
result.current.history(); expect(paginatedResult.items.length).toBe(2); });
while keeping the existing renderHook wrapper (AblyProvider/ChannelProvider) and
otherClient.publish steps intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a8168125-8218-4345-9593-4ef65135047c
📒 Files selected for processing (3)
src/platform/react-hooks/src/fakes/ably.tssrc/platform/react-hooks/src/hooks/useChannel.test.tsxsrc/platform/react-hooks/src/hooks/useChannel.ts
VeskeR
reviewed
Mar 12, 2026
Contributor
VeskeR
left a comment
VeskeR
left a comment
There was a problem hiding this comment.
LGTM, I'd only suggest adding at least a test for history with derived channels ('useChannel with deriveOptions') for peace of mind that nothing funny is going on there
github-actions
bot
temporarily deployed
to
staging/pull/2177/bundle-report
owenpearson
force-pushed
the
use-channel-history
branch
from
0b5be0d to
bc15069
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/platform/react-hooks/src/fakes/ably.ts (1)
167-176: Consider extracting duplicated history implementation.The
history()method implementation is identical in bothClientSingleChannelConnectionandClientSingleDerivedChannelConnection. While this is acceptable for a mock, you could reduce duplication by extracting a shared helper or using inheritance.♻️ Optional: Extract shared helper
// Add helper function at module level function createPaginatedResult(messages: any[]) { const result = { items: messages.map((m: any) => m.messageEnvelope), hasNext: () => false, isLast: () => true, first: () => Promise.resolve(result), current: () => Promise.resolve(result), next: () => Promise.resolve(null), }; return result; } // Then in both classes: public async history(_params?: any) { return createPaginatedResult(this.channel.publishedMessages); }Also applies to: 218-227
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/platform/react-hooks/src/fakes/ably.ts` around lines 167 - 176, The history() implementation is duplicated in ClientSingleChannelConnection and ClientSingleDerivedChannelConnection; extract a shared helper (e.g., createPaginatedResult) at module scope that accepts messages (this.channel.publishedMessages) and returns the paginated object used by history(), then have both classes' history() methods return that helper result (or alternatively move history() into a common base class both clients extend) so the mapping to messageEnvelope and the promise callbacks are defined once.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/platform/react-hooks/src/fakes/ably.ts`:
- Around line 167-176: The history() implementation is duplicated in
ClientSingleChannelConnection and ClientSingleDerivedChannelConnection; extract
a shared helper (e.g., createPaginatedResult) at module scope that accepts
messages (this.channel.publishedMessages) and returns the paginated object used
by history(), then have both classes' history() methods return that helper
result (or alternatively move history() into a common base class both clients
extend) so the mapping to messageEnvelope and the promise callbacks are defined
once.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1392cd03-ae9a-4da2-a437-88a28a6e23d8
📒 Files selected for processing (3)
src/platform/react-hooks/src/fakes/ably.tssrc/platform/react-hooks/src/hooks/useChannel.test.tsxsrc/platform/react-hooks/src/hooks/useChannel.ts
VeskeR
approved these changes
Mar 12, 2026
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
historyfunction returned fromuseChannelas specified by AITDR-004Summary by CodeRabbit
New Features
Tests