From 4a96d27e6e4a57e58d5702de5733d017f03e25c9 Mon Sep 17 00:00:00 2001 From: Nathaniel Tucker Date: Thu, 27 Jun 2024 14:29:57 +0200 Subject: [PATCH] enhance: Make argsKey optional in memo.query and memo.buildQueryKey --- examples/benchmark/normalizr.js | 10 +---- packages/core/src/controller/Controller.ts | 13 +------ .../src/schemas/__tests__/All.test.ts | 15 +++---- .../src/schemas/__tests__/Collection.test.ts | 3 -- .../src/schemas/__tests__/Query.test.ts | 18 ++++----- packages/normalizr/README.md | 6 +-- packages/normalizr/src/__tests__/MemoCache.ts | 39 ++----------------- packages/normalizr/src/memo/MemoCache.ts | 8 ++-- .../editor-types/@data-client/core.d.ts | 8 ++-- .../editor-types/@data-client/normalizr.d.ts | 8 ++-- 10 files changed, 35 insertions(+), 93 deletions(-) diff --git a/examples/benchmark/normalizr.js b/examples/benchmark/normalizr.js index e0941cdd21b7..d632f4229da3 100644 --- a/examples/benchmark/normalizr.js +++ b/examples/benchmark/normalizr.js @@ -22,14 +22,12 @@ const { result, entities } = normalize(ProjectSchema, data); const queryState = normalize(ProjectQuery, data); const queryMemo = new MemoCache(); queryState.result = queryMemo.buildQueryKey( - '', ProjectQuery, [], queryState.entities, queryState.indexes, ); const queryInfer = queryMemo.buildQueryKey( - '', ProjectQuerySorted, [], queryState.entities, @@ -57,17 +55,11 @@ export default function addNormlizrSuite(suite) { let curState = initialState; return suite .add('normalizeLong', () => { - normalize( - ProjectSchema, - data, - actionMeta, - curState, - ); + normalize(ProjectSchema, data, actionMeta, curState); curState = { ...initialState, entities: {}, endpoints: {} }; }) .add('infer All', () => { return new MemoCache().buildQueryKey( - '', ProjectQuery, [], queryState.entities, diff --git a/packages/core/src/controller/Controller.ts b/packages/core/src/controller/Controller.ts index fb31eea26e76..431c4b54a9d1 100644 --- a/packages/core/src/controller/Controller.ts +++ b/packages/core/src/controller/Controller.ts @@ -430,11 +430,11 @@ export default class Controller< shouldQuery ? // nothing in endpoints cache, so try querying if we have a schema to do so this.memo.buildQueryKey( - key, schema, args, state.entities as any, state.indexes, + key, ) : cacheEndpoints; @@ -500,16 +500,7 @@ export default class Controller< .slice(0, rest.length - 1) .map(ensurePojo) as SchemaArgs; - // NOTE: different orders can result in cache busting here; but since it's just a perf penalty we will allow for now - const key = JSON.stringify(args); - - return this.memo.query( - key, - schema, - args, - state.entities as any, - state.indexes, - ); + return this.memo.query(schema, args, state.entities as any, state.indexes); } private getSchemaResponse( diff --git a/packages/endpoint/src/schemas/__tests__/All.test.ts b/packages/endpoint/src/schemas/__tests__/All.test.ts index 67f123fc1c5a..9c61eeb31f90 100644 --- a/packages/endpoint/src/schemas/__tests__/All.test.ts +++ b/packages/endpoint/src/schemas/__tests__/All.test.ts @@ -115,7 +115,7 @@ describe.each([ }; const sch = new schema.All(Cat); expect( - new MemoCache().query('', sch, [], createInput(entities), {}), + new MemoCache().query(sch, [], createInput(entities), {}), ).toMatchSnapshot(); }); @@ -129,7 +129,7 @@ describe.each([ }, }; expect( - new MemoCache().query('', catSchema, [], createInput(entities), {}), + new MemoCache().query(catSchema, [], createInput(entities), {}), ).toMatchSnapshot(); }); @@ -143,7 +143,6 @@ describe.each([ }, }; const value = new MemoCache().query( - '', catSchema, [], createInput(entities), @@ -167,7 +166,6 @@ describe.each([ }, }; const value = new MemoCache().query( - '', catSchema, [], createInput(entities) as any, @@ -191,11 +189,11 @@ describe.each([ }, }; const memo = new MemoCache(); - const value = memo.query('', catSchema, [], entities, {}); + const value = memo.query(catSchema, [], entities, {}); expect(createOutput(value).results?.length).toBe(2); expect(createOutput(value).results).toMatchSnapshot(); - const value2 = memo.query('', catSchema, [], entities, {}); + const value2 = memo.query(catSchema, [], entities, {}); expect(createOutput(value).results[0]).toBe( createOutput(value2).results[0], ); @@ -208,7 +206,7 @@ describe.each([ 3: { id: '3', name: 'Jelico' }, }, }; - const value3 = memo.query('', catSchema, [], entities, {}); + const value3 = memo.query(catSchema, [], entities, {}); expect(createOutput(value3).results?.length).toBe(3); expect(createOutput(value3).results).toMatchSnapshot(); expect(createOutput(value).results[0]).toBe( @@ -231,7 +229,6 @@ describe.each([ }; const value = new MemoCache().query( - '', catSchema, [], createInput(entities), @@ -267,7 +264,6 @@ describe.each([ }, }; const value = new MemoCache().query( - '', listSchema, [], createInput(entities), @@ -331,7 +327,6 @@ describe.each([ }, }; const value = new MemoCache().query( - '', listSchema, [], createInput(entities) as any, diff --git a/packages/endpoint/src/schemas/__tests__/Collection.test.ts b/packages/endpoint/src/schemas/__tests__/Collection.test.ts index b6e910f1def0..d8925e03d052 100644 --- a/packages/endpoint/src/schemas/__tests__/Collection.test.ts +++ b/packages/endpoint/src/schemas/__tests__/Collection.test.ts @@ -518,7 +518,6 @@ describe(`${schema.Collection.name} denormalization`, () => { it('should buildQueryKey with matching args', () => { const memo = new MemoCache(); const queryKey = memo.buildQueryKey( - '', userTodos, [{ userId: '1' }], normalizeNested.entities, @@ -543,7 +542,6 @@ describe(`${schema.Collection.name} denormalization`, () => { it('should buildQueryKey undefined when not in cache', () => { const memo = new MemoCache(); const queryKey = memo.buildQueryKey( - '', userTodos, [{ userId: '100' }], normalizeNested.entities, @@ -555,7 +553,6 @@ describe(`${schema.Collection.name} denormalization`, () => { it('should buildQueryKey undefined with nested Collection', () => { const memo = new MemoCache(); const queryKey = memo.buildQueryKey( - '', User.schema.todos, [{ userId: '1' }], normalizeNested.entities, diff --git a/packages/endpoint/src/schemas/__tests__/Query.test.ts b/packages/endpoint/src/schemas/__tests__/Query.test.ts index 40a33fe005e8..7eae1389a043 100644 --- a/packages/endpoint/src/schemas/__tests__/Query.test.ts +++ b/packages/endpoint/src/schemas/__tests__/Query.test.ts @@ -72,7 +72,7 @@ describe.each([ }, }; const users: DenormalizeNullable | symbol = - new MemoCache().query('', sortedUsers, [], createInput(entities), {}); + new MemoCache().query(sortedUsers, [], createInput(entities), {}); expect(users).not.toEqual(expect.any(Symbol)); if (typeof users === 'symbol') return; expect(users && users[0].name).toBe('Zeta'); @@ -95,7 +95,6 @@ describe.each([ }; expect( new MemoCache().query( - '', sortedUsers, [{ asc: true }], createInput(entities), @@ -111,7 +110,7 @@ describe.each([ 2: { id: '2', name: 'Jake' }, }, }; - const data = new MemoCache().query('', sortedUsers, [], entities, {}); + const data = new MemoCache().query(sortedUsers, [], entities, {}); expect(createOutput(data)).toEqual(undefined); }); @@ -147,7 +146,6 @@ describe.each([ const totalCount: | DenormalizeNullable | symbol = new MemoCache().query( - '', userCountByAdmin, [], createInput(entities), @@ -158,7 +156,6 @@ describe.each([ const nonAdminCount: | DenormalizeNullable | symbol = new MemoCache().query( - '', userCountByAdmin, [{ isAdmin: false }], createInput(entities), @@ -168,7 +165,6 @@ describe.each([ const adminCount: | DenormalizeNullable | symbol = new MemoCache().query( - '', userCountByAdmin, [{ isAdmin: true }], createInput(entities), @@ -208,7 +204,7 @@ describe('top level schema', () => { [new schema.Collection([User]).pk({}, undefined, '', [])]: [1, 2, 3, 4], }, }; - const users = new MemoCache().query('', sortedUsers, [], entities, {}); + const users = new MemoCache().query(sortedUsers, [], entities, {}); expect(users).not.toEqual(expect.any(Symbol)); if (typeof users === 'symbol') return; expect(users && users[0].name).toBe('Zeta'); @@ -224,7 +220,7 @@ describe('top level schema', () => { 4: { id: '4', name: 'Alpha' }, }, }; - const users = new MemoCache().query('', sortedUsers, [], entities, {}); + const users = new MemoCache().query(sortedUsers, [], entities, {}); expect(users).toBeUndefined(); }); @@ -239,7 +235,7 @@ describe('top level schema', () => { return sorted.reverse(); }, ); - const users = new MemoCache().query('', allSortedUsers, [], {}, {}); + const users = new MemoCache().query(allSortedUsers, [], {}, {}); expect(users).toBeUndefined(); }); @@ -254,7 +250,7 @@ describe('top level schema', () => { return sorted.reverse(); }, ); - const users = new MemoCache().query('', allSortedUsers, [], {}, {}); + const users = new MemoCache().query(allSortedUsers, [], {}, {}); expect(users).toBeUndefined(); }); @@ -266,7 +262,7 @@ describe('top level schema', () => { }, }; - const value = new MemoCache().query('', sortedUsers, [], entities, {}); + const value = new MemoCache().query(sortedUsers, [], entities, {}); expect(value).toEqual(undefined); }); diff --git a/packages/normalizr/README.md b/packages/normalizr/README.md index a53a084ae334..c1235d50c074 100644 --- a/packages/normalizr/README.md +++ b/packages/normalizr/README.md @@ -216,15 +216,15 @@ const memo = new MemoCache(); const { data, paths } = memo.denormalize(schema, input, state.entities, args); -const data = memo.query(schema, key, args, state.entities, state.indexes); +const data = memo.query(schema, args, state.entities, state.indexes); -function query(schema, key, args, state) { +function query(schema, args, state, key) { const queryKey = memo.buildQueryKey( - key, schema, args, state.entities, state.indexes, + key, ); const { data } = this.denormalize(schema, queryKey, state.entities, args); return typeof data === 'symbol' ? undefined : (data as any); diff --git a/packages/normalizr/src/__tests__/MemoCache.ts b/packages/normalizr/src/__tests__/MemoCache.ts index 51e169a48e73..2d30d95194ed 100644 --- a/packages/normalizr/src/__tests__/MemoCache.ts +++ b/packages/normalizr/src/__tests__/MemoCache.ts @@ -626,7 +626,6 @@ describe('MemoCache', () => { }); expect( new MemoCache().buildQueryKey( - '', schema, [{ id: 5 }], { @@ -647,7 +646,6 @@ describe('MemoCache', () => { }); expect( new MemoCache().buildQueryKey( - '', schema, [5], { @@ -668,7 +666,6 @@ describe('MemoCache', () => { }); expect( new MemoCache().buildQueryKey( - '', schema, ['5'], { @@ -687,7 +684,6 @@ describe('MemoCache', () => { }; expect( new MemoCache().buildQueryKey( - '', schema, [{ id: 5 }], { @@ -704,7 +700,6 @@ describe('MemoCache', () => { }; expect( new MemoCache().buildQueryKey( - '', schema2, [{ id: 5 }], { @@ -723,7 +718,6 @@ describe('MemoCache', () => { }; expect( new MemoCache().buildQueryKey( - '', schema, [{ id: 5 }], { @@ -740,7 +734,6 @@ describe('MemoCache', () => { const schema = UnionResource.get.schema; expect( new MemoCache().buildQueryKey( - '', schema, [{ id: 5 }], { @@ -757,7 +750,6 @@ describe('MemoCache', () => { const schema = UnionResource.get.schema; expect( new MemoCache().buildQueryKey( - '', schema, [{ id: 5, type: 'first' }], { @@ -782,7 +774,6 @@ describe('MemoCache', () => { }; expect( new MemoCache().buildQueryKey( - '', schema, [{ id: 5 }], { @@ -805,7 +796,6 @@ describe('MemoCache', () => { }; expect( new MemoCache().buildQueryKey( - '', schema, [{ username: 'bob' }], { @@ -827,7 +817,6 @@ describe('MemoCache', () => { }); expect( new MemoCache().buildQueryKey( - '', schema, [{ username: 'bob', mary: 'five' }], { @@ -856,7 +845,6 @@ describe('MemoCache', () => { }; expect( new MemoCache().buildQueryKey( - '', schema, [{ username: 'bob' }], { @@ -878,7 +866,6 @@ describe('MemoCache', () => { }); expect( new MemoCache().buildQueryKey( - '', schema, [{ hover: 'bob' }], { @@ -907,7 +894,6 @@ describe('MemoCache', () => { }; expect( new MemoCache().buildQueryKey( - '', schema, [{ username: 'bob' }], { @@ -921,7 +907,6 @@ describe('MemoCache', () => { }); expect( new MemoCache().buildQueryKey( - '', schema, [{ hover: 'bob' }], { @@ -964,7 +949,6 @@ describe('MemoCache', () => { }); expect( new MemoCache().buildQueryKey( - '', schema, ['5'], { @@ -984,12 +968,11 @@ describe('MemoCache', () => { }), }); const memo = new MemoCache(); - expect(memo.buildQueryKey('', schema, ['5'], {}, {})).toEqual({ + expect(memo.buildQueryKey(schema, ['5'], {}, {})).toEqual({ data: { article: undefined }, }); expect( memo.buildQueryKey( - '', schema, ['5'], { [MyEntity.key]: { '5': { id: '5', title: 'hi' } } }, @@ -1012,7 +995,6 @@ describe('MemoCache', () => { indexes: {}, }; const first = memo.buildQueryKey( - '', schema, ['5'], state.entities, @@ -1020,25 +1002,18 @@ describe('MemoCache', () => { ); it('should maintain referential equality', () => { expect( - memo.buildQueryKey( - '', - schema, - ['5'], - state.entities, - state.indexes, - ), + memo.buildQueryKey(schema, ['5'], state.entities, state.indexes), ).toBe(first); }); it('should not change on index update if not used', () => { expect( - memo.buildQueryKey('', schema, ['5'], state.entities, { + memo.buildQueryKey(schema, ['5'], state.entities, { [MyEntity.key]: {}, }), ).toBe(first); }); it('should be new when entity is updated', () => { const withEntity = memo.buildQueryKey( - '', schema, ['5'], { [MyEntity.key]: { '5': { id: '5', title: 'hi' } } }, @@ -1049,7 +1024,6 @@ describe('MemoCache', () => { }); it('should be the same if other entities are updated', () => { const withEntity = memo.buildQueryKey( - '', schema, ['5'], { @@ -1062,7 +1036,6 @@ describe('MemoCache', () => { }); it('should be the same if other entities of the same type are updated', () => { const withEntity = memo.buildQueryKey( - '', schema, ['5'], { @@ -1081,7 +1054,7 @@ describe('MemoCache', () => { }); describe.each([ - ['direct', (data: T) => data, (data: T) => data], + ['direct', (data: T) => data, (data: T) => data], [ 'immutable', fromJS, @@ -1108,7 +1081,6 @@ describe('MemoCache', () => { test('works with indexes', () => { const m = new MemoCache().query( - '', Cat, [{ username: 'm' }], createInput(entities), @@ -1118,7 +1090,6 @@ describe('MemoCache', () => { expect(m).toMatchSnapshot(); expect( new MemoCache().query( - '', Cat, [{ username: 'doesnotexist' }], createInput(entities), @@ -1129,7 +1100,6 @@ describe('MemoCache', () => { test('works with pk', () => { const m = new MemoCache().query( - '', Cat, [{ id: '1' }], createInput(entities), @@ -1139,7 +1109,6 @@ describe('MemoCache', () => { expect(m).toMatchSnapshot(); expect( new MemoCache().query( - '', Cat, [{ id: 'doesnotexist' }], createInput(entities), diff --git a/packages/normalizr/src/memo/MemoCache.ts b/packages/normalizr/src/memo/MemoCache.ts index dfc0cc7a1694..9dc857fc72f9 100644 --- a/packages/normalizr/src/memo/MemoCache.ts +++ b/packages/normalizr/src/memo/MemoCache.ts @@ -55,7 +55,6 @@ export default class MemoCache { /** Compute denormalized form maintaining referential equality for same inputs */ query( - argsKey: string, schema: S, args: any[], entities: @@ -68,8 +67,10 @@ export default class MemoCache { | { getIn(k: string[]): any; }, + // NOTE: different orders can result in cache busting here; but since it's just a perf penalty we will allow for now + argsKey: string = JSON.stringify(args), ): DenormalizeNullable | undefined { - const input = this.buildQueryKey(argsKey, schema, args, entities, indexes); + const input = this.buildQueryKey(schema, args, entities, indexes, argsKey); if (!input) { return; @@ -80,7 +81,6 @@ export default class MemoCache { } buildQueryKey( - argsKey: string, schema: S, args: any[], entities: @@ -93,6 +93,8 @@ export default class MemoCache { | { getIn(k: string[]): any; }, + // NOTE: different orders can result in cache busting here; but since it's just a perf penalty we will allow for now + argsKey: string = JSON.stringify(args), ): NormalizeNullable { // This is redundant for buildQueryKey checks, but that was is used for recursion so we still need the checks there // TODO: If we make each recursive call include cache lookups, we combine these checks together diff --git a/website/src/components/Playground/editor-types/@data-client/core.d.ts b/website/src/components/Playground/editor-types/@data-client/core.d.ts index 94b3eeded30e..aa341b23801a 100644 --- a/website/src/components/Playground/editor-types/@data-client/core.d.ts +++ b/website/src/components/Playground/editor-types/@data-client/core.d.ts @@ -149,16 +149,16 @@ declare class MemoCache { paths: EntityPath[]; }; /** Compute denormalized form maintaining referential equality for same inputs */ - query(argsKey: string, schema: S, args: any[], entities: Record> | { + query(schema: S, args: any[], entities: Record> | { getIn(k: string[]): any; }, indexes: NormalizedIndex | { getIn(k: string[]): any; - }): DenormalizeNullable | undefined; - buildQueryKey(argsKey: string, schema: S, args: any[], entities: Record> | { + }, argsKey?: string): DenormalizeNullable | undefined; + buildQueryKey(schema: S, args: any[], entities: Record> | { getIn(k: string[]): any; }, indexes: NormalizedIndex | { getIn(k: string[]): any; - }): NormalizeNullable; + }, argsKey?: string): NormalizeNullable; } type IndexPath = [key: string, field: string, value: string]; type EntitySchemaPath = [key: string] | [key: string, pk: string]; diff --git a/website/src/components/Playground/editor-types/@data-client/normalizr.d.ts b/website/src/components/Playground/editor-types/@data-client/normalizr.d.ts index d8a5382fe11f..3175d698f6c2 100644 --- a/website/src/components/Playground/editor-types/@data-client/normalizr.d.ts +++ b/website/src/components/Playground/editor-types/@data-client/normalizr.d.ts @@ -205,16 +205,16 @@ declare class MemoCache { paths: EntityPath[]; }; /** Compute denormalized form maintaining referential equality for same inputs */ - query(argsKey: string, schema: S, args: any[], entities: Record> | { + query(schema: S, args: any[], entities: Record> | { getIn(k: string[]): any; }, indexes: NormalizedIndex | { getIn(k: string[]): any; - }): DenormalizeNullable | undefined; - buildQueryKey(argsKey: string, schema: S, args: any[], entities: Record> | { + }, argsKey?: string): DenormalizeNullable | undefined; + buildQueryKey(schema: S, args: any[], entities: Record> | { getIn(k: string[]): any; }, indexes: NormalizedIndex | { getIn(k: string[]): any; - }): NormalizeNullable; + }, argsKey?: string): NormalizeNullable; } type IndexPath = [key: string, field: string, value: string]; type EntitySchemaPath = [key: string] | [key: string, pk: string];