Replace react-rxjs with observable-hooks

react-rxjs is the library we've been using to connect our React components to view models and consume observables. However, after spending some time with react-rxjs, I feel that it's a very heavy-handed solution. It requires us to sprinkle <Subscribe /> and <RemoveSubscribe /> components all throughout the code, and makes React go through an extra render cycle whenever we mount a component that binds to a view model. What I really want is a lightweight React hook that just gets the current value out of a plain observable, without any extra setup. Luckily the observable-hooks library with its useObservableEagerState hook seems to do just that—and it's more actively maintained, too!
2024-05-16 15:23:10 -04:00
parent 0d485ef97f
commit af0bd795b5
9 changed files with 627 additions and 723 deletions
--- a/src/state/CallViewModel.ts
+++ b/src/state/CallViewModel.ts
@@ -50,7 +50,6 @@ import {
  timer,
  zip,
 } from "rxjs";
-import { StateObservable, state } from "@react-rxjs/core";
 import { logger } from "matrix-js-sdk/src/logger";

 import { ViewModel } from "./ViewModel";
@@ -158,7 +157,7 @@ class UserMedia {
        ? new LocalUserMediaViewModel(id, member, participant, callEncrypted)
        : new RemoteUserMediaViewModel(id, member, participant, callEncrypted);

-    this.speaker = this.vm.speaking.pipeState(
+    this.speaker = this.vm.speaking.pipe(
      // Require 1 s of continuous speaking to become a speaker, and 60 s of
      // continuous silence to stop being considered a speaker
      audit((s) =>
@@ -234,9 +233,9 @@ function findMatrixMember(

 // TODO: Move wayyyy more business logic from the call and lobby views into here
 export class CallViewModel extends ViewModel {
-  private readonly rawRemoteParticipants = state(
-    connectedParticipantsObserver(this.livekitRoom),
-  );
+  private readonly rawRemoteParticipants = connectedParticipantsObserver(
+    this.livekitRoom,
+  ).pipe(shareReplay(1));

  // Lists of participants to "hold" on display, even if LiveKit claims that
  // they've left
@@ -309,64 +308,60 @@ export class CallViewModel extends ViewModel {
      },
    );

-  private readonly mediaItems: StateObservable<MediaItem[]> = state(
-    combineLatest([
-      this.remoteParticipants,
-      observeParticipantMedia(this.livekitRoom.localParticipant),
-    ]).pipe(
-      scan(
-        (
-          prevItems,
-          [remoteParticipants, { participant: localParticipant }],
-        ) => {
-          let allGhosts = true;
+  private readonly mediaItems: Observable<MediaItem[]> = combineLatest([
+    this.remoteParticipants,
+    observeParticipantMedia(this.livekitRoom.localParticipant),
+  ]).pipe(
+    scan(
+      (prevItems, [remoteParticipants, { participant: localParticipant }]) => {
+        let allGhosts = true;

-          const newItems = new Map(
-            function* (this: CallViewModel): Iterable<[string, MediaItem]> {
-              for (const p of [localParticipant, ...remoteParticipants]) {
-                const member = findMatrixMember(this.matrixRoom, p.identity);
-                allGhosts &&= member === undefined;
-                // We always start with a local participant with the empty string as
-                // their ID before we're connected, this is fine and we'll be in
-                // "all ghosts" mode.
-                if (p.identity !== "" && member === undefined) {
-                  logger.warn(
-                    `Ruh, roh! No matrix member found for SFU participant '${p.identity}': creating g-g-g-ghost!`,
-                  );
-                }
-
-                const userMediaId = p.identity;
-                yield [
-                  userMediaId,
-                  prevItems.get(userMediaId) ??
-                    new UserMedia(userMediaId, member, p, this.encrypted),
-                ];
-
-                if (p.isScreenShareEnabled) {
-                  const screenShareId = `${userMediaId}:screen-share`;
-                  yield [
-                    screenShareId,
-                    prevItems.get(screenShareId) ??
-                      new ScreenShare(screenShareId, member, p, this.encrypted),
-                  ];
-                }
+        const newItems = new Map(
+          function* (this: CallViewModel): Iterable<[string, MediaItem]> {
+            for (const p of [localParticipant, ...remoteParticipants]) {
+              const member = findMatrixMember(this.matrixRoom, p.identity);
+              allGhosts &&= member === undefined;
+              // We always start with a local participant with the empty string as
+              // their ID before we're connected, this is fine and we'll be in
+              // "all ghosts" mode.
+              if (p.identity !== "" && member === undefined) {
+                logger.warn(
+                  `Ruh, roh! No matrix member found for SFU participant '${p.identity}': creating g-g-g-ghost!`,
+                );
              }
-            }.bind(this)(),
-          );

-          for (const [id, t] of prevItems) if (!newItems.has(id)) t.destroy();
+              const userMediaId = p.identity;
+              yield [
+                userMediaId,
+                prevItems.get(userMediaId) ??
+                  new UserMedia(userMediaId, member, p, this.encrypted),
+              ];

-          // If every item is a ghost, that probably means we're still connecting
-          // and shouldn't bother showing anything yet
-          return allGhosts ? new Map() : newItems;
-        },
-        new Map<string, MediaItem>(),
-      ),
-      map((ms) => [...ms.values()]),
-      finalizeValue((ts) => {
-        for (const t of ts) t.destroy();
-      }),
+              if (p.isScreenShareEnabled) {
+                const screenShareId = `${userMediaId}:screen-share`;
+                yield [
+                  screenShareId,
+                  prevItems.get(screenShareId) ??
+                    new ScreenShare(screenShareId, member, p, this.encrypted),
+                ];
+              }
+            }
+          }.bind(this)(),
+        );
+
+        for (const [id, t] of prevItems) if (!newItems.has(id)) t.destroy();
+
+        // If every item is a ghost, that probably means we're still connecting
+        // and shouldn't bother showing anything yet
+        return allGhosts ? new Map() : newItems;
+      },
+      new Map<string, MediaItem>(),
    ),
+    map((ms) => [...ms.values()]),
+    finalizeValue((ts) => {
+      for (const t of ts) t.destroy();
+    }),
+    shareReplay(1),
  );

  private readonly userMedia: Observable<UserMedia[]> = this.mediaItems.pipe(
@@ -462,14 +457,15 @@ export class CallViewModel extends ViewModel {
  /**
   * The layout mode of the media tile grid.
   */
-  public readonly gridMode = state(this._gridMode);
+  public readonly gridMode: Observable<GridMode> = this._gridMode;

  public setGridMode(value: GridMode): void {
    this._gridMode.next(value);
  }

-  public readonly layout: StateObservable<Layout> = state(
-    combineLatest([this._gridMode, this.windowMode], (gridMode, windowMode) => {
+  public readonly layout: Observable<Layout> = combineLatest(
+    [this._gridMode, this.windowMode],
+    (gridMode, windowMode) => {
      switch (windowMode) {
        case "full screen":
          throw new Error("unimplemented");
@@ -498,110 +494,109 @@ export class CallViewModel extends ViewModel {
          }
        }
      }
-    }).pipe(switchAll()),
-  );
+    },
+  ).pipe(switchAll(), shareReplay(1));

  /**
   * The media tiles to be displayed in the call view.
   */
  // TODO: Get rid of this field, replacing it with the 'layout' field above
  // which keeps more details of the layout order internal to the view model
-  public readonly tiles: StateObservable<TileDescriptor<MediaViewModel>[]> =
-    state(
-      combineLatest([
-        this.remoteParticipants,
-        observeParticipantMedia(this.livekitRoom.localParticipant),
-      ]).pipe(
-        scan((ts, [remoteParticipants, { participant: localParticipant }]) => {
-          const ps = [localParticipant, ...remoteParticipants];
-          const tilesById = new Map(ts.map((t) => [t.id, t]));
-          const now = Date.now();
-          let allGhosts = true;
+  public readonly tiles: Observable<TileDescriptor<MediaViewModel>[]> =
+    combineLatest([
+      this.remoteParticipants,
+      observeParticipantMedia(this.livekitRoom.localParticipant),
+    ]).pipe(
+      scan((ts, [remoteParticipants, { participant: localParticipant }]) => {
+        const ps = [localParticipant, ...remoteParticipants];
+        const tilesById = new Map(ts.map((t) => [t.id, t]));
+        const now = Date.now();
+        let allGhosts = true;

-          const newTiles = ps.flatMap((p) => {
-            const userMediaId = p.identity;
-            const member = findMatrixMember(this.matrixRoom, userMediaId);
-            allGhosts &&= member === undefined;
-            const spokeRecently =
-              p.lastSpokeAt !== undefined && now - +p.lastSpokeAt <= 10000;
+        const newTiles = ps.flatMap((p) => {
+          const userMediaId = p.identity;
+          const member = findMatrixMember(this.matrixRoom, userMediaId);
+          allGhosts &&= member === undefined;
+          const spokeRecently =
+            p.lastSpokeAt !== undefined && now - +p.lastSpokeAt <= 10000;

-            // We always start with a local participant with the empty string as
-            // their ID before we're connected, this is fine and we'll be in
-            // "all ghosts" mode.
-            if (userMediaId !== "" && member === undefined) {
-              logger.warn(
-                `Ruh, roh! No matrix member found for SFU participant '${userMediaId}': creating g-g-g-ghost!`,
-              );
-            }
+          // We always start with a local participant with the empty string as
+          // their ID before we're connected, this is fine and we'll be in
+          // "all ghosts" mode.
+          if (userMediaId !== "" && member === undefined) {
+            logger.warn(
+              `Ruh, roh! No matrix member found for SFU participant '${userMediaId}': creating g-g-g-ghost!`,
+            );
+          }

-            const userMediaVm =
-              tilesById.get(userMediaId)?.data ??
-              (p instanceof LocalParticipant
-                ? new LocalUserMediaViewModel(
-                    userMediaId,
-                    member,
-                    p,
-                    this.encrypted,
-                  )
-                : new RemoteUserMediaViewModel(
-                    userMediaId,
-                    member,
-                    p,
-                    this.encrypted,
-                  ));
-            tilesById.delete(userMediaId);
-
-            const userMediaTile: TileDescriptor<MediaViewModel> = {
-              id: userMediaId,
-              focused: false,
-              isPresenter: p.isScreenShareEnabled,
-              isSpeaker: (p.isSpeaking || spokeRecently) && !p.isLocal,
-              hasVideo: p.isCameraEnabled,
-              local: p.isLocal,
-              largeBaseSize: false,
-              data: userMediaVm,
-            };
-
-            if (p.isScreenShareEnabled) {
-              const screenShareId = `${userMediaId}:screen-share`;
-              const screenShareVm =
-                tilesById.get(screenShareId)?.data ??
-                new ScreenShareViewModel(
-                  screenShareId,
+          const userMediaVm =
+            tilesById.get(userMediaId)?.data ??
+            (p instanceof LocalParticipant
+              ? new LocalUserMediaViewModel(
+                  userMediaId,
                  member,
                  p,
                  this.encrypted,
-                );
-              tilesById.delete(screenShareId);
+                )
+              : new RemoteUserMediaViewModel(
+                  userMediaId,
+                  member,
+                  p,
+                  this.encrypted,
+                ));
+          tilesById.delete(userMediaId);

-              const screenShareTile: TileDescriptor<MediaViewModel> = {
-                id: screenShareId,
-                focused: true,
-                isPresenter: false,
-                isSpeaker: false,
-                hasVideo: true,
-                local: p.isLocal,
-                largeBaseSize: true,
-                placeNear: userMediaId,
-                data: screenShareVm,
-              };
-              return [userMediaTile, screenShareTile];
-            } else {
-              return [userMediaTile];
-            }
-          });
+          const userMediaTile: TileDescriptor<MediaViewModel> = {
+            id: userMediaId,
+            focused: false,
+            isPresenter: p.isScreenShareEnabled,
+            isSpeaker: (p.isSpeaking || spokeRecently) && !p.isLocal,
+            hasVideo: p.isCameraEnabled,
+            local: p.isLocal,
+            largeBaseSize: false,
+            data: userMediaVm,
+          };

-          // Any tiles left in the map are unused and should be destroyed
-          for (const t of tilesById.values()) t.data.destroy();
+          if (p.isScreenShareEnabled) {
+            const screenShareId = `${userMediaId}:screen-share`;
+            const screenShareVm =
+              tilesById.get(screenShareId)?.data ??
+              new ScreenShareViewModel(
+                screenShareId,
+                member,
+                p,
+                this.encrypted,
+              );
+            tilesById.delete(screenShareId);

-          // If every item is a ghost, that probably means we're still connecting
-          // and shouldn't bother showing anything yet
-          return allGhosts ? [] : newTiles;
-        }, [] as TileDescriptor<MediaViewModel>[]),
-        finalizeValue((ts) => {
-          for (const t of ts) t.data.destroy();
-        }),
-      ),
+            const screenShareTile: TileDescriptor<MediaViewModel> = {
+              id: screenShareId,
+              focused: true,
+              isPresenter: false,
+              isSpeaker: false,
+              hasVideo: true,
+              local: p.isLocal,
+              largeBaseSize: true,
+              placeNear: userMediaId,
+              data: screenShareVm,
+            };
+            return [userMediaTile, screenShareTile];
+          } else {
+            return [userMediaTile];
+          }
+        });
+
+        // Any tiles left in the map are unused and should be destroyed
+        for (const t of tilesById.values()) t.data.destroy();
+
+        // If every item is a ghost, that probably means we're still connecting
+        // and shouldn't bother showing anything yet
+        return allGhosts ? [] : newTiles;
+      }, [] as TileDescriptor<MediaViewModel>[]),
+      finalizeValue((ts) => {
+        for (const t of ts) t.data.destroy();
+      }),
+      shareReplay(1),
    );

  public constructor(
--- a/src/state/MediaViewModel.ts
+++ b/src/state/MediaViewModel.ts
@@ -21,7 +21,6 @@ import {
  observeParticipantEvents,
  observeParticipantMedia,
 } from "@livekit/components-core";
-import { StateObservable, state } from "@react-rxjs/core";
 import {
  LocalParticipant,
  LocalTrack,
@@ -35,12 +34,14 @@ import {
 import { RoomMember, RoomMemberEvent } from "matrix-js-sdk/src/matrix";
 import {
  BehaviorSubject,
+  Observable,
  combineLatest,
  distinctUntilChanged,
  distinctUntilKeyChanged,
  fromEvent,
  map,
  of,
+  shareReplay,
  startWith,
  switchMap,
 } from "rxjs";
@@ -92,16 +93,15 @@ export function useNameData(vm: MediaViewModel): NameData {
 function observeTrackReference(
  participant: Participant,
  source: Track.Source,
-): StateObservable<TrackReferenceOrPlaceholder> {
-  return state(
-    observeParticipantMedia(participant).pipe(
-      map(() => ({
-        participant,
-        publication: participant.getTrackPublication(source),
-        source,
-      })),
-      distinctUntilKeyChanged("publication"),
-    ),
+): Observable<TrackReferenceOrPlaceholder> {
+  return observeParticipantMedia(participant).pipe(
+    map(() => ({
+      participant,
+      publication: participant.getTrackPublication(source),
+      source,
+    })),
+    distinctUntilKeyChanged("publication"),
+    shareReplay(1),
  );
 }

@@ -113,11 +113,11 @@ abstract class BaseMediaViewModel extends ViewModel {
  /**
   * The LiveKit video track for this media.
   */
-  public readonly video: StateObservable<TrackReferenceOrPlaceholder>;
+  public readonly video: Observable<TrackReferenceOrPlaceholder>;
  /**
   * Whether there should be a warning that this media is unencrypted.
   */
-  public readonly unencryptedWarning: StateObservable<boolean>;
+  public readonly unencryptedWarning: Observable<boolean>;

  public constructor(
    /**
@@ -138,15 +138,13 @@ abstract class BaseMediaViewModel extends ViewModel {
    super();
    const audio = observeTrackReference(participant, audioSource);
    this.video = observeTrackReference(participant, videoSource);
-    this.unencryptedWarning = state(
-      combineLatest(
-        [audio, this.video],
-        (a, v) =>
-          callEncrypted &&
-          (a.publication?.isEncrypted === false ||
-            v.publication?.isEncrypted === false),
-      ).pipe(distinctUntilChanged()),
-    );
+    this.unencryptedWarning = combineLatest(
+      [audio, this.video],
+      (a, v) =>
+        callEncrypted &&
+        (a.publication?.isEncrypted === false ||
+          v.publication?.isEncrypted === false),
+    ).pipe(distinctUntilChanged(), shareReplay(1));
  }
 }

@@ -165,27 +163,28 @@ abstract class BaseUserMediaViewModel extends BaseMediaViewModel {
  /**
   * Whether the participant is speaking.
   */
-  public readonly speaking = state(
-    observeParticipantEvents(
-      this.participant,
-      ParticipantEvent.IsSpeakingChanged,
-    ).pipe(map((p) => p.isSpeaking)),
+  public readonly speaking = observeParticipantEvents(
+    this.participant,
+    ParticipantEvent.IsSpeakingChanged,
+  ).pipe(
+    map((p) => p.isSpeaking),
+    shareReplay(1),
  );

  /**
   * Whether this participant is sending audio (i.e. is unmuted on their side).
   */
-  public readonly audioEnabled: StateObservable<boolean>;
+  public readonly audioEnabled: Observable<boolean>;
  /**
   * Whether this participant is sending video.
   */
-  public readonly videoEnabled: StateObservable<boolean>;
+  public readonly videoEnabled: Observable<boolean>;

  private readonly _cropVideo = new BehaviorSubject(true);
  /**
   * Whether the tile video should be contained inside the tile or be cropped to fit.
   */
-  public readonly cropVideo = state(this._cropVideo);
+  public readonly cropVideo: Observable<boolean> = this._cropVideo;

  public constructor(
    id: string,
@@ -202,12 +201,12 @@ abstract class BaseUserMediaViewModel extends BaseMediaViewModel {
      Track.Source.Camera,
    );

-    const media = observeParticipantMedia(participant);
-    this.audioEnabled = state(
-      media.pipe(map((m) => m.microphoneTrack?.isMuted === false)),
+    const media = observeParticipantMedia(participant).pipe(shareReplay(1));
+    this.audioEnabled = media.pipe(
+      map((m) => m.microphoneTrack?.isMuted === false),
    );
-    this.videoEnabled = state(
-      media.pipe(map((m) => m.cameraTrack?.isMuted === false)),
+    this.videoEnabled = media.pipe(
+      map((m) => m.cameraTrack?.isMuted === false),
    );
  }

@@ -223,19 +222,18 @@ export class LocalUserMediaViewModel extends BaseUserMediaViewModel {
  /**
   * Whether the video should be mirrored.
   */
-  public readonly mirror = state(
-    this.video.pipe(
-      switchMap((v) => {
-        const track = v.publication?.track;
-        if (!(track instanceof LocalTrack)) return of(false);
-        // Watch for track restarts, because they indicate a camera switch
-        return fromEvent(track, TrackEvent.Restarted).pipe(
-          startWith(null),
-          // Mirror only front-facing cameras (those that face the user)
-          map(() => facingModeFromLocalTrack(track).facingMode === "user"),
-        );
-      }),
-    ),
+  public readonly mirror = this.video.pipe(
+    switchMap((v) => {
+      const track = v.publication?.track;
+      if (!(track instanceof LocalTrack)) return of(false);
+      // Watch for track restarts, because they indicate a camera switch
+      return fromEvent(track, TrackEvent.Restarted).pipe(
+        startWith(null),
+        // Mirror only front-facing cameras (those that face the user)
+        map(() => facingModeFromLocalTrack(track).facingMode === "user"),
+      );
+    }),
+    shareReplay(1),
  );

  /**
@@ -263,14 +261,14 @@ export class RemoteUserMediaViewModel extends BaseUserMediaViewModel {
  /**
   * Whether we've disabled this participant's audio.
   */
-  public readonly locallyMuted = state(this._locallyMuted);
+  public readonly locallyMuted: Observable<boolean> = this._locallyMuted;

  private readonly _localVolume = new BehaviorSubject(1);
  /**
   * The volume to which we've set this participant's audio, as a scalar
   * multiplier.
   */
-  public readonly localVolume = state(this._localVolume);
+  public readonly localVolume: Observable<number> = this._localVolume;

  public constructor(
    id: string,
--- a/src/state/subscribe.tsx
+++ b/src/state/subscribe.tsx
@@ -1,49 +0,0 @@
-/*
-Copyright 2023 New Vector Ltd
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-    http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
-*/
-
-import {
-  ForwardRefExoticComponent,
-  ForwardRefRenderFunction,
-  PropsWithoutRef,
-  RefAttributes,
-  forwardRef,
-} from "react";
-// eslint-disable-next-line no-restricted-imports
-import { Subscribe, RemoveSubscribe } from "@react-rxjs/core";
-
-/**
- * Wraps a React component that consumes Observables, resulting in a component
- * that safely subscribes to its Observables before rendering. The component
- * will return null until the subscriptions are created.
- */
-export function subscribe<P, R>(
-  render: ForwardRefRenderFunction<R, P>,
-): ForwardRefExoticComponent<PropsWithoutRef<P> & RefAttributes<R>> {
-  const Subscriber = forwardRef<R, { p: P }>(({ p }, ref) => (
-    <RemoveSubscribe>{render(p, ref)}</RemoveSubscribe>
-  ));
-  Subscriber.displayName = "Subscriber";
-
-  // eslint-disable-next-line react/display-name
-  const OuterComponent = forwardRef<R, P>((p, ref) => (
-    <Subscribe>
-      <Subscriber ref={ref} p={p} />
-    </Subscribe>
-  ));
-  // Copy over the component's display name, default props, etc.
-  Object.assign(OuterComponent, render);
-  return OuterComponent;
-}