Skip to content

chore: tighten topic parameter types in request client API - #1694

Open
Olexandr88 wants to merge 1 commit into
RequestNetwork:masterfrom
Olexandr88:chore/ts-deps-sweep
Open

chore: tighten topic parameter types in request client API#1694
Olexandr88 wants to merge 1 commit into
RequestNetwork:masterfrom
Olexandr88:chore/ts-deps-sweep

Conversation

@Olexandr88

@Olexandr88Olexandr88 commented Jan 31, 2026

Copy link
Copy Markdown
Contributor

This improves type safety for (fromTopic) and (fromMultipleTopics) by replacing any with the actual supported input types IdentityTypes.IIdentity | string

Summary by CodeRabbit

  • New Features
    • RequestNetwork methods now accept Identity objects in addition to strings, providing greater flexibility in specifying topics.

✏️ Tip: You can customize this high-level summary in your review settings.

Signed-off-by: Olexandr88 <radole1203@gmail.com>
@coderabbitai

coderabbitaiBot commented Jan 31, 2026

Copy link
Copy Markdown
Contributor

Walkthrough

Type signatures for two public methods in the RequestNetwork class were refined. The fromTopic and fromMultipleTopics methods now accept specifically typed parameters (IdentityTypes.IIdentity | string instead of any) with no functional changes.

Changes

Cohort / File(s)Summary
Type Signature Refinement
packages/request-client.js/src/api/request-network.ts
Updated method signatures for fromTopic() and fromMultipleTopics() to accept explicit union types instead of generic any types, improving type safety for Identity objects and strings.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check nameStatusExplanation
Title check✅ PassedThe title clearly and concisely describes the main change: tightening type signatures for topic parameters in the request client API.
Description check✅ PassedThe description explains the purpose of the change (improving type safety) and identifies which parameters and types are affected, though it is minimal.
Docstring Coverage✅ PassedNo functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@greptile-apps

Copy link
Copy Markdown
Contributor

Greptile Overview

Greptile Summary

Replaced any with IdentityTypes.IIdentity | string for topic parameters in fromTopic and fromMultipleTopics methods.

  • Improved type safety by explicitly defining accepted input types
  • Backward compatible change - all existing usages pass IdentityTypes.IIdentity objects or strings
  • The underlying normalizeKeccak256Hash utility accepts unknown, so both object and string types are supported
  • Internal callers like fromIdentity already pass IdentityTypes.IIdentity, confirming the type correctness

Confidence Score: 5/5

  • This PR is safe to merge with no risks
  • The change only narrows the type from any to the actual supported types without any breaking changes. All existing code already uses these exact types, and the underlying implementation supports both Identity objects and strings through the normalizeKeccak256Hash utility.
  • No files require special attention

Important Files Changed

FilenameOverview
packages/request-client.js/src/api/request-network.tsType parameter tightened from any to `IdentityTypes.IIdentity

Sequence Diagram

sequenceDiagram
participant Client
participant RequestNetwork
participant RequestLogic
participant Utils
participant TransactionManager
Note over Client,TransactionManager: Type Flow: IdentityTypes.IIdentity | string
Client->>RequestNetwork: fromTopic(topic: IdentityTypes.IIdentity | string)
Note over RequestNetwork: topic can be an IIdentity object or string
RequestNetwork->>RequestLogic: getRequestsByTopic(topic: string)
Note over RequestLogic: Expects string parameter
RequestLogic->>Utils: normalizeKeccak256Hash(topic: unknown)
Note over Utils: Accepts any type (unknown)
Utils->>Utils: normalize(data)
Utils->>Utils: keccak256Hash(normalized)
Utils-->>RequestLogic: hashedTopic
RequestLogic->>TransactionManager: getChannelsByTopic(hashedTopic)
TransactionManager-->>RequestLogic: channels
RequestLogic-->>RequestNetwork: requests
RequestNetwork-->>Client: Request[]
Note over Client,TransactionManager: Similar flow for fromMultipleTopics with Array type
Loading

@greptile-appsgreptile-appsBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 file reviewed, no comments

Edit Code Review Agent Settings | Greptile

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@Olexandr88