enhance: Entity runs validate after marking circular reference point (#3133)

Author: ntucker

.changeset/wet-mirrors-visit.md

@@ -0,0 +1,9 @@
+---
+'@data-client/endpoint': patch
+---
+
+Validate after marking cirucular reference loops
+
+This should not change any behavior as validate should be deterministic so if it fails
+it will fail again and failure measure throwing which exits the whole stack.
+This improves code grouping. (And possibly cache locality improvement - though didn't check.)

packages/endpoint/src/schemas/EntitySchema.ts

@@ -300,8 +300,9 @@ export default function EntitySchema<TBase extends Constructor>(
       } else {
         id = `${id}`;
       }
-      const entityType = this.key;
 
+      /* Circular reference short-circuiter */
+      const entityType = this.key;
       if (!(entityType in visitedEntities)) {
         visitedEntities[entityType] = {};
       }
@@ -313,11 +314,11 @@ export default function EntitySchema<TBase extends Constructor>(
       ) {
         return id;
       }
+      visitedEntities[entityType][id].push(input);
+
       const errorMessage = this.validate(processedEntity);
       throwValidationError(errorMessage);
 
-      visitedEntities[entityType][id].push(input);
-
       Object.keys(this.schema).forEach(key => {
         if (Object.hasOwn(processedEntity, key)) {
           processedEntity[key] = visit(

packages/normalizr/src/__tests__/__snapshots__/index.test.js.snap

@@ -538,6 +538,8 @@ exports[`normalize normalizes entities with circular references 1`] = `
 }
 `;
 
+exports[`normalize normalizes entities with circular references that fails validation 1`] = `"this always fails"`;
+
 exports[`normalize normalizes nested entities 1`] = `
 {
   "entities": {

packages/normalizr/src/__tests__/index.test.js

@@ -216,6 +216,20 @@ describe('normalize', () => {
     expect(normalize(input, User)).toMatchSnapshot();
   });
 
+  test('normalizes entities with circular references that fails validation', () => {
+    class User extends IDEntity {
+      static validate(processedEntity) {
+        return 'this always fails';
+      }
+    }
+    User.schema = { friends: [User] };
+
+    const input = { id: '123', friends: [] };
+    input.friends.push(input);
+
+    expect(() => normalize(input, User)).toThrowErrorMatchingSnapshot();
+  });
+
   test('normalizes nested entities', () => {
     class User extends IDEntity {}
     class Comment extends IDEntity {

packages/normalizr/src/normalize.ts

@@ -59,9 +59,9 @@ const addEntities =
   (
     newEntities: Record<string, any>,
     newIndexes: Record<string, any>,
-    storeEntities: Record<string, any>,
-    storeIndexes: Record<string, any>,
-    storeEntityMeta: {
+    entitiesCopy: Record<string, any>,
+    indexesCopy: Record<string, any>,
+    entityMetaCopy: {
       [entityKey: string]: {
         [pk: string]: {
           date: number;
@@ -78,8 +78,8 @@ const addEntities =
     if (!(schemaKey in newEntities)) {
       newEntities[schemaKey] = {};
       // we will be editing these, so we need to clone them first
-      storeEntities[schemaKey] = { ...storeEntities[schemaKey] };
-      storeEntityMeta[schemaKey] = { ...storeEntityMeta[schemaKey] };
+      entitiesCopy[schemaKey] = { ...entitiesCopy[schemaKey] };
+      entityMetaCopy[schemaKey] = { ...entityMetaCopy[schemaKey] };
     }
 
     const existingEntity = newEntities[schemaKey][id];
@@ -89,49 +89,49 @@ const addEntities =
         processedEntity,
       );
     } else {
-      const inStoreEntity = storeEntities[schemaKey][id];
+      const inStoreEntity = entitiesCopy[schemaKey][id];
       let inStoreMeta: {
         date: number;
         expiresAt: number;
         fetchedAt: number;
       };
       // this case we already have this entity in store
-      if (inStoreEntity && (inStoreMeta = storeEntityMeta[schemaKey][id])) {
+      if (inStoreEntity && (inStoreMeta = entityMetaCopy[schemaKey][id])) {
         newEntities[schemaKey][id] = schema.mergeWithStore(
           inStoreMeta,
           actionMeta,
           inStoreEntity,
           processedEntity,
         );
-        storeEntityMeta[schemaKey][id] = schema.mergeMetaWithStore(
+        entityMetaCopy[schemaKey][id] = schema.mergeMetaWithStore(
           inStoreMeta,
           actionMeta,
           inStoreEntity,
           processedEntity,
         );
       } else {
         newEntities[schemaKey][id] = processedEntity;
-        storeEntityMeta[schemaKey][id] = actionMeta;
+        entityMetaCopy[schemaKey][id] = actionMeta;
       }
     }
 
     // update index
     if (schema.indexes) {
       if (!(schemaKey in newIndexes)) {
         newIndexes[schemaKey] = {};
-        storeIndexes[schemaKey] = { ...storeIndexes[schemaKey] };
+        indexesCopy[schemaKey] = { ...indexesCopy[schemaKey] };
       }
       handleIndexes(
         id,
         schema.indexes,
         newIndexes[schemaKey],
-        storeIndexes[schemaKey],
+        indexesCopy[schemaKey],
         newEntities[schemaKey][id],
-        storeEntities[schemaKey],
+        entitiesCopy[schemaKey],
       );
     }
     // set this after index updates so we know what indexes to remove from
-    storeEntities[schemaKey][id] = newEntities[schemaKey][id];
+    entitiesCopy[schemaKey][id] = newEntities[schemaKey][id];
   };
 
 function handleIndexes(