One contract per layer, or keep ISocket and IDriver

Open
  • Is something wrong?
#239 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
5/5
Estimated time
Over a week
Newbie friendliness
25/100
Issue type
Refactor
Clarity
Needs clarification
Activity status
Quiet
Tech stack
typescript
Domain
backend-api-design

Research direction

Start by comparing ISocket, IDriver, DDPDriver, and the RocketChatClient.socket call sites, noting where casts and delegated implementations occur. Read lib/clients/Livechat.ts and the commented Protocols.MQTT import to understand the remaining protocol path. Done means the layering and contract strategy are decided, affected interfaces and callers are consistently updated, and the MQTT fate is resolved.

Written by the indexing model from the issue text.

Description

wayfinder:grilling

Question

Are ISocket and IDriver the right seam?

Evidence they are not:

  • DDPDriver implements both, so at runtime there is exactly one object behind two contracts.
  • The split is arbitrary: subscribe sits on ISocket but subscribeRoom on IDriver; login is on IDriver though Socket implements it; IDriver.config is any; subscribeNotifyUser is declared twice with different return types.
  • The clients implement ISocket only to delegate — that is a facade, not a transport contract. RocketChatClient.socket is typed as the union and nearly every call site casts to pick a half.
  • Protocols.MQTT has no driver and the import is commented out in lib/clients/Livechat.ts. That dead branch was the only justification for protocol pluggability.

The layering the code actually has is three: the DDP engine (frames, send, open/close, heartbeat), the driver (Rocket.Chat streams, method calls, login), and the client facade (REST plus realtime).

Decide whether to collapse to one contract per layer, whether the client facade is derived from the driver contract or hand-maintained, and whether Protocols.MQTT dies with the split.

Dominant language
TypeScript
Stars
146
Forks
101
No merged PRs in 30d

Getting set up

This project ships no dev container, Dockerfile or contributing guide, so setting up is up to you: start from its README, and see our first-contribution guide for the general steps.

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from RocketChat/Rocket.Chat.js.SDK

All issues in RocketChat/Rocket.Chat.js.SDK

Similar issues

More TypeScript issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.