refactor: add StateID branded type on state_key from m.room.member #308
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #308 +/- ##
=======================================
Coverage 52.79% 52.79%
=======================================
Files 96 96
Lines 12589 12590 +1
=======================================
+ Hits 6646 6647 +1
Misses 5943 5943 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
WalkthroughThis pull request introduces and applies Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 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 (7)
🧰 Additional context used🧠 Learnings (3)📓 Common learnings📚 Learning: 2025-10-10T22:18:31.655ZApplied to files:
📚 Learning: 2025-09-14T13:34:12.448ZApplied to files:
🧬 Code graph analysis (6)packages/homeserver/src/controllers/internal/invite.controller.ts (1)
packages/homeserver/src/controllers/internal/room.controller.ts (1)
packages/federation-sdk/src/services/invite.service.ts (1)
packages/federation-sdk/src/services/room.service.ts (1)
packages/federation-sdk/src/services/profiles.service.ts (1)
packages/room/src/types/v3-11.ts (1)
🔇 Additional comments (16)
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 review |
|
@ggazzo: I'll review the changes in this PR. 🧠 Learnings used✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Pull request overview
This PR refactors the codebase to introduce a StateID branded type for the state_key field in state events, replacing the previous use of UserID. This change improves type safety by distinguishing between user identifiers and state keys, which can represent different types of values depending on the event type.
Key Changes:
- Moved and reorganized the
stateIdSchemaandStateIDtype definition in_common.tsfor better code organization - Updated
PduNoContentStateEventSchemato usestateIdSchemainstead ofuserIdSchemafor thestate_keyfield - Updated all event creation and comparison logic across controllers and services to properly parse user IDs to state IDs using
stateIdSchema.parse()
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/room/src/types/_common.ts | Reorganized type definitions by moving stateIdSchema and StateID type after userIdSchema |
| packages/room/src/types/v3-11.ts | Updated PduNoContentStateEventSchema to use stateIdSchema for state_key, added import for stateIdSchema |
| packages/homeserver/src/controllers/internal/room.controller.ts | Added stateIdSchema import and updated ban event creation to parse user ID to state ID |
| packages/homeserver/src/controllers/internal/invite.controller.ts | Added stateIdSchema import and updated invite event creation to parse user ID to state ID |
| packages/federation-sdk/src/services/room.service.ts | Added stateIdSchema import and updated multiple membership event creations and comparisons to parse user IDs to state IDs |
| packages/federation-sdk/src/services/profiles.service.ts | Added stateIdSchema import and updated profile join event creation to parse user ID to state ID |
| packages/federation-sdk/src/services/invite.service.ts | Added stateIdSchema import and updated invite event creation to parse user ID to state ID |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| currentUserIds.includes(stateIdSchema.parse(userId1)) && | ||
| currentUserIds.includes(stateIdSchema.parse(userId2)) |
There was a problem hiding this comment.
Calling stateIdSchema.parse() inside the includes() check adds unnecessary parsing overhead for each comparison. Consider parsing userId1 and userId2 to StateID once before the comparison:
const stateId1 = stateIdSchema.parse(userId1);
const stateId2 = stateIdSchema.parse(userId2);
if (
currentUserIds.length === 2 &&
currentUserIds.includes(stateId1) &&
currentUserIds.includes(stateId2)
) {
return roomId;
}| authEventIds, | ||
| 'm.room.member', | ||
| (e) => e.event.state_key === senderId, | ||
| (e) => e.event.state_key === stateIdSchema.parse(senderId), |
There was a problem hiding this comment.
Calling stateIdSchema.parse(senderId) inside the filter callback causes the parsing to be executed for each event during the search, which is inefficient. Consider parsing once before the callback:
const senderStateId = stateIdSchema.parse(senderId);
const memberAuthResult = this.getEventByType(
authEventIds,
'm.room.member',
(e) => e.event.state_key === senderStateId,
);
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.