fix: better reordering detection when remoteClearsLocal is true (#9771)

* Fix DEPRECATE_RELATIONSHIP_REMOTE_UPDATE_CLEARING_LOCAL_STATE reordering detection

* add failing test

* notification tests

* implement fix

* cleanup console logs

* fix spurrious type issue

---------

Co-authored-by: Chris Thoburn <runspired@users.noreply.github.com>

Author: gitKrystan

packages/build-config/vite.config.mjs

@@ -10,7 +10,6 @@ export const entryPoints = [
   './src/debugging.ts',
   './src/deprecations.ts',
   './src/canary-features.ts',
-  './src/runtime.ts',
 ];
 
 export default createConfig(

packages/core-types/src/-private.ts

@@ -16,7 +16,9 @@ type UniversalKey =
   | 'RequestCache'
   // @warp-drive/core-types/request
   | 'SkipCache'
-  | 'EnableHydration';
+  | 'EnableHydration'
+  // @warp-drive/core-types/runtime
+  | 'WarpDriveRuntimeConfig';
 
 type TransientKey =
   // @ember-data/tracking

packages/build-config/src/runtime.ts packages/core-types/src/runtime.ts

@@ -1,8 +1,10 @@
-import { LOG_CONFIG } from './-private/utils/logging';
+import type { LOG_CONFIG } from '@warp-drive/build-config/-private/utils/logging';
 
-const RuntimeConfig = {
+import { getOrSetUniversal } from './-private';
+
+const RuntimeConfig: { debug: Partial<LOG_CONFIG> } = getOrSetUniversal('WarpDriveRuntimeConfig', {
   debug: {},
-};
+});
 
 const settings = globalThis.sessionStorage?.getItem('WarpDriveRuntimeConfig');
 if (settings) {

packages/core-types/vite.config.mjs

@@ -18,6 +18,7 @@ export const entryPoints = [
   './src/utils.ts',
   // non-public
   './src/-private.ts',
+  './src/runtime.ts',
 ];
 
 export default createConfig(

packages/graph/src/-private/-diff.ts

@@ -235,34 +235,24 @@ function _compare<T>(
         // however: if the new remote order matches the current local order
         // we can disregard the change notification generation so long as
         // we are not configured to reset on remote update (which is deprecated)
-        if (DEPRECATE_RELATIONSHIP_REMOTE_UPDATE_CLEARING_LOCAL_STATE) {
-          if (!remoteClearsLocal && i < priorLocalLength) {
-            const priorLocalMember = priorLocalState![i];
-            if (priorLocalMember !== member) {
-              changed = true;
-            }
-          } else {
-            changed = true;
-          }
-        } else {
-          if (i < priorLocalLength) {
-            const priorLocalMember = priorLocalState![i];
-            if (priorLocalMember !== member) {
-              if (DEBUG_RELATIONSHIP_NOTIFICATIONS) {
-                // eslint-disable-next-line @typescript-eslint/no-unused-expressions, no-console
-                !changed && console.log(`changed because priorLocalMember !== member && member !== prevMember`);
-              }
-              changed = true;
-            }
-          } else if (i < finalLength) {
-            // if we have exceeded the length of priorLocalState and we are within the range
-            // of the finalState then we must have changed
+
+        if (i < priorLocalLength) {
+          const priorLocalMember = priorLocalState![i];
+          if (priorLocalMember !== member) {
             if (DEBUG_RELATIONSHIP_NOTIFICATIONS) {
               // eslint-disable-next-line @typescript-eslint/no-unused-expressions, no-console
-              !changed && console.log(`changed because priorMember !== member && index >= priorLocalLength`);
+              !changed && console.log(`changed because priorLocalMember !== member && member !== prevMember`);
             }
             changed = true;
           }
+        } else if (i < finalLength) {
+          // if we have exceeded the length of priorLocalState and we are within the range
+          // of the finalState then we must have changed
+          if (DEBUG_RELATIONSHIP_NOTIFICATIONS) {
+            // eslint-disable-next-line @typescript-eslint/no-unused-expressions, no-console
+            !changed && console.log(`changed because priorMember !== member && index >= priorLocalLength`);
+          }
+          changed = true;
         }
 
         // if remote order hasn't changed but local order differs
@@ -273,6 +263,10 @@ function _compare<T>(
           if (equalLength && !changed && i < priorLocalLength) {
             const priorLocalMember = priorLocalState![i];
             if (priorLocalMember !== prevMember) {
+              if (DEBUG_RELATIONSHIP_NOTIFICATIONS) {
+                // eslint-disable-next-line @typescript-eslint/no-unused-expressions, no-console
+                !changed && console.log(`changed because priorLocalMember !== prevMember && remoteClearsLocal`);
+              }
               changed = true;
             }
           }

packages/request/src/index.ts

@@ -1,4 +1,4 @@
-import { getRuntimeConfig, setLogging } from '@warp-drive/build-config/runtime';
+import { getRuntimeConfig, setLogging } from '@warp-drive/core-types/runtime';
 
 export { RequestManager as default } from './-private/manager';
 export { createDeferred } from './-private/future';

packages/store/src/-private/managers/notification-manager.ts

@@ -250,8 +250,20 @@ export default class NotificationManager {
           );
         }
       }
-    } else if (LOG_METRIC_COUNTS) {
-      count(`DISCARDED notify ${'type' in identifier ? identifier.type : '<document>'} ${value} ${key}`);
+    } else {
+      if (LOG_NOTIFICATIONS) {
+        log(
+          'notify',
+          'discarded',
+          `${'type' in identifier ? identifier.type : 'document'}`,
+          identifier.lid,
+          `${value}`,
+          key || ''
+        );
+      }
+      if (LOG_METRIC_COUNTS) {
+        count(`DISCARDED notify ${'type' in identifier ? identifier.type : '<document>'} ${value} ${key}`);
+      }
     }
 
     return hasSubscribers;

packages/store/src/-private/store-service.ts

@@ -24,7 +24,6 @@ import {
 } from '@warp-drive/build-config/deprecations';
 import { DEBUG, TESTING } from '@warp-drive/build-config/env';
 import { assert } from '@warp-drive/build-config/macros';
-import { getRuntimeConfig, setLogging } from '@warp-drive/build-config/runtime';
 import type { Cache } from '@warp-drive/core-types/cache';
 import type { Graph } from '@warp-drive/core-types/graph';
 import type {
@@ -34,6 +33,7 @@ import type {
 } from '@warp-drive/core-types/identifier';
 import type { TypedRecordInstance, TypeFromInstance } from '@warp-drive/core-types/record';
 import { EnableHydration, SkipCache } from '@warp-drive/core-types/request';
+import { getRuntimeConfig, setLogging } from '@warp-drive/core-types/runtime';
 import type { ResourceDocument } from '@warp-drive/core-types/spec/document';
 import type {
   CollectionResourceDocument,

packages/unpublished-test-infra/src/test-support/asserts/assert-notification.ts

@@ -8,7 +8,7 @@ import type Store from '@ember-data/store';
 import type { DocumentCacheOperation, NotificationType } from '@ember-data/store';
 import type { StableDocumentIdentifier } from '@warp-drive/core-types/identifier';
 
-type Counter = { count: number };
+type Counter = { count: number; delivered: number; ignored: number };
 type NotificationStorage = Map<
   StableDocumentIdentifier | StableRecordIdentifier | 'document' | 'resource',
   Map<NotificationType | DocumentCacheOperation, Counter | Map<string | symbol, Counter>>
@@ -34,7 +34,7 @@ function getCounter(
   let bucketStorage = identifierStorage.get(bucket);
   if (!bucketStorage) {
     if (bucket === 'added' || bucket === 'removed' || bucket === 'updated' || bucket === 'state') {
-      bucketStorage = { count: 0 };
+      bucketStorage = { count: 0, delivered: 0, ignored: 0 };
     } else {
       bucketStorage = new Map();
     }
@@ -46,7 +46,7 @@ function getCounter(
     const _key = key || Symbol.for(bucket);
     counter = bucketStorage.get(_key)!;
     if (!counter) {
-      counter = { count: 0 };
+      counter = { count: 0, delivered: 0, ignored: 0 };
       bucketStorage.set(_key, counter);
     }
   } else {
@@ -79,7 +79,15 @@ function setupNotifications(context: TestContext, store: Store) {
     counter.count++;
 
     // @ts-expect-error TS is bad at curried overloads
-    return originalNotify.apply(notifications, [identifier, bucket, key]);
+    const scheduled = originalNotify.apply(notifications, [identifier, bucket, key]);
+
+    if (scheduled) {
+      counter.delivered++;
+    } else {
+      counter.ignored++;
+    }
+
+    return scheduled;
   };
 }
 
@@ -97,15 +105,16 @@ export function configureNotificationsAssert(this: TestContext, assert: Assert)
     identifier: StableRecordIdentifier | StableDocumentIdentifier,
     bucket: NotificationType | DocumentCacheOperation,
     key: string | null,
-    count: number
+    count: number,
+    message?: string
   ) {
     const counter = getCounter(context, identifier, bucket, key);
 
     this.pushResult({
       result: counter.count === count,
       actual: counter.count,
       expected: count,
-      message: `Expected ${count} ${bucket} notifications for ${identifier.lid} ${key || ''}, got ${counter.count}`,
+      message: `${message ? message + ' | ' : ''}Expected ${count} ${bucket} notifications for ${identifier.lid} ${key || ''}, got ${counter.count}`,
     });
 
     counter.count = 0;

packages/unpublished-test-infra/src/test-support/asserts/index.ts

@@ -48,7 +48,8 @@ declare module '@warp-drive/diagnostic' {
       identifier: StableDocumentIdentifier | StableRecordIdentifier,
       bucket: NotificationType | CacheOperation,
       key: string | null,
-      count: number
+      count: number,
+      message?: string
     ): void;
 
     clearNotifications(): void;

tests/ember-data__graph/ember-cli-build.js

@@ -22,6 +22,7 @@ module.exports = async function (defaults) {
     compatWith: process.env.EMBER_DATA_FULL_COMPAT ? '99.0' : null,
     deprecations: {
       DISABLE_6X_DEPRECATIONS: false,
+      DEPRECATE_STORE_EXTENDS_EMBER_OBJECT: false,
     },
   });