chore: Make reflect-client use ReplicacheImpl (#1652)

and remove `exposeInternalAPI` hacks
rocicorp · Apr 26, 2024 · 68710da · 68710da
1 parent 02a81f7
commit 68710da
Show file tree

Hide file tree

Showing 5 changed files with 28 additions and 87 deletions.
diff --git a/packages/reflect-client/src/client/reflect.ts b/packages/reflect-client/src/client/reflect.ts
@@ -33,12 +33,12 @@ import {
   PushRequestV1,
   Pusher,
   PusherResult,
-  Replicache,
   ReplicacheOptions,
   UpdateNeededReason as ReplicacheUpdateNeededReason,
   SubscribeOptions,
   dropDatabase,
 } from 'replicache';
+import {ReplicacheImpl} from 'replicache/src/replicache-impl.js';
 import {assert} from 'shared/src/asserts.js';
 import {getDocumentVisibilityWatcher} from 'shared/src/document-visible.js';
 import {getDocument} from 'shared/src/get-document.js';
@@ -166,15 +166,10 @@ const enum PingResult {
   Success = 1,
 }
 
-// Keep in sync with packages/replicache/src/replicache-options.ts
-export interface ReplicacheInternalAPI {
-  lastMutationID(): number;
-}
-
 export class Reflect<MD extends MutatorDefs> {
   readonly version = version;
 
-  readonly #rep: Replicache<MD>;
+  readonly #rep: ReplicacheImpl<MD>;
   readonly #server: HTTPString | null;
   readonly userID: string;
   readonly roomID: string;
@@ -215,7 +210,6 @@ export class Reflect<MD extends MutatorDefs> {
     // intentionally empty
   };
 
-  #internalAPI: ReplicacheInternalAPI;
   readonly #closeBeaconManager: CloseBeaconManager;
 
   /**
@@ -365,19 +359,15 @@ export class Reflect<MD extends MutatorDefs> {
       licenseKey: 'reflect-client-static-key',
       kvStore,
     };
-    let internalAPI: ReplicacheInternalAPI;
     const replicacheInternalOptions = {
       enableLicensing: false,
-      exposeInternalAPI: (api: ReplicacheInternalAPI) => {
-        internalAPI = api;
-      },
     };
 
-    this.#rep = new Replicache({
+    this.#rep = new ReplicacheImpl({
       ...replicacheOptions,
       ...replicacheInternalOptions,
     });
-    this.#internalAPI = internalAPI!;
+
     this.#rep.getAuth = this.#getAuthToken;
     this.#onUpdateNeeded = this.#rep.onUpdateNeeded; // defaults to reload.
     this.#server = server;
@@ -424,7 +414,7 @@ export class Reflect<MD extends MutatorDefs> {
       this.#lc,
       server,
       () => this.#rep.auth,
-      () => this.#internalAPI.lastMutationID(),
+      () => this.#rep.lastMutationID,
       this.#closeAbortController.signal,
       getWindow(),
     );

diff --git a/packages/replicache/perf/replicache.ts b/packages/replicache/perf/replicache.ts
@@ -1,19 +1,18 @@
 import {resolver} from '@rocicorp/resolver';
 import {assert} from 'shared/src/asserts.js';
 import {deepEqual} from 'shared/src/json.js';
+import {dropIDBStoreWithMemFallback} from '../src/kv/idb-store-with-mem-fallback.js';
 import {
   IndexDefinitions,
   JSONValue,
   MutatorDefs,
   PatchOperation,
   ReadTransaction,
-  Replicache,
   ReplicacheOptions,
   TEST_LICENSE_KEY,
   WriteTransaction,
-} from '../out/replicache.js';
-import {dropIDBStoreWithMemFallback} from '../src/kv/idb-store-with-mem-fallback.js';
-import type {ReplicacheInternalAPI} from '../src/replicache-options.js';
+} from '../src/mod.js';
+import {ReplicacheImpl as Replicache} from '../src/replicache-impl.js';
 import {uuid} from '../src/uuid.js';
 import {
   TestDataObject,
@@ -322,33 +321,16 @@ export function benchmarkRebase(opts: {
 }
 
 class ReplicachePerfTest<MD extends MutatorDefs> extends Replicache<MD> {
-  readonly #internalAPI: ReplicacheInternalAPI;
   constructor(options: Omit<ReplicacheOptions<MD>, 'licenseKey'>) {
-    let internalAPI!: ReplicacheInternalAPI;
     super({
       ...options,
       licenseKey: TEST_LICENSE_KEY,
-      exposeInternalAPI: (api: ReplicacheInternalAPI) => {
-        internalAPI = api;
-      },
+
       enableLicensing: false,
       enableMutationRecovery: false,
       enableScheduledRefresh: false,
       enableScheduledPersist: false,
-    } as ReplicacheOptions<MD>);
-    this.#internalAPI = internalAPI;
-  }
-
-  get onUpdateNeeded() {
-    return () => {};
-  }
-
-  persist(): Promise<void> {
-    return this.#internalAPI.persist();
-  }
-
-  refresh(): Promise<void> {
-    return this.#internalAPI.refresh();
+    });
   }
 }
 

diff --git a/packages/replicache/src/replicache-impl.ts b/packages/replicache/src/replicache-impl.ts
@@ -260,14 +260,14 @@ export class ReplicacheImpl<MD extends MutatorDefs = {}> {
   readonly #enableScheduledRefresh: boolean;
   readonly #enablePullAndPushInOpen: boolean;
   #persistScheduler = new ProcessScheduler(
-    () => this.#persist(),
+    () => this.persist(),
     PERSIST_IDLE_TIMEOUT_MS,
     PERSIST_THROTTLE_MS,
     this.#closeAbortController.signal,
   );
   readonly #onPersist: OnPersist;
   #refreshScheduler = new ProcessScheduler(
-    () => this.#refresh(),
+    () => this.refresh(),
     REFRESH_IDLE_TIMEOUT_MS,
     REFRESH_THROTTLE_MS,
     this.#closeAbortController.signal,
@@ -345,7 +345,7 @@ export class ReplicacheImpl<MD extends MutatorDefs = {}> {
   onBeginPull = () => undefined;
   onRecoverMutations = (r: Promise<boolean>) => r;
 
-  constructor(options: ReplicacheOptions<MD>) {
+  constructor(options: ReplicacheOptions<MD> & ReplicacheInternalOptions) {
     const {
       name,
       logLevel = 'info',
@@ -362,6 +362,13 @@ export class ReplicacheImpl<MD extends MutatorDefs = {}> {
       pusher,
       licenseKey,
       indexes = {},
+      enableMutationRecovery = true,
+      enableLicensing = true,
+      enableScheduledPersist = true,
+
+      enableScheduledRefresh = true,
+
+      enablePullAndPushInOpen = true,
     } = options;
     this.auth = auth ?? '';
     this.pullURL = pullURL;
@@ -376,27 +383,10 @@ export class ReplicacheImpl<MD extends MutatorDefs = {}> {
     this.puller = puller ?? getDefaultPuller(this);
     this.pusher = pusher ?? getDefaultPusher(this);
 
-    const internalOptions = options as unknown as ReplicacheInternalOptions;
-    const enableMutationRecovery =
-      internalOptions.enableMutationRecovery ?? true;
-    this.#enableLicensing = internalOptions.enableLicensing ?? true;
-    this.#enableScheduledPersist =
-      internalOptions.enableScheduledPersist ?? true;
-    this.#enableScheduledRefresh =
-      internalOptions.enableScheduledRefresh ?? true;
-    this.#enablePullAndPushInOpen =
-      internalOptions.enablePullAndPushInOpen ?? true;
-
-    if (internalOptions.exposeInternalAPI) {
-      internalOptions.exposeInternalAPI({
-        // Needed by perf test
-        persist: () => this.#persist(),
-        // Needed by perf test
-        refresh: () => this.#refresh(),
-        // Needed by reflect
-        lastMutationID: () => this.lastMutationID,
-      });
-    }
+    this.#enableLicensing = enableLicensing;
+    this.#enableScheduledPersist = enableScheduledPersist;
+    this.#enableScheduledRefresh = enableScheduledRefresh;
+    this.#enablePullAndPushInOpen = enablePullAndPushInOpen;
 
     this.#lc = createLogContext(logLevel, logSinks, {name});
     this.#lc.debug?.('Constructing Replicache', {
@@ -1233,7 +1223,7 @@ export class ReplicacheImpl<MD extends MutatorDefs = {}> {
     return {requestID, syncHead, ok: httpRequestInfo.httpStatusCode === 200};
   }
 
-  async #persist(): Promise<void> {
+  async persist(): Promise<void> {
     assert(!this.#persistIsRunning);
     this.#persistIsRunning = true;
     try {
@@ -1271,7 +1261,7 @@ export class ReplicacheImpl<MD extends MutatorDefs = {}> {
     this.#onPersist({clientID, clientGroupID});
   }
 
-  async #refresh(): Promise<void> {
+  async refresh(): Promise<void> {
     await this.#ready;
     const {clientID} = this;
     if (this.#closed) {

diff --git a/packages/replicache/src/replicache-options.ts b/packages/replicache/src/replicache-options.ts
@@ -244,16 +244,4 @@ export type ReplicacheInternalOptions = {
    * Defaults to true.
    */
   enablePullAndPushInOpen?: boolean | undefined;
-
-  /**
-   * Allows exposing parts of the internal API to a subclass. This works when
-   * thing have been minified and with the npm package.
-   */
-  exposeInternalAPI?: (api: ReplicacheInternalAPI) => void;
 };
-
-export interface ReplicacheInternalAPI {
-  persist(): Promise<void>;
-  refresh(): Promise<void>;
-  lastMutationID(): number;
-}
diff --git a/packages/replicache/src/test-util.ts b/packages/replicache/src/test-util.ts
@@ -17,7 +17,6 @@ import {
 } from './persist/idb-databases-store-db-name.js';
 import type {PullResponseV1} from './puller.js';
 import type {
-  ReplicacheInternalAPI,
   ReplicacheInternalOptions,
   ReplicacheOptions,
 } from './replicache-options.js';
@@ -39,19 +38,11 @@ export class ReplicacheTest<
   // eslint-disable-next-line @typescript-eslint/ban-types
   MD extends MutatorDefs = {},
 > extends Replicache<MD> {
-  readonly #internalAPI!: ReplicacheInternalAPI;
   readonly #impl: ReplicacheImpl<MD>;
   recoverMutationsFake: sinon.SinonSpy<[r: Promise<boolean>], Promise<boolean>>;
 
   constructor(options: ReplicacheOptions<MD>) {
-    let internalAPI!: ReplicacheInternalAPI;
-    super({
-      ...options,
-      exposeInternalAPI: (api: ReplicacheInternalAPI) => {
-        internalAPI = api;
-      },
-    } as ReplicacheOptions<MD>);
-    this.#internalAPI = internalAPI;
+    super(options);
     this.#impl = getImpl(this);
     this.recoverMutationsFake = this.onRecoverMutations = sinon.fake(r => r);
   }
@@ -69,7 +60,7 @@ export class ReplicacheTest<
   }
 
   persist() {
-    return this.#internalAPI.persist();
+    return this.#impl.persist();
   }
 
   recoverMutations(): Promise<boolean> {