fix: keep server stores until destroy() and compare object query hashes by content (#48)

On the server every store armed a 500 ms init timer and a 10 s touched
timer although a request renders once and calls destroy(); the timers only
kept ~1.7 MB of state and 20 handles alive per request. The manager now
detects the server and arms no destroy timers there.

SuspenseQuery compared hashes with strict equality, so an object or array
hash created on every render never matched the stored one and the query
was re-run and re-thrown forever. Object hashes are now serialised before
they are stored and compared.

Author: MatthewPattell

__tests__/manager.ts

@@ -506,6 +506,52 @@ describe('Manager', () => {
     clock.restore();
   });
 
+  it('should not arm destroy timers on the server and remove stores on destroy', async () => {
+    const clock = sandbox.useFakeTimers();
+    const onDestroy = sandbox.stub();
+
+    class RelativeStore {
+      public libStoreId?: string;
+      public libStoreStatus?: StoreStatus;
+      public libDestroyTimer?: ReturnType<typeof setTimeout>;
+      public isGlobal?: boolean;
+      public onDestroy = onDestroy;
+
+      constructor() {}
+    }
+
+    const manager = new Manager({ options: { destroyTimers: { init: 1, touched: 1, unused: 1 } } });
+
+    manager.isServer = true;
+
+    const result = manager.createStores(
+      [['relativeStore', RelativeStore]],
+      parentContextId,
+      managedContextId,
+      suspenseId,
+      managedComponentName,
+    );
+    const store = result.relativeStores.relativeStore as RelativeStore;
+
+    manager.touchedStores({ relativeStore: store });
+    manager.mountStores(managedContextId, result)();
+
+    expect(store.libStoreStatus).to.equal(StoreStatus.unused);
+    expect(store.libDestroyTimer).to.equal(undefined);
+
+    await clock.tickAsync(100);
+
+    expect(manager.getStores().has(store.libStoreId!)).to.equal(true);
+    sinon.assert.notCalled(onDestroy);
+
+    manager.destroy();
+
+    expect(manager.getStores().size).to.equal(0);
+    sinon.assert.calledOnce(onDestroy);
+
+    clock.restore();
+  });
+
   it('should run cleanup returned from init when store is destroyed', async () => {
     const clock = sandbox.useFakeTimers();
     const onDestroy = sandbox.stub();

__tests__/suspense-query.ts

@@ -46,6 +46,24 @@ describe('SuspenseQuery', () => {
     expect(target.query(() => Promise.resolve('unused'), { hash: 'hash-1' })).to.be.undefined;
   });
 
+  it('should treat object hashes by content for queries and subqueries', async () => {
+    const store: Record<string, unknown> = {};
+    const target = new SuspenseQuery(store);
+    const request = () => Promise.resolve('done');
+
+    expect(() => target.query(request, { hash: { page: 1, tags: ['a'] } })).to.throw();
+    await Promise.resolve();
+    await Promise.resolve();
+
+    expect(store.sR).to.deep.equal({ hash: '{"page":1,"tags":["a"]}', done: true });
+    expect(target.query(request, { hash: { page: 1, tags: ['a'] } })).to.be.undefined;
+    expect(() => target.query(request, { hash: { page: 2, tags: ['a'] } })).to.throw();
+
+    expect(target.subquery(request, { id: subqueryId, hash: { page: 1 } })).to.be.undefined;
+    expect(target.subquery(request, { id: subqueryId, hash: { page: 1 } })).to.be.undefined;
+    expect(() => target.subquery(request, { id: subqueryId, hash: { page: 2 } })).to.throw();
+  });
+
   it.each(['resolve', 'reject'] as const)(
     'should ignore an obsolete query that settles with %s',
     async (settlement) => {

docs/guide/ssr.md

@@ -43,6 +43,8 @@ Examples that need cleanup:
 - cleanup returned from `init`
 - internal store cleanup
 
+On the server the manager arms no destroy timers (`destroyTimers` only apply in the browser): stores live exactly as long as the request, and `destroy()` is the only teardown.
+
 ## Practical guidance
 
 Do:

src/manager.ts

@@ -92,6 +92,11 @@ class Manager {
   /**
    * @constructor
    */
+  /**
+   * Detect server side: stores live until the manager is destroyed, no timers are armed
+   */
+  public isServer = typeof window === 'undefined';
+
   public constructor({ initState, storesParams, storage, options, logger }: IManagerParams = {}) {
     this.initState = initState || {};
     this.storesParams = storesParams || {};
@@ -613,6 +618,11 @@ class Manager {
 
     clearTimeout(store.libDestroyTimer);
 
+    // A request renders once and calls destroy(): timers would only keep the state alive longer.
+    if (this.isServer) {
+      return;
+    }
+
     let destroyTime = 0;
 
     switch (status) {

src/suspense-query.ts

@@ -16,6 +16,12 @@ interface ISuspenseQueryOptions {
   hash?: unknown;
 }
 
+/**
+ * Objects and arrays are compared by content: a fresh hash object on every render must still match.
+ */
+const normalizeHash = (hash: unknown): unknown =>
+  typeof hash === 'object' && hash !== null ? JSON.stringify(hash) : hash;
+
 interface ISuspenseSubqueryOptions {
   id: string;
   hash: unknown;
@@ -142,7 +148,7 @@ class SuspenseQuery {
     promise: () => Promise<TReturn>,
     options: ISuspenseQueryOptions = {},
   ): TReturn | undefined => {
-    const { hash = '' } = options;
+    const hash = normalizeHash(options.hash ?? '');
     const { fieldName } = this.params;
 
     if (this.isComplete(hash)) {
@@ -190,7 +196,8 @@ class SuspenseQuery {
     promise: () => Promise<TReturn>,
     options: ISuspenseSubqueryOptions,
   ): TReturn | undefined => {
-    const { id, hash } = options;
+    const { id } = options;
+    const hash = normalizeHash(options.hash);
     const subquery = this.subqueries.get(id);
 
     // skip first run