Fix @signalwire/client TS types #1259

jpsantosbh · 2025-07-18T12:35:06Z

Description

The usage of Brand Types was causing issues to use the SDK. Plus the solution only "renamed" the declared type, all other types used by the inner properties continue to have Room and Fabric references.

This PR replaces the Brand and Prettify types with Compute from the ts-toolbelt library, fixing the issue

Type of change

Internal refactoring
Bug fix (bugfix - non-breaking)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)

Code snippets

In case of new feature or breaking changes, please include code snippets.

- Created typeUtils.ts with ShallowCompute utility for types that need flat computation - Applied ShallowCompute to complex types with generics and function signatures - Kept deep computation for simpler types - Build now passes successfully without TypeScript errors Types using ShallowCompute: - EmitterContract and other contract types - PaginatedResponse/Result generics - Callback and handler types - SignalWireClient and SignalWireContract This approach prevents TypeScript from hitting complexity limits while maintaining type safety through selective computation strategies. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>

changeset-bot · 2025-07-18T12:35:09Z

⚠️ No Changeset found

Latest commit: f113e96

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

packages/client/src/fabric/workers/callSegmentWorker.ts

Copilot

Pull Request Overview

This PR fixes TypeScript type issues in the @signalwire/client SDK by replacing Brand and Prettify types with Compute from the ts-toolbelt library. The change resolves SDK usage issues while providing better type computation for complex generic types.

Reviewed Changes

Copilot reviewed 7 out of 8 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
packages/client/src/utils/typeUtils.ts	Introduces new type utilities using ts-toolbelt's Compute with different computation depths
packages/client/src/utils/interfaces/fabric.ts	Replaces Brand types with DeepCompute/ShallowCompute wrappers
packages/client/src/index.ts	Refactors type exports from Prettify to Compute-based approach with aliased imports
packages/client/src/fabric/workers/fabricWorker.ts	Updates worker types to use ShallowCompute and adds type assertions
packages/client/src/fabric/workers/callSegmentWorker.ts	Adds type assertions for MapToPubSubShape compatibility
packages/client/snippets/CLIENT_SDK_OVERVIEW.md	Adds comprehensive SDK documentation
packages/client/package.json	Adds ts-toolbelt dependency

packages/client/src/utils/typeUtils.ts

Copilot · 2025-07-18T14:05:33Z

packages/client/src/fabric/workers/callSegmentWorker.ts

+          action: action as unknown as MapToPubSubShape<
+            CallMemberJoinedEvent | CallMemberLeftEvent
+          >,
        })
-        const videoAction =
-          mapFabricMemberActionToVideoMemberJoinAndLeftAction(action)
+        const videoAction = mapFabricMemberActionToVideoMemberJoinAndLeftAction(
+          // TS is complaining {[x: string]: {}} in assignable to Record<string, unknown>
+          action as unknown as MapToPubSubShape<
+            CallMemberJoinedEvent | CallMemberLeftEvent
+          >


The type assertion with 'unknown' and the comment suggest a deeper type compatibility issue. Consider creating a proper type mapping function or updating the MapToPubSubShape type definition to handle these cases more elegantly.

The problem is how Compute resolve Record<string, unknown> to {[x: string], {}}
{} is not equal to unknown, but is safe to cast {} to unknown

packages/client/src/fabric/workers/callSegmentWorker.ts

packages/client/src/index.ts

Co-authored-by: Copilot <[email protected]>

iAmmar7 · 2025-07-21T13:54:49Z

packages/client/package.json

@@ -41,7 +41,8 @@
  "dependencies": {
    "@signalwire/core": "4.3.0",
    "@signalwire/webrtc": "3.14.0",
-    "jwt-decode": "^3.1.2"
+    "jwt-decode": "^3.1.2",
+    "ts-toolbelt": "^9.6.0"


Can we please not do this? We always try not to install any packages on the SDK.

Also, even with this lib addition, I see in this PR copilot has highlighted issues with the TS usage. Maybe we need to stop adding these quick fixes and think hard and understand the root cause?

We can take @LittorWired's help as well and understand the TS problems that you have been facing.

This was from a @LittorWired suggestion already :-)
#1257 (comment)

Since we're heavily relying on Prettify/Compute on this SDK package, I agree that using a well-tested library is better in this case.

If it solves one problem but causes two new ones, is it really worth it?

Also, the dependency should be a devDependency?

I was trying to point out in that comment that Prettify/Compute might require a more well-battle-tested utility type if we really want to improve the DX without type "bugs".

Maybe the prettify/compute is not needed at the moment, but to be honest I don't yet fully understand the full context around the root of the issue. I gather it is around type aliases?

iAmmar7 · 2025-07-21T13:55:28Z

packages/client/src/fabric/workers/callSegmentWorker.ts

@@ -83,16 +89,25 @@ export const callSegmentWorker = function* (
      case 'member.left': {
        yield sagaEffects.fork(fabricMemberWorker, {
          ...options,
-          action,
+          // TS is complaining {[x: string]: {}} in assignable to Record<string, unknown>


We need to double-check these deeper type compatibility issues. It could happen anywhere in the SDK.

LittorWired · 2025-07-21T14:18:45Z

packages/client/snippets/CLIENT_SDK_OVERVIEW.md

Is this markdown file supposed to be part of this PR?

LittorWired · 2025-07-21T14:24:03Z

packages/client/src/fabric/workers/fabricWorker.ts

              member: action.payload.room_session.members.find(
                (m) => m.member_id === action.payload.member_id
-              )!,
+              )! as InternalCallMemberEntityUpdated,


Should the error be correctly handled if the member is not found?

I believe if (!cfRoomSession.selfMember) covers that already

If that is already covered is it possible then for a simple refactor to remove the ! as InternalCallMemberEntityUpdated type casting to make it safer?

LittorWired · 2025-07-21T14:28:52Z

packages/client/package.json

@@ -41,7 +41,8 @@
  "dependencies": {
    "@signalwire/core": "4.3.0",
    "@signalwire/webrtc": "3.14.0",
-    "jwt-decode": "^3.1.2"
+    "jwt-decode": "^3.1.2",
+    "ts-toolbelt": "^9.6.0"


Should ts-toolbelt be a dev dependency?

@LittorWired actually, it's imported in the code, so it's not just a dev dependency.

OK thanks just wanted to double check

jpsantosbh and others added 4 commits July 17, 2025 14:30

Add ts-toolbelt

6ec6c27

docs

e60a1cf

ts-toolbelt dependency

89966d6

jpsantosbh commented Jul 18, 2025

View reviewed changes

packages/client/src/fabric/workers/callSegmentWorker.ts Outdated Show resolved Hide resolved

jpsantosbh marked this pull request as draft July 18, 2025 12:40

jpsantosbh added 2 commits July 18, 2025 12:58

using ComputeDeep utility

5af7876

better than any

5d43d25

jpsantosbh requested a review from Copilot July 18, 2025 14:04

jpsantosbh marked this pull request as ready for review July 18, 2025 14:05

Copilot AI reviewed Jul 18, 2025

View reviewed changes

Update packages/client/src/utils/typeUtils.ts

f113e96

Co-authored-by: Copilot <[email protected]>

jpsantosbh requested review from iAmmar7, RafaelPChequer and LittorWired July 18, 2025 14:11

iAmmar7 reviewed Jul 21, 2025

View reviewed changes

LittorWired reviewed Jul 21, 2025

View reviewed changes

Fix @signalwire/client TS types #1259

Are you sure you want to change the base?

Fix @signalwire/client TS types #1259

Uh oh!

Conversation

jpsantosbh commented Jul 18, 2025

Description

Type of change

Code snippets

Uh oh!

changeset-bot bot commented Jul 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚠️ No Changeset found

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Copilot AI Jul 18, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

iAmmar7 Jul 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

changeset-bot bot commented Jul 18, 2025 •

edited

Loading

iAmmar7 Jul 21, 2025 •

edited

Loading