enhance: Add consolidate callbacks for queryKey and normalize in 'delegates' (#3427)

Author: ntucker

.changeset/cold-walls-like.md

@@ -0,0 +1,6 @@
+---
+'@data-client/core': patch
+'@data-client/react': patch
+---
+
+Fix controller.get and controller.getQueryMeta 'state' argument types

.changeset/forty-masks-join.md

@@ -0,0 +1,7 @@
+---
+'@data-client/endpoint': patch
+'@data-client/graphql': patch
+'@data-client/rest': patch
+---
+
+Fix: ensure string id in Entity set when process returns undefined (meaning INVALID)

.changeset/proud-insects-smile.md

@@ -0,0 +1,49 @@
+---
+'@data-client/normalizr': minor
+'@data-client/endpoint': minor
+'@data-client/core': minor
+'@data-client/graphql': minor
+'@data-client/react': minor
+'@data-client/rest': minor
+---
+
+BREAKING CHANGE: schema.normalize(...args, addEntity, getEntity, checkLoop) -> schema.normalize(...args, delegate)
+
+We consolidate all 'callback' functions during recursion calls into a single 'delegate' argument.
+
+```ts
+/** Helpers during schema.normalize() */
+export interface INormalizeDelegate {
+  /** Action meta-data for this normalize call */
+  readonly meta: { fetchedAt: number; date: number; expiresAt: number };
+  /** Gets any previously normalized entity from store */
+  getEntity: GetEntity;
+  /** Updates an entity using merge lifecycles when it has previously been set */
+  mergeEntity(
+    schema: Mergeable & { indexes?: any },
+    pk: string,
+    incomingEntity: any,
+  ): void;
+  /** Sets an entity overwriting any previously set values */
+  setEntity(
+    schema: { key: string; indexes?: any },
+    pk: string,
+    entity: any,
+    meta?: { fetchedAt: number; date: number; expiresAt: number },
+  ): void;
+  /** Returns true when we're in a cycle, so we should not continue recursing */
+  checkLoop(key: string, pk: string, input: object): boolean;
+}
+```
+
+#### Before
+
+```ts
+addEntity(this, processedEntity, id);
+```
+
+#### After
+
+```ts
+delegate.mergeEntity(this, id, processedEntity);
+```

.changeset/wicked-bags-appear.md

@@ -0,0 +1,43 @@
+---
+'@data-client/normalizr': minor
+'@data-client/endpoint': minor
+'@data-client/core': minor
+'@data-client/graphql': minor
+'@data-client/react': minor
+'@data-client/rest': minor
+---
+
+BREAKING CHANGE: schema.queryKey(args, queryKey, getEntity, getIndex) -> schema.queryKey(args, unvisit, delegate)
+BREAKING CHANGE: delegate.getIndex() returns the index directly, rather than object.
+
+We consolidate all 'callback' functions during recursion calls into a single 'delegate' argument.
+
+Our recursive call is renamed from queryKey to unvisit, and does not require the last two arguments.
+
+```ts
+/** Accessors to the currently processing state while building query */
+export interface IQueryDelegate {
+  getEntity: GetEntity;
+  getIndex: GetIndex;
+}
+```
+
+#### Before
+
+```ts
+queryKey(args, queryKey, getEntity, getIndex) {
+  getIndex(schema.key, indexName, value)[value];
+  getEntity(this.key, id);
+  return queryKey(this.schema, args, getEntity, getIndex);
+}
+```
+
+#### After
+
+```ts
+queryKey(args, unvisit, delegate) {
+  delegate.getIndex(schema.key, indexName, value);
+  delegate.getEntity(this.key, id);
+  return unvisit(this.schema, args);
+}
+```

packages/core/src/controller/Controller.ts

@@ -580,7 +580,7 @@ export default class Controller<
     schema: S,
     ...rest: readonly [
       ...SchemaArgs<S>,
-      Pick<State<unknown>, 'entities' | 'entityMeta'>,
+      Pick<State<unknown>, 'entities' | 'indexes'>,
     ]
   ): DenormalizeNullable<S> | undefined {
     const state = rest[rest.length - 1] as State<any>;
@@ -600,7 +600,7 @@ export default class Controller<
     schema: S,
     ...rest: readonly [
       ...SchemaArgs<S>,
-      Pick<State<unknown>, 'entities' | 'entityMeta'>,
+      Pick<State<unknown>, 'entities' | 'indexes'>,
     ]
   ): {
     data: DenormalizeNullable<S> | undefined;

packages/core/src/controller/__tests__/__snapshots__/get.ts.snap

@@ -53,6 +53,22 @@ Group {
 }
 `;
 
+exports[`Controller.get() indexes query Entity based on index 1`] = `
+User {
+  "id": "1",
+  "staff": false,
+  "username": "bob",
+}
+`;
+
+exports[`Controller.get() indexes query indexes after empty state 1`] = `
+User {
+  "id": "1",
+  "staff": false,
+  "username": "bob",
+}
+`;
+
 exports[`Controller.get() query All should get all entities 1`] = `
 [
   Tacos {
@@ -105,13 +121,6 @@ exports[`Controller.get() query Collection based on args 2`] = `
 ]
 `;
 
-exports[`Controller.get() query Entity based on index 1`] = `
-User {
-  "id": "1",
-  "username": "bob",
-}
-`;
-
 exports[`Controller.get() query Entity based on pk 1`] = `
 Tacos {
   "id": "1",

packages/core/src/controller/__tests__/get.ts

@@ -1,6 +1,7 @@
 import { Entity, schema } from '@data-client/endpoint';
 
 import { initialState } from '../../state/reducer/createReducer';
+import { State } from '../../types';
 import Controller from '../Controller';
 
 class Tacos extends Entity {
@@ -46,37 +47,113 @@ describe('Controller.get()', () => {
     () => controller.get(Tacos, { doesnotexist: 5 }, state);
   });
 
-  it('query Entity based on index', () => {
+  describe('indexes', () => {
     class User extends Entity {
       id = '';
       username = '';
+      staff = false;
 
       static indexes = ['username'] as const;
     }
+    it('query Entity based on index', () => {
+      const controller = new Controller();
+      const state: State<unknown> = {
+        ...initialState,
+        entities: {
+          User: {
+            '1': { id: '1', username: 'bob' },
+            '2': { id: '2', username: 'george' },
+          },
+        },
+        indexes: {
+          User: {
+            username: {
+              bob: '1',
+              george: '2',
+            },
+          },
+        },
+      };
 
-    const controller = new Controller();
-    const state = {
-      ...initialState,
-      entities: {
-        User: {
-          '1': { id: '1', username: 'bob' },
+      const bob = controller.get(User, { username: 'bob' }, state);
+      expect(bob).toBeDefined();
+      expect(bob).toBeInstanceOf(User);
+      expect(bob).toMatchSnapshot();
+      // stability
+      expect(controller.get(User, { username: 'bob' }, state)).toBe(bob);
+      // should be same as id lookup
+      expect(controller.get(User, { id: '1' }, state)).toBe(bob);
+      // update index
+      let nextState: State<unknown> = {
+        ...state,
+        entities: {
+          ...state.entities,
+          User: {
+            ...state.entities.User,
+            '1': { id: '1', username: 'george' },
+            '2': { id: '2', username: 'bob' },
+          },
         },
-      },
-      indexes: {
-        User: {
-          username: {
-            bob: '1',
+        indexes: {
+          ...state.indexes,
+          User: {
+            ...state.indexes.User,
+            username: {
+              ...state.indexes.User.username,
+              bob: '2',
+              george: '1',
+            },
           },
         },
-      },
-    };
+      };
+      expect(controller.get(User, { username: 'bob' }, nextState)).not.toBe(
+        bob,
+      );
+      nextState = {
+        ...state,
+        entities: {
+          ...state.entities,
+          User: {
+            ...state.entities.User,
+            '1': { id: '1', username: 'bob', staff: true },
+          },
+        },
+      };
+      // update entity keep index
+      const nextBob = controller.get(User, { username: 'bob' }, nextState);
+      expect(nextBob).not.toBe(bob);
+      expect(nextBob).toBeDefined();
+      expect(nextBob).toBeInstanceOf(User);
+      expect(nextBob?.staff).toBe(true);
+    });
 
-    const bob = controller.get(User, { username: 'bob' }, state);
-    expect(bob).toBeDefined();
-    expect(bob).toBeInstanceOf(User);
-    expect(bob).toMatchSnapshot();
-    // should be same as id lookup
-    expect(bob).toBe(controller.get(User, { id: '1' }, state));
+    it('query indexes after empty state', () => {
+      const controller = new Controller();
+      expect(
+        controller.get(User, { username: 'bob' }, initialState),
+      ).toBeUndefined();
+      const state: State<unknown> = {
+        ...initialState,
+        entities: {
+          User: {
+            '1': { id: '1', username: 'bob' },
+            '2': { id: '2', username: 'george' },
+          },
+        },
+        indexes: {
+          User: {
+            username: {
+              bob: '1',
+              george: '2',
+            },
+          },
+        },
+      };
+      const bob = controller.get(User, { username: 'bob' }, state);
+      expect(bob).toBeDefined();
+      expect(bob).toBeInstanceOf(User);
+      expect(bob).toMatchSnapshot();
+    });
   });
 
   it('query Collection based on args', () => {

packages/core/src/index.ts

@@ -16,6 +16,9 @@ export type {
   EndpointExtraOptions,
   Queryable,
   SchemaArgs,
+  Mergeable,
+  IQueryDelegate,
+  INormalizeDelegate,
   NI,
 } from '@data-client/normalizr';
 export { ExpiryStatus } from '@data-client/normalizr';

packages/core/src/state/__tests__/reducer.ts

@@ -259,6 +259,11 @@ describe('reducer', () => {
           [id]: { id, counter: 5 },
         },
       },
+      entityMeta: {
+        [Counter.key]: {
+          [id]: { date: 0, fetchedAt: 0, expiresAt: 0 },
+        },
+      },
     };
     const newState = reducer(state, action);
     // eslint-disable-next-line @typescript-eslint/ban-ts-comment

packages/endpoint/src-4.0-types/schemaArgs.d.ts

@@ -1,6 +1,6 @@
-import type { Schema, EntityInterface } from './interface.js';
+import type { Schema } from './interface.js';
 import type { EntityFields } from './schemas/EntityFields.js';
-export type SchemaArgs<S extends Schema> = S extends EntityInterface<infer U> ? [EntityFields<U>] : S extends ({
+export type SchemaArgs<S extends Schema> = S extends { createIfValid: any; pk: any; key: string; prototype: infer U } ? [EntityFields<U>] : S extends ({
     queryKey(args: infer Args, ...rest: any): any;
 }) ? Args : S extends {
     [K: string]: any;