From 724b078d3acc4291d30871a1482eacec4fdd7281 Mon Sep 17 00:00:00 2001 From: PatrykWalach <35966385+PatrykWalach@users.noreply.github.com> Date: Sun, 23 Feb 2025 13:18:08 +0100 Subject: [PATCH 1/6] create optimistic proxy --- .../src/core/IsographEnvironment.ts | 6 + .../src/core/optimisticProxy.ts | 145 ++++++++++++++++++ 2 files changed, 151 insertions(+) create mode 100644 libs/isograph-react/src/core/optimisticProxy.ts diff --git a/libs/isograph-react/src/core/IsographEnvironment.ts b/libs/isograph-react/src/core/IsographEnvironment.ts index 0f2b2199d..d90907d50 100644 --- a/libs/isograph-react/src/core/IsographEnvironment.ts +++ b/libs/isograph-react/src/core/IsographEnvironment.ts @@ -8,6 +8,7 @@ import { } from './FragmentReference'; import { RetainedQuery } from './garbageCollection'; import { LogFunction, WrappedLogFunction } from './logging'; +import { createOptimisticProxy, type OptimisticLayer } from './optimisticProxy'; import { PromiseWrapper, wrapPromise } from './PromiseWrapper'; import { WithEncounteredRecords } from './read'; import type { ReaderAst, StartUpdate } from './reader'; @@ -52,6 +53,8 @@ export type CacheMap = { [index: string]: ParentCache }; export type IsographEnvironment = { readonly store: IsographStore; + readonly optimisticLayer: OptimisticLayer; + readonly optimisticStore: OptimisticLayer; readonly networkFunction: IsographNetworkFunction; readonly missingFieldHandler: MissingFieldHandler | null; readonly componentCache: FieldCache>; @@ -134,8 +137,11 @@ export function createIsographEnvironment( logFunction?.({ kind: 'EnvironmentCreated', }); + let optimisticLayer: OptimisticLayer = {}; return { store, + optimisticLayer, + optimisticStore: createOptimisticProxy(store, optimisticLayer), networkFunction, missingFieldHandler: missingFieldHandler ?? null, componentCache: {}, diff --git a/libs/isograph-react/src/core/optimisticProxy.ts b/libs/isograph-react/src/core/optimisticProxy.ts new file mode 100644 index 000000000..a863834b8 --- /dev/null +++ b/libs/isograph-react/src/core/optimisticProxy.ts @@ -0,0 +1,145 @@ +import type { + DataId, + IsographEnvironment, + IsographStore, + StoreRecord, + TypeName, +} from './IsographEnvironment'; + +type RecordsById = { + [index: DataId]: StoreRecord | null; +}; + +export function createOptimisticRecord( + storeRecord: StoreRecord, + optimisticStoreRecord: StoreRecord, +): StoreRecord { + return new Proxy(optimisticStoreRecord, { + get(target, p: string) { + const optimisticValue = target[p]; + + if (optimisticValue === undefined) { + return storeRecord[p]; + } + return optimisticValue; + }, + has(target, p) { + return Reflect.has(target, p) || Reflect.has(storeRecord, p); + }, + ownKeys(target) { + const merged = { + ...storeRecord, + ...target, + }; + return Reflect.ownKeys(merged); + }, + }); +} + +export function createOptimisticRecordsById( + recordsById: RecordsById, + optimisticRecordsById: RecordsById, +): RecordsById { + return new Proxy(optimisticRecordsById, { + get(target, p: string) { + let optimisticStoreRecord = target[p]; + + if (optimisticStoreRecord === null) { + return optimisticStoreRecord; + } + + const storeRecord = recordsById[p]; + + if (storeRecord == null) { + return storeRecord; + } + + optimisticStoreRecord = target[p] ??= {}; + + return createOptimisticRecord(storeRecord, optimisticStoreRecord); + }, + has(target, p) { + return Reflect.has(target, p) || Reflect.has(recordsById, p); + }, + ownKeys(target) { + const merged = { + ...recordsById, + ...target, + }; + return Reflect.ownKeys(merged); + }, + }); +} + +export function createOptimisticProxy( + store: IsographStore, + optimisticLayer: OptimisticLayer, +): OptimisticLayer { + return new Proxy(optimisticLayer, { + get(target, p: string) { + let optimisticRecordsById = target[p]; + + if (optimisticRecordsById === null) { + return optimisticRecordsById; + } + + const recordsById = store[p]; + + if (recordsById == null) { + return recordsById; + } + + optimisticRecordsById = target[p] ??= {}; + + return createOptimisticRecordsById(recordsById, optimisticRecordsById); + }, + has(target, p) { + return Reflect.has(target, p) || Reflect.has(store, p); + }, + ownKeys(target) { + const merged = { + ...store, + ...target, + }; + return Reflect.ownKeys(merged); + }, + }); +} + +export type OptimisticLayer = { + [index: TypeName]: { + [index: DataId]: StoreRecord | null; + } | null; +}; + +export function mergeOptimisticLayer(environment: IsographEnvironment): void { + for (const [typeName, patchById] of Object.entries( + environment.optimisticLayer, + )) { + let recordById = environment.store[typeName]; + + if (patchById === null) { + environment.store[typeName] = null; + continue; + } + recordById = environment.store[typeName] ??= {}; + + for (const [recordId, patch] of Object.entries(patchById)) { + const data = recordById[recordId]; + + if (patch == null || data == null) { + recordById[recordId] = patch; + } else { + Object.assign(data, patch); + } + } + } + + resetOptimisticLayer(environment); +} + +export function resetOptimisticLayer(environment: IsographEnvironment) { + for (const key in environment.optimisticLayer) { + delete environment.optimisticLayer[key]; + } +} From 9e75e6a4410ad32809595f69cc77ed3915e03423 Mon Sep 17 00:00:00 2001 From: PatrykWalach <35966385+PatrykWalach@users.noreply.github.com> Date: Sun, 23 Feb 2025 14:00:05 +0100 Subject: [PATCH 2/6] test optimisticProxy --- .../src/core/optimisticProxy.ts | 27 ++++++++ .../src/tests/optimisticProxy.test.ts | 62 +++++++++++++++++++ 2 files changed, 89 insertions(+) create mode 100644 libs/isograph-react/src/tests/optimisticProxy.test.ts diff --git a/libs/isograph-react/src/core/optimisticProxy.ts b/libs/isograph-react/src/core/optimisticProxy.ts index a863834b8..4a8b9a6a4 100644 --- a/libs/isograph-react/src/core/optimisticProxy.ts +++ b/libs/isograph-react/src/core/optimisticProxy.ts @@ -33,6 +33,15 @@ export function createOptimisticRecord( }; return Reflect.ownKeys(merged); }, + set(target, p: string, value: any) { + return Reflect.set(target, p, value); + }, + getOwnPropertyDescriptor(target, p: string) { + return ( + Reflect.getOwnPropertyDescriptor(target, p) ?? + Reflect.getOwnPropertyDescriptor(storeRecord, p) + ); + }, }); } @@ -68,6 +77,15 @@ export function createOptimisticRecordsById( }; return Reflect.ownKeys(merged); }, + set(target, p: string, value: any) { + return Reflect.set(target, p, value); + }, + getOwnPropertyDescriptor(target, p: string) { + return ( + Reflect.getOwnPropertyDescriptor(target, p) ?? + Reflect.getOwnPropertyDescriptor(recordsById, p) + ); + }, }); } @@ -103,6 +121,15 @@ export function createOptimisticProxy( }; return Reflect.ownKeys(merged); }, + set(target, p: string, value: any) { + return Reflect.set(target, p, value); + }, + getOwnPropertyDescriptor(target, p: string) { + return ( + Reflect.getOwnPropertyDescriptor(target, p) ?? + Reflect.getOwnPropertyDescriptor(store, p) + ); + }, }); } diff --git a/libs/isograph-react/src/tests/optimisticProxy.test.ts b/libs/isograph-react/src/tests/optimisticProxy.test.ts new file mode 100644 index 000000000..6dbd392e3 --- /dev/null +++ b/libs/isograph-react/src/tests/optimisticProxy.test.ts @@ -0,0 +1,62 @@ +import { beforeEach, describe, expect, test, vi } from 'vitest'; +import { + createIsographEnvironment, + type IsographStore, +} from '../core/IsographEnvironment'; +import { mergeOptimisticLayer } from '../core/optimisticProxy'; + +describe('optimisticProxy', () => { + let environment: ReturnType; + + beforeEach(() => { + const store: IsographStore = { + Query: { + __ROOT: {}, + }, + Economist: { + 0: { + __typename: 'Economist', + id: '0', + name: 'Jeremy Bentham', + successor: { __link: '1', __typename: 'Economist' }, + }, + }, + }; + const networkFunction = vi + .fn() + .mockRejectedValue(new Error('Fetch failed')); + environment = createIsographEnvironment(store, networkFunction); + }); + + test('is equal to store', () => { + expect(environment.optimisticStore.Economist?.[0]).toStrictEqual({ + __typename: 'Economist', + id: '0', + name: 'Jeremy Bentham', + successor: { __link: '1', __typename: 'Economist' }, + }); + }); + + test('writes update optimistic layer', () => { + environment.optimisticStore.Economist![0]!.name = 'Updated Jeremy Bentham'; + + expect(environment.optimisticLayer).toStrictEqual({ + Economist: { + 0: { name: 'Updated Jeremy Bentham' }, + }, + }); + }); + + test('mergeOptimisticLayer', () => { + environment.optimisticStore.Economist![0]!.name = 'Updated Jeremy Bentham'; + + mergeOptimisticLayer(environment); + expect(environment.optimisticLayer).toStrictEqual({}); + expect(environment.optimisticStore.Economist?.[0]).toStrictEqual({ + __typename: 'Economist', + id: '0', + name: 'Updated Jeremy Bentham', + successor: { __link: '1', __typename: 'Economist' }, + }); + }); +}); From 25a0722ef5004f3e5b8df4aa893ef214f1339a6e Mon Sep 17 00:00:00 2001 From: PatrykWalach <35966385+PatrykWalach@users.noreply.github.com> Date: Sun, 23 Feb 2025 14:05:30 +0100 Subject: [PATCH 3/6] refactor test --- .../src/tests/optimisticProxy.test.ts | 36 ++++++++++++++----- 1 file changed, 28 insertions(+), 8 deletions(-) diff --git a/libs/isograph-react/src/tests/optimisticProxy.test.ts b/libs/isograph-react/src/tests/optimisticProxy.test.ts index 6dbd392e3..ec0936fd3 100644 --- a/libs/isograph-react/src/tests/optimisticProxy.test.ts +++ b/libs/isograph-react/src/tests/optimisticProxy.test.ts @@ -18,7 +18,6 @@ describe('optimisticProxy', () => { __typename: 'Economist', id: '0', name: 'Jeremy Bentham', - successor: { __link: '1', __typename: 'Economist' }, }, }, }; @@ -33,7 +32,16 @@ describe('optimisticProxy', () => { __typename: 'Economist', id: '0', name: 'Jeremy Bentham', - successor: { __link: '1', __typename: 'Economist' }, + }); + }); + + test('writes update proxy', () => { + environment.optimisticStore.Economist![0]!.name = 'Updated Jeremy Bentham'; + + expect(environment.optimisticStore.Economist![0]).toStrictEqual({ + __typename: 'Economist', + id: '0', + name: 'Updated Jeremy Bentham', }); }); @@ -47,16 +55,28 @@ describe('optimisticProxy', () => { }); }); - test('mergeOptimisticLayer', () => { + test('writes keep store intact', () => { environment.optimisticStore.Economist![0]!.name = 'Updated Jeremy Bentham'; - mergeOptimisticLayer(environment); - expect(environment.optimisticLayer).toStrictEqual({}); - expect(environment.optimisticStore.Economist?.[0]).toStrictEqual({ + expect(environment.store.Economist?.[0]).toStrictEqual({ __typename: 'Economist', id: '0', - name: 'Updated Jeremy Bentham', - successor: { __link: '1', __typename: 'Economist' }, + name: 'Jeremy Bentham', + }); + }); + + describe('mergeOptimisticLayer', () => { + test('merges optimistic layer with store', () => { + environment.optimisticStore.Economist![0]!.name = + 'Updated Jeremy Bentham'; + + mergeOptimisticLayer(environment); + expect(environment.optimisticLayer).toStrictEqual({}); + expect(environment.optimisticStore.Economist?.[0]).toStrictEqual({ + __typename: 'Economist', + id: '0', + name: 'Updated Jeremy Bentham', + }); }); }); }); From 404a67434136393c9c75c9d9e1e572989421f631 Mon Sep 17 00:00:00 2001 From: PatrykWalach <35966385+PatrykWalach@users.noreply.github.com> Date: Sun, 23 Feb 2025 15:20:26 +0100 Subject: [PATCH 4/6] refactor proxy All the proxies work in the same way: if value present in `A` return value otherwise return value from `B` --- .../src/core/optimisticProxy.ts | 132 ++++++------------ 1 file changed, 40 insertions(+), 92 deletions(-) diff --git a/libs/isograph-react/src/core/optimisticProxy.ts b/libs/isograph-react/src/core/optimisticProxy.ts index 4a8b9a6a4..69ba8f77b 100644 --- a/libs/isograph-react/src/core/optimisticProxy.ts +++ b/libs/isograph-react/src/core/optimisticProxy.ts @@ -6,73 +6,39 @@ import type { TypeName, } from './IsographEnvironment'; -type RecordsById = { - [index: DataId]: StoreRecord | null; -}; - -export function createOptimisticRecord( - storeRecord: StoreRecord, - optimisticStoreRecord: StoreRecord, -): StoreRecord { - return new Proxy(optimisticStoreRecord, { - get(target, p: string) { - const optimisticValue = target[p]; - - if (optimisticValue === undefined) { - return storeRecord[p]; - } - return optimisticValue; - }, - has(target, p) { - return Reflect.has(target, p) || Reflect.has(storeRecord, p); - }, - ownKeys(target) { - const merged = { - ...storeRecord, - ...target, - }; - return Reflect.ownKeys(merged); - }, - set(target, p: string, value: any) { - return Reflect.set(target, p, value); - }, - getOwnPropertyDescriptor(target, p: string) { - return ( - Reflect.getOwnPropertyDescriptor(target, p) ?? - Reflect.getOwnPropertyDescriptor(storeRecord, p) - ); - }, - }); -} - -export function createOptimisticRecordsById( - recordsById: RecordsById, - optimisticRecordsById: RecordsById, -): RecordsById { - return new Proxy(optimisticRecordsById, { +function createLayerProxy( + object: { + [key: string]: T | null; + }, + optimisticObject: { + [key: string]: T | null; + }, + getter: (value: T, optimisticValue: T | undefined, p: string) => T, +): { + [key: string]: T | null; +} { + return new Proxy(optimisticObject, { get(target, p: string) { - let optimisticStoreRecord = target[p]; + let optimisticValue = target[p]; - if (optimisticStoreRecord === null) { - return optimisticStoreRecord; + if (optimisticValue === null) { + return optimisticValue; } - const storeRecord = recordsById[p]; + const value = object[p]; - if (storeRecord == null) { - return storeRecord; + if (value == null) { + return value; } - optimisticStoreRecord = target[p] ??= {}; - - return createOptimisticRecord(storeRecord, optimisticStoreRecord); + return getter(value, optimisticValue, p); }, has(target, p) { - return Reflect.has(target, p) || Reflect.has(recordsById, p); + return Reflect.has(target, p) || Reflect.has(object, p); }, ownKeys(target) { const merged = { - ...recordsById, + ...object, ...target, }; return Reflect.ownKeys(merged); @@ -83,7 +49,7 @@ export function createOptimisticRecordsById( getOwnPropertyDescriptor(target, p: string) { return ( Reflect.getOwnPropertyDescriptor(target, p) ?? - Reflect.getOwnPropertyDescriptor(recordsById, p) + Reflect.getOwnPropertyDescriptor(object, p) ); }, }); @@ -93,44 +59,26 @@ export function createOptimisticProxy( store: IsographStore, optimisticLayer: OptimisticLayer, ): OptimisticLayer { - return new Proxy(optimisticLayer, { - get(target, p: string) { - let optimisticRecordsById = target[p]; - - if (optimisticRecordsById === null) { - return optimisticRecordsById; - } - - const recordsById = store[p]; - - if (recordsById == null) { - return recordsById; - } - - optimisticRecordsById = target[p] ??= {}; - - return createOptimisticRecordsById(recordsById, optimisticRecordsById); - }, - has(target, p) { - return Reflect.has(target, p) || Reflect.has(store, p); - }, - ownKeys(target) { - const merged = { - ...store, - ...target, - }; - return Reflect.ownKeys(merged); - }, - set(target, p: string, value: any) { - return Reflect.set(target, p, value); - }, - getOwnPropertyDescriptor(target, p: string) { - return ( - Reflect.getOwnPropertyDescriptor(target, p) ?? - Reflect.getOwnPropertyDescriptor(store, p) + return createLayerProxy( + store, + optimisticLayer, + (recordsById, optimisticRecordsById, p) => { + optimisticRecordsById = optimisticLayer[p] ??= {}; + return createLayerProxy( + recordsById, + optimisticRecordsById, + (storeRecord, optimisticStoreRecord, p) => { + optimisticStoreRecord = optimisticRecordsById[p] ??= {}; + return createLayerProxy( + storeRecord, + optimisticStoreRecord, + (value, optimisticValue) => + optimisticValue === undefined ? value : optimisticValue, + ); + }, ); }, - }); + ); } export type OptimisticLayer = { From d53aac4fd87ad9d2aa8743b431cf42fd3ee25a90 Mon Sep 17 00:00:00 2001 From: PatrykWalach <35966385+PatrykWalach@users.noreply.github.com> Date: Sun, 23 Feb 2025 15:36:09 +0100 Subject: [PATCH 5/6] fix logic error --- libs/isograph-react/src/core/optimisticProxy.ts | 13 +++++++++---- .../src/tests/optimisticProxy.test.ts | 16 ++++++++++++++++ 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/libs/isograph-react/src/core/optimisticProxy.ts b/libs/isograph-react/src/core/optimisticProxy.ts index 69ba8f77b..a3992f036 100644 --- a/libs/isograph-react/src/core/optimisticProxy.ts +++ b/libs/isograph-react/src/core/optimisticProxy.ts @@ -1,5 +1,6 @@ import type { DataId, + DataTypeValue, IsographEnvironment, IsographStore, StoreRecord, @@ -13,7 +14,11 @@ function createLayerProxy( optimisticObject: { [key: string]: T | null; }, - getter: (value: T, optimisticValue: T | undefined, p: string) => T, + getter: ( + value: T | null | undefined, + optimisticValue: T | undefined, + p: string, + ) => T | null | undefined, ): { [key: string]: T | null; } { @@ -27,7 +32,7 @@ function createLayerProxy( const value = object[p]; - if (value == null) { + if (optimisticValue === undefined && value == null) { return value; } @@ -65,12 +70,12 @@ export function createOptimisticProxy( (recordsById, optimisticRecordsById, p) => { optimisticRecordsById = optimisticLayer[p] ??= {}; return createLayerProxy( - recordsById, + recordsById ?? {}, optimisticRecordsById, (storeRecord, optimisticStoreRecord, p) => { optimisticStoreRecord = optimisticRecordsById[p] ??= {}; return createLayerProxy( - storeRecord, + storeRecord ?? {}, optimisticStoreRecord, (value, optimisticValue) => optimisticValue === undefined ? value : optimisticValue, diff --git a/libs/isograph-react/src/tests/optimisticProxy.test.ts b/libs/isograph-react/src/tests/optimisticProxy.test.ts index ec0936fd3..1de2e0f66 100644 --- a/libs/isograph-react/src/tests/optimisticProxy.test.ts +++ b/libs/isograph-react/src/tests/optimisticProxy.test.ts @@ -65,6 +65,22 @@ describe('optimisticProxy', () => { }); }); + test('reads from optimistic layer if record is undefined', () => { + environment.optimisticLayer.Economist = { + 1: { + __typename: 'Economist', + id: '1', + name: 'John Stuart Mill', + }, + }; + + expect(environment.optimisticStore.Economist![1]).toStrictEqual({ + __typename: 'Economist', + id: '1', + name: 'John Stuart Mill', + }); + }); + describe('mergeOptimisticLayer', () => { test('merges optimistic layer with store', () => { environment.optimisticStore.Economist![0]!.name = From 1b76c61d3c3379ac02f2480dc421d2558853782e Mon Sep 17 00:00:00 2001 From: PatrykWalach <35966385+PatrykWalach@users.noreply.github.com> Date: Sun, 23 Feb 2025 15:39:03 +0100 Subject: [PATCH 6/6] remove unused import --- libs/isograph-react/src/core/optimisticProxy.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/libs/isograph-react/src/core/optimisticProxy.ts b/libs/isograph-react/src/core/optimisticProxy.ts index a3992f036..c3366a0dc 100644 --- a/libs/isograph-react/src/core/optimisticProxy.ts +++ b/libs/isograph-react/src/core/optimisticProxy.ts @@ -1,6 +1,5 @@ import type { DataId, - DataTypeValue, IsographEnvironment, IsographStore, StoreRecord,