From fb66f0bd8552d50286d0e91385b09913fe66e460 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Sat, 8 Jun 2024 12:42:13 +0100 Subject: [PATCH 01/31] Improve connect typings --- lib/OnyxUtils.ts | 3 +-- lib/types.ts | 35 +++++++++++++++++------------------ lib/withOnyx/index.tsx | 6 +++--- 3 files changed, 21 insertions(+), 23 deletions(-) diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index 2e8fbda6..fe9756de 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -25,7 +25,6 @@ import type { OnyxKey, OnyxValue, Selector, - WithOnyxConnectOptions, } from './types'; import utils from './utils'; import type {WithOnyxState} from './withOnyx/types'; @@ -305,7 +304,7 @@ function isSafeEvictionKey(testKey: OnyxKey): boolean { * Tries to get a value from the cache. If the value is not present in cache it will return the default value or undefined. * If the requested key is a collection, it will return an object with all the collection members. */ -function tryGetCachedValue(key: TKey, mapping?: Partial>): OnyxValue { +function tryGetCachedValue(key: TKey, mapping?: Partial>): OnyxValue { let val = cache.get(key); if (isCollectionKey(key)) { diff --git a/lib/types.ts b/lib/types.ts index f7e491cc..4c7ea8f8 100644 --- a/lib/types.ts +++ b/lib/types.ts @@ -265,29 +265,21 @@ type BaseConnectOptions = { initWithStoredValues?: boolean; }; -/** Represents additional options used inside withOnyx HOC */ -type WithOnyxConnectOptions = { - withOnyxInstance: WithOnyxInstance; - statePropertyName: string; - displayName: string; - initWithStoredValues?: boolean; - selector?: Selector; - canEvict?: boolean; -}; - +/** Represents the callback function used in `Onyx.connect()` method with a regular key. */ type DefaultConnectCallback = (value: OnyxEntry, key: TKey) => void; +/** Represents the callback function used in `Onyx.connect()` method with a collection key. */ type CollectionConnectCallback = (value: NonUndefined>) => void; -/** Represents the callback function used in `Onyx.connect()` method with a regular key. */ -type DefaultConnectOptions = { +/** Represents the options used in `Onyx.connect()` method with a regular key. */ +type DefaultConnectOptions = BaseConnectOptions & { key: TKey; callback?: DefaultConnectCallback; waitForCollectionCallback?: false; }; -/** Represents the callback function used in `Onyx.connect()` method with a collection key. */ -type CollectionConnectOptions = { +/** Represents the options used in `Onyx.connect()` method with a collection key. */ +type CollectionConnectOptions = BaseConnectOptions & { key: TKey extends CollectionKeyBase ? TKey : never; callback?: CollectionConnectCallback; waitForCollectionCallback: true; @@ -303,12 +295,19 @@ type CollectionConnectOptions = { * * If `waitForCollectionCallback` is `false` or not specified, the `key` can be any Onyx key and `callback` will be triggered with updates of each collection item * and will pass `value` as an `OnyxEntry`. - * - * The type is also extended with `BaseConnectOptions` and `WithOnyxConnectOptions` to include additional options, depending on the context where it's used. */ -type ConnectOptions = (CollectionConnectOptions | DefaultConnectOptions) & (BaseConnectOptions | WithOnyxConnectOptions); +type ConnectOptions = DefaultConnectOptions | CollectionConnectOptions; + +/** Represents additional `Onyx.connect()` options used inside withOnyx HOC. */ +type WithOnyxConnectOptions = ConnectOptions & { + withOnyxInstance: WithOnyxInstance; + statePropertyName: string; + displayName: string; + selector?: Selector; + canEvict?: boolean; +}; -type Mapping = ConnectOptions & { +type Mapping = WithOnyxConnectOptions & { connectionID: number; }; diff --git a/lib/withOnyx/index.tsx b/lib/withOnyx/index.tsx index fdc5bed5..77f26930 100644 --- a/lib/withOnyx/index.tsx +++ b/lib/withOnyx/index.tsx @@ -7,7 +7,7 @@ import React from 'react'; import Onyx from '../Onyx'; import OnyxUtils from '../OnyxUtils'; import * as Str from '../Str'; -import type {GenericFunction, OnyxKey, WithOnyxConnectOptions} from '../types'; +import type {GenericFunction, Mapping, OnyxKey, WithOnyxConnectOptions} from '../types'; import utils from '../utils'; import type {MapOnyxToState, WithOnyxInstance, WithOnyxMapping, WithOnyxProps, WithOnyxState} from './types'; import cache from '../OnyxCache'; @@ -84,7 +84,7 @@ export default function ( const cachedState = mapOnyxToStateEntries(mapOnyxToState).reduce>((resultObj, [propName, mapping]) => { const key = Str.result(mapping.key as GenericFunction, props); - let value = OnyxUtils.tryGetCachedValue(key, mapping as Partial>); + let value = OnyxUtils.tryGetCachedValue(key, mapping as Mapping); const hasCacheForKey = cache.hasCacheForKey(key); if (!hasCacheForKey && !value && mapping.initialValue) { @@ -325,7 +325,7 @@ export default function ( statePropertyName: statePropertyName as string, withOnyxInstance: this as unknown as WithOnyxInstance, displayName, - }); + } as WithOnyxConnectOptions); } flushPendingSetStates() { From 65edb35b89feb6f37bf1cee6ca8178a01011b507 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Tue, 11 Jun 2024 15:47:44 +0100 Subject: [PATCH 02/31] Implement first version of OnyxConnectionManager --- lib/OnyxConnectionManager.ts | 105 +++++++++++++++++++++++++++++++++++ 1 file changed, 105 insertions(+) create mode 100644 lib/OnyxConnectionManager.ts diff --git a/lib/OnyxConnectionManager.ts b/lib/OnyxConnectionManager.ts new file mode 100644 index 00000000..ff211114 --- /dev/null +++ b/lib/OnyxConnectionManager.ts @@ -0,0 +1,105 @@ +import bindAll from 'lodash/bindAll'; +import type {ConnectOptions} from './Onyx'; +import Onyx from './Onyx'; +import type {OnyxKey} from './types'; + +type ConnectCallback = () => void; + +type Connection = { + id: number; + onyxKey: OnyxKey; + isConnectionMade: boolean; + callbacks: Map; +}; + +class OnyxConnectionManager { + private connectionsMap: Map; + + private lastCallbackID: number; + + constructor() { + this.connectionsMap = new Map(); + this.lastCallbackID = 0; + + bindAll(this, 'fireCallbacks', 'connectionMapKey', 'connect', 'disconnect', 'disconnectAll'); + } + + private fireCallbacks(connectOptions: ConnectOptions): void { + const mapKey = this.connectionMapKey(connectOptions); + const connection = this.connectionsMap.get(mapKey); + + connection?.callbacks.forEach((callback) => { + callback(); + }); + } + + private connectionMapKey(connectOptions: ConnectOptions): string { + return `key=${connectOptions.key},initWithStoredValues=${connectOptions.initWithStoredValues ?? true},waitForCollectionCallback=${connectOptions.waitForCollectionCallback ?? false}`; + } + + connect(connectOptions: ConnectOptions): [number, string, string] { + const mapKey = this.connectionMapKey(connectOptions); + let connection = this.connectionsMap.get(mapKey); + let connectionID: number | undefined; + + const callbackID = String(this.lastCallbackID++); + + if (!connection) { + connectionID = Onyx.connect({ + ...connectOptions, + callback: () => { + const createdConnection = this.connectionsMap.get(mapKey); + if (createdConnection) { + createdConnection.isConnectionMade = true; + } + + this.fireCallbacks(connectOptions); + }, + }); + + connection = { + id: connectionID, + onyxKey: connectOptions.key, + isConnectionMade: false, + callbacks: new Map(), + }; + + this.connectionsMap.set(mapKey, connection); + } + + connection.callbacks.set(callbackID, connectOptions.callback as ConnectCallback); + + if (connection.isConnectionMade) { + (connectOptions.callback as ConnectCallback)(); + } + + return [connection.id, mapKey, callbackID]; + } + + disconnect(mapKey: string, callbackID: string): void { + const connection = this.connectionsMap.get(mapKey); + + if (!connection) { + return; + } + + connection.callbacks.delete(callbackID); + + if (connection.callbacks.size === 0) { + Onyx.disconnect(connection.id); + this.connectionsMap.delete(mapKey); + } + } + + disconnectAll(): void { + Array.from(this.connectionsMap.values()).forEach((connection) => { + Onyx.disconnect(connection.id); + }); + + this.connectionsMap.clear(); + } +} + +const connectionManager = new OnyxConnectionManager(); + +export default connectionManager; From 8db2eed4480547ba6e9fbfb3c5ea0480524021ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Mon, 1 Jul 2024 16:13:05 +0100 Subject: [PATCH 03/31] Add value support to connection manager callback and tests --- lib/OnyxConnectionManager.ts | 35 +++--- lib/useOnyx.ts | 12 +- tests/unit/OnyxConnectionManagerTest.ts | 143 ++++++++++++++++++++++++ 3 files changed, 169 insertions(+), 21 deletions(-) create mode 100644 tests/unit/OnyxConnectionManagerTest.ts diff --git a/lib/OnyxConnectionManager.ts b/lib/OnyxConnectionManager.ts index ff211114..20a81a73 100644 --- a/lib/OnyxConnectionManager.ts +++ b/lib/OnyxConnectionManager.ts @@ -1,15 +1,17 @@ import bindAll from 'lodash/bindAll'; import type {ConnectOptions} from './Onyx'; import Onyx from './Onyx'; -import type {OnyxKey} from './types'; +import type {DefaultConnectCallback, DefaultConnectOptions, OnyxKey, OnyxValue} from './types'; -type ConnectCallback = () => void; +type ConnectCallback = DefaultConnectCallback; type Connection = { id: number; onyxKey: OnyxKey; isConnectionMade: boolean; callbacks: Map; + cachedCallbackValue?: OnyxValue; + cachedCallbackKey?: OnyxKey; }; class OnyxConnectionManager { @@ -24,19 +26,18 @@ class OnyxConnectionManager { bindAll(this, 'fireCallbacks', 'connectionMapKey', 'connect', 'disconnect', 'disconnectAll'); } - private fireCallbacks(connectOptions: ConnectOptions): void { - const mapKey = this.connectionMapKey(connectOptions); + private connectionMapKey(connectOptions: ConnectOptions): string { + return `key=${connectOptions.key},initWithStoredValues=${connectOptions.initWithStoredValues ?? true},waitForCollectionCallback=${connectOptions.waitForCollectionCallback ?? false}`; + } + + private fireCallbacks(mapKey: string): void { const connection = this.connectionsMap.get(mapKey); connection?.callbacks.forEach((callback) => { - callback(); + callback(connection.cachedCallbackValue, connection.cachedCallbackKey as OnyxKey); }); } - private connectionMapKey(connectOptions: ConnectOptions): string { - return `key=${connectOptions.key},initWithStoredValues=${connectOptions.initWithStoredValues ?? true},waitForCollectionCallback=${connectOptions.waitForCollectionCallback ?? false}`; - } - connect(connectOptions: ConnectOptions): [number, string, string] { const mapKey = this.connectionMapKey(connectOptions); let connection = this.connectionsMap.get(mapKey); @@ -46,14 +47,16 @@ class OnyxConnectionManager { if (!connection) { connectionID = Onyx.connect({ - ...connectOptions, - callback: () => { + ...(connectOptions as DefaultConnectOptions), + callback: (value, key) => { const createdConnection = this.connectionsMap.get(mapKey); if (createdConnection) { createdConnection.isConnectionMade = true; - } + createdConnection.cachedCallbackValue = value; + createdConnection.cachedCallbackKey = key; - this.fireCallbacks(connectOptions); + this.fireCallbacks(mapKey); + } }, }); @@ -67,10 +70,12 @@ class OnyxConnectionManager { this.connectionsMap.set(mapKey, connection); } - connection.callbacks.set(callbackID, connectOptions.callback as ConnectCallback); + if (connectOptions.callback) { + connection.callbacks.set(callbackID, connectOptions.callback as ConnectCallback); + } if (connection.isConnectionMade) { - (connectOptions.callback as ConnectCallback)(); + (connectOptions as DefaultConnectOptions).callback?.(connection.cachedCallbackValue, connection.cachedCallbackKey as OnyxKey); } return [connection.id, mapKey, callbackID]; diff --git a/lib/useOnyx.ts b/lib/useOnyx.ts index 230cbafa..a1ca804d 100644 --- a/lib/useOnyx.ts +++ b/lib/useOnyx.ts @@ -5,7 +5,7 @@ import OnyxUtils from './OnyxUtils'; import type {CollectionKeyBase, OnyxCollection, OnyxKey, OnyxValue, Selector} from './types'; import useLiveRef from './useLiveRef'; import usePrevious from './usePrevious'; -import Onyx from './Onyx'; +import connectionManager from './OnyxConnectionManager'; type BaseUseOnyxOptions = { /** @@ -66,7 +66,7 @@ function useOnyx>( options?: BaseUseOnyxOptions & UseOnyxInitialValueOption>, ): UseOnyxResult; function useOnyx>(key: TKey, options?: UseOnyxOptions): UseOnyxResult { - const connectionIDRef = useRef(null); + const connectionIDRef = useRef<[number, string, string] | null>(null); const previousKey = usePrevious(key); // Used to stabilize the selector reference and avoid unnecessary calls to `getSnapshot()`. @@ -154,7 +154,7 @@ function useOnyx>(key: TKey const subscribe = useCallback( (onStoreChange: () => void) => { - connectionIDRef.current = Onyx.connect({ + connectionIDRef.current = connectionManager.connect({ key, callback: () => { // We don't need to update the Onyx cache again here, when `callback` is called the cache is already @@ -171,7 +171,7 @@ function useOnyx>(key: TKey return; } - Onyx.disconnect(connectionIDRef.current); + connectionManager.disconnect(connectionIDRef.current[1], connectionIDRef.current[2]); isFirstConnectionRef.current = false; }; }, @@ -189,9 +189,9 @@ function useOnyx>(key: TKey } if (options.canEvict) { - OnyxUtils.removeFromEvictionBlockList(key, connectionIDRef.current); + OnyxUtils.removeFromEvictionBlockList(key, connectionIDRef.current[0]); } else { - OnyxUtils.addToEvictionBlockList(key, connectionIDRef.current); + OnyxUtils.addToEvictionBlockList(key, connectionIDRef.current[0]); } }, [key, options?.canEvict]); diff --git a/tests/unit/OnyxConnectionManagerTest.ts b/tests/unit/OnyxConnectionManagerTest.ts new file mode 100644 index 00000000..e2cda36e --- /dev/null +++ b/tests/unit/OnyxConnectionManagerTest.ts @@ -0,0 +1,143 @@ +import {act} from '@testing-library/react-native'; +import Onyx from '../../lib'; +import connectionManager from '../../lib/OnyxConnectionManager'; +import waitForPromisesToResolve from '../utils/waitForPromisesToResolve'; +import StorageMock from '../../lib/storage'; +import type GenericCollection from '../utils/GenericCollection'; + +// eslint-disable-next-line dot-notation +const connectionsMap = connectionManager['connectionsMap']; + +const ONYXKEYS = { + TEST_KEY: 'test', + TEST_KEY_2: 'test2', + COLLECTION: { + TEST_KEY: 'test_', + TEST_KEY_2: 'test2_', + }, +}; + +Onyx.init({ + keys: ONYXKEYS, +}); + +beforeEach(() => Onyx.clear()); + +describe('OnyxConnectionManager', () => { + // Always use a "fresh" instance + beforeEach(() => { + connectionManager.disconnectAll(); + }); + + describe('connect', () => { + it('should connect to a key and fire the callback with its value', async () => { + await StorageMock.setItem(ONYXKEYS.TEST_KEY, 'test'); + + const callback1 = jest.fn(); + const [, mapKey, callbackID] = connectionManager.connect({key: ONYXKEYS.TEST_KEY, callback: callback1}); + + expect(connectionsMap.has(mapKey)).toBeTruthy(); + + await act(async () => waitForPromisesToResolve()); + + expect(callback1).toHaveBeenCalledTimes(1); + expect(callback1).toHaveBeenCalledWith('test', ONYXKEYS.TEST_KEY); + + connectionManager.disconnect(mapKey, callbackID); + + expect(connectionsMap.size).toEqual(0); + }); + + it('should connect two times to the same key and fire both callbacks with its value', async () => { + await StorageMock.setItem(ONYXKEYS.TEST_KEY, 'test'); + + const callback1 = jest.fn(); + const [, mapKey1, callbackID1] = connectionManager.connect({key: ONYXKEYS.TEST_KEY, callback: callback1}); + + const callback2 = jest.fn(); + const [, mapKey2, callbackID2] = connectionManager.connect({key: ONYXKEYS.TEST_KEY, callback: callback2}); + + expect(mapKey1).toEqual(mapKey2); + expect(connectionsMap.size).toEqual(1); + expect(connectionsMap.has(mapKey1)).toBeTruthy(); + + await act(async () => waitForPromisesToResolve()); + + expect(callback1).toHaveBeenCalledTimes(1); + expect(callback1).toHaveBeenCalledWith('test', ONYXKEYS.TEST_KEY); + expect(callback2).toHaveBeenCalledTimes(1); + expect(callback2).toHaveBeenCalledWith('test', ONYXKEYS.TEST_KEY); + + connectionManager.disconnect(mapKey1, callbackID1); + connectionManager.disconnect(mapKey1, callbackID2); + + expect(connectionsMap.size).toEqual(0); + }); + + it('should connect two times to the same key but with different options, and fire the callbacks differently', async () => { + const obj1 = {id: 'entry1_id', name: 'entry1_name'}; + const obj2 = {id: 'entry2_id', name: 'entry2_name'}; + const collection: GenericCollection = { + [`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`]: obj1, + [`${ONYXKEYS.COLLECTION.TEST_KEY}entry2`]: obj2, + }; + await StorageMock.multiSet([ + [`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`, obj1], + [`${ONYXKEYS.COLLECTION.TEST_KEY}entry2`, obj2], + ]); + + const callback1 = jest.fn(); + const [, mapKey1, callbackID1] = connectionManager.connect({key: ONYXKEYS.COLLECTION.TEST_KEY, callback: callback1}); + + const callback2 = jest.fn(); + const [, mapKey2, callbackID2] = connectionManager.connect({key: ONYXKEYS.COLLECTION.TEST_KEY, callback: callback2, waitForCollectionCallback: true}); + + expect(mapKey1).not.toEqual(mapKey2); + expect(connectionsMap.size).toEqual(2); + expect(connectionsMap.has(mapKey1)).toBeTruthy(); + expect(connectionsMap.has(mapKey2)).toBeTruthy(); + + await act(async () => waitForPromisesToResolve()); + + expect(callback1).toHaveBeenCalledTimes(2); + expect(callback1).toHaveBeenNthCalledWith(1, obj1, `${ONYXKEYS.COLLECTION.TEST_KEY}entry1`); + expect(callback1).toHaveBeenNthCalledWith(2, obj2, `${ONYXKEYS.COLLECTION.TEST_KEY}entry2`); + + expect(callback2).toHaveBeenCalledTimes(1); + expect(callback2).toHaveBeenCalledWith(collection, undefined); + + connectionManager.disconnect(mapKey1, callbackID1); + connectionManager.disconnect(mapKey2, callbackID2); + + expect(connectionsMap.size).toEqual(0); + }); + + it('should connect to a key, connect some times more after first connection is made, and fire all subsequent callbacks immediately with its value', async () => { + await StorageMock.setItem(ONYXKEYS.TEST_KEY, 'test'); + + const callback1 = jest.fn(); + connectionManager.connect({key: ONYXKEYS.TEST_KEY, callback: callback1}); + + await act(async () => waitForPromisesToResolve()); + + expect(callback1).toHaveBeenCalledTimes(1); + expect(callback1).toHaveBeenCalledWith('test', ONYXKEYS.TEST_KEY); + + const callback2 = jest.fn(); + connectionManager.connect({key: ONYXKEYS.TEST_KEY, callback: callback2}); + + const callback3 = jest.fn(); + connectionManager.connect({key: ONYXKEYS.TEST_KEY, callback: callback3}); + + const callback4 = jest.fn(); + connectionManager.connect({key: ONYXKEYS.TEST_KEY, callback: callback4}); + + expect(callback2).toHaveBeenCalledTimes(1); + expect(callback2).toHaveBeenCalledWith('test', ONYXKEYS.TEST_KEY); + expect(callback3).toHaveBeenCalledTimes(1); + expect(callback3).toHaveBeenCalledWith('test', ONYXKEYS.TEST_KEY); + expect(callback4).toHaveBeenCalledTimes(1); + expect(callback4).toHaveBeenCalledWith('test', ONYXKEYS.TEST_KEY); + }); + }); +}); From 15276d08b8a662047259dbba9a791b517009d628 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Mon, 1 Jul 2024 16:29:45 +0100 Subject: [PATCH 04/31] Return a object instead of tuple --- lib/OnyxConnectionManager.ts | 29 +++++++++++++++++---- lib/useOnyx.ts | 15 ++++++----- tests/unit/OnyxConnectionManagerTest.ts | 34 ++++++++++++------------- tests/unit/useOnyxTest.ts | 22 ++++++++++++++++ 4 files changed, 71 insertions(+), 29 deletions(-) diff --git a/lib/OnyxConnectionManager.ts b/lib/OnyxConnectionManager.ts index 20a81a73..9978cfd8 100644 --- a/lib/OnyxConnectionManager.ts +++ b/lib/OnyxConnectionManager.ts @@ -14,6 +14,12 @@ type Connection = { cachedCallbackKey?: OnyxKey; }; +type ConnectionMetadata = { + key: string; + callbackID: string; + connectionID: number; +}; + class OnyxConnectionManager { private connectionsMap: Map; @@ -38,7 +44,7 @@ class OnyxConnectionManager { }); } - connect(connectOptions: ConnectOptions): [number, string, string] { + connect(connectOptions: ConnectOptions): ConnectionMetadata { const mapKey = this.connectionMapKey(connectOptions); let connection = this.connectionsMap.get(mapKey); let connectionID: number | undefined; @@ -78,11 +84,11 @@ class OnyxConnectionManager { (connectOptions as DefaultConnectOptions).callback?.(connection.cachedCallbackValue, connection.cachedCallbackKey as OnyxKey); } - return [connection.id, mapKey, callbackID]; + return {key: mapKey, callbackID, connectionID: connection.id}; } - disconnect(mapKey: string, callbackID: string): void { - const connection = this.connectionsMap.get(mapKey); + disconnect(key: string, callbackID: string): void { + const connection = this.connectionsMap.get(key); if (!connection) { return; @@ -92,10 +98,21 @@ class OnyxConnectionManager { if (connection.callbacks.size === 0) { Onyx.disconnect(connection.id); - this.connectionsMap.delete(mapKey); + this.connectionsMap.delete(key); } } + disconnectKey(key: string): void { + const connection = this.connectionsMap.get(key); + + if (!connection) { + return; + } + + Onyx.disconnect(connection.id); + this.connectionsMap.delete(key); + } + disconnectAll(): void { Array.from(this.connectionsMap.values()).forEach((connection) => { Onyx.disconnect(connection.id); @@ -108,3 +125,5 @@ class OnyxConnectionManager { const connectionManager = new OnyxConnectionManager(); export default connectionManager; + +export type {ConnectionMetadata}; diff --git a/lib/useOnyx.ts b/lib/useOnyx.ts index a0b2e67c..7dadd2d3 100644 --- a/lib/useOnyx.ts +++ b/lib/useOnyx.ts @@ -2,6 +2,7 @@ import {deepEqual, shallowEqual} from 'fast-equals'; import {useCallback, useEffect, useRef, useSyncExternalStore} from 'react'; import type {IsEqual} from 'type-fest'; import OnyxCache from './OnyxCache'; +import type {ConnectionMetadata} from './OnyxConnectionManager'; import connectionManager from './OnyxConnectionManager'; import OnyxUtils from './OnyxUtils'; import type {CollectionKeyBase, OnyxCollection, OnyxEntry, OnyxKey, OnyxValue, Selector} from './types'; @@ -69,7 +70,7 @@ function useOnyx>( options?: BaseUseOnyxOptions & UseOnyxInitialValueOption>, ): UseOnyxResult; function useOnyx>(key: TKey, options?: UseOnyxOptions): UseOnyxResult { - const connectionIDRef = useRef<[number, string, string] | null>(null); + const connectionRef = useRef(null); const previousKey = usePrevious(key); // Used to stabilize the selector reference and avoid unnecessary calls to `getSnapshot()`. @@ -186,7 +187,7 @@ function useOnyx>(key: TKey const subscribe = useCallback( (onStoreChange: () => void) => { - connectionIDRef.current = connectionManager.connect({ + connectionRef.current = connectionManager.connect({ key, callback: () => { // Signals that the first connection was made, so some logics in `getSnapshot()` @@ -204,11 +205,11 @@ function useOnyx>(key: TKey }); return () => { - if (!connectionIDRef.current) { + if (!connectionRef.current) { return; } - connectionManager.disconnect(connectionIDRef.current[1], connectionIDRef.current[2]); + connectionManager.disconnect(connectionRef.current.key, connectionRef.current.callbackID); isFirstConnectionRef.current = false; }; }, @@ -217,7 +218,7 @@ function useOnyx>(key: TKey // Mimics withOnyx's checkEvictableKeys() behavior. useEffect(() => { - if (options?.canEvict === undefined || !connectionIDRef.current) { + if (options?.canEvict === undefined || !connectionRef.current) { return; } @@ -226,9 +227,9 @@ function useOnyx>(key: TKey } if (options.canEvict) { - OnyxUtils.removeFromEvictionBlockList(key, connectionIDRef.current[0]); + OnyxUtils.removeFromEvictionBlockList(key, connectionRef.current.connectionID); } else { - OnyxUtils.addToEvictionBlockList(key, connectionIDRef.current[0]); + OnyxUtils.addToEvictionBlockList(key, connectionRef.current.connectionID); } }, [key, options?.canEvict]); diff --git a/tests/unit/OnyxConnectionManagerTest.ts b/tests/unit/OnyxConnectionManagerTest.ts index e2cda36e..0de17853 100644 --- a/tests/unit/OnyxConnectionManagerTest.ts +++ b/tests/unit/OnyxConnectionManagerTest.ts @@ -1,9 +1,9 @@ import {act} from '@testing-library/react-native'; import Onyx from '../../lib'; import connectionManager from '../../lib/OnyxConnectionManager'; -import waitForPromisesToResolve from '../utils/waitForPromisesToResolve'; import StorageMock from '../../lib/storage'; import type GenericCollection from '../utils/GenericCollection'; +import waitForPromisesToResolve from '../utils/waitForPromisesToResolve'; // eslint-disable-next-line dot-notation const connectionsMap = connectionManager['connectionsMap']; @@ -34,16 +34,16 @@ describe('OnyxConnectionManager', () => { await StorageMock.setItem(ONYXKEYS.TEST_KEY, 'test'); const callback1 = jest.fn(); - const [, mapKey, callbackID] = connectionManager.connect({key: ONYXKEYS.TEST_KEY, callback: callback1}); + const {key, callbackID} = connectionManager.connect({key: ONYXKEYS.TEST_KEY, callback: callback1}); - expect(connectionsMap.has(mapKey)).toBeTruthy(); + expect(connectionsMap.has(key)).toBeTruthy(); await act(async () => waitForPromisesToResolve()); expect(callback1).toHaveBeenCalledTimes(1); expect(callback1).toHaveBeenCalledWith('test', ONYXKEYS.TEST_KEY); - connectionManager.disconnect(mapKey, callbackID); + connectionManager.disconnect(key, callbackID); expect(connectionsMap.size).toEqual(0); }); @@ -52,14 +52,14 @@ describe('OnyxConnectionManager', () => { await StorageMock.setItem(ONYXKEYS.TEST_KEY, 'test'); const callback1 = jest.fn(); - const [, mapKey1, callbackID1] = connectionManager.connect({key: ONYXKEYS.TEST_KEY, callback: callback1}); + const {key: key1, callbackID: callbackID1} = connectionManager.connect({key: ONYXKEYS.TEST_KEY, callback: callback1}); const callback2 = jest.fn(); - const [, mapKey2, callbackID2] = connectionManager.connect({key: ONYXKEYS.TEST_KEY, callback: callback2}); + const {key: key2, callbackID: callbackID2} = connectionManager.connect({key: ONYXKEYS.TEST_KEY, callback: callback2}); - expect(mapKey1).toEqual(mapKey2); + expect(key1).toEqual(key2); expect(connectionsMap.size).toEqual(1); - expect(connectionsMap.has(mapKey1)).toBeTruthy(); + expect(connectionsMap.has(key1)).toBeTruthy(); await act(async () => waitForPromisesToResolve()); @@ -68,8 +68,8 @@ describe('OnyxConnectionManager', () => { expect(callback2).toHaveBeenCalledTimes(1); expect(callback2).toHaveBeenCalledWith('test', ONYXKEYS.TEST_KEY); - connectionManager.disconnect(mapKey1, callbackID1); - connectionManager.disconnect(mapKey1, callbackID2); + connectionManager.disconnect(key1, callbackID1); + connectionManager.disconnect(key1, callbackID2); expect(connectionsMap.size).toEqual(0); }); @@ -87,15 +87,15 @@ describe('OnyxConnectionManager', () => { ]); const callback1 = jest.fn(); - const [, mapKey1, callbackID1] = connectionManager.connect({key: ONYXKEYS.COLLECTION.TEST_KEY, callback: callback1}); + const {key: key1, callbackID: callbackID1} = connectionManager.connect({key: ONYXKEYS.COLLECTION.TEST_KEY, callback: callback1}); const callback2 = jest.fn(); - const [, mapKey2, callbackID2] = connectionManager.connect({key: ONYXKEYS.COLLECTION.TEST_KEY, callback: callback2, waitForCollectionCallback: true}); + const {key: key2, callbackID: callbackID2} = connectionManager.connect({key: ONYXKEYS.COLLECTION.TEST_KEY, callback: callback2, waitForCollectionCallback: true}); - expect(mapKey1).not.toEqual(mapKey2); + expect(key1).not.toEqual(key2); expect(connectionsMap.size).toEqual(2); - expect(connectionsMap.has(mapKey1)).toBeTruthy(); - expect(connectionsMap.has(mapKey2)).toBeTruthy(); + expect(connectionsMap.has(key1)).toBeTruthy(); + expect(connectionsMap.has(key2)).toBeTruthy(); await act(async () => waitForPromisesToResolve()); @@ -106,8 +106,8 @@ describe('OnyxConnectionManager', () => { expect(callback2).toHaveBeenCalledTimes(1); expect(callback2).toHaveBeenCalledWith(collection, undefined); - connectionManager.disconnect(mapKey1, callbackID1); - connectionManager.disconnect(mapKey2, callbackID2); + connectionManager.disconnect(key1, callbackID1); + connectionManager.disconnect(key2, callbackID2); expect(connectionsMap.size).toEqual(0); }); diff --git a/tests/unit/useOnyxTest.ts b/tests/unit/useOnyxTest.ts index 42f1c9f1..41608b94 100644 --- a/tests/unit/useOnyxTest.ts +++ b/tests/unit/useOnyxTest.ts @@ -483,4 +483,26 @@ describe('useOnyx', () => { expect(result.current[1].status).toEqual('loaded'); }); }); + + describe('multiple usage', () => { + it('should 1', async () => { + await StorageMock.setItem(ONYXKEYS.TEST_KEY, 'test'); + + const {result: result1} = renderHook(() => useOnyx(ONYXKEYS.TEST_KEY)); + // const {result: result2} = renderHook(() => useOnyx(ONYXKEYS.TEST_KEY)); + + expect(result1.current[0]).toBeUndefined(); + expect(result1.current[1].status).toEqual('loading'); + + await act(async () => waitForPromisesToResolve()); + + expect(result1.current[0]).toEqual('test'); + expect(result1.current[1].status).toEqual('loaded'); + + const {result: result2} = renderHook(() => useOnyx(ONYXKEYS.TEST_KEY)); + + expect(result2.current[0]).toEqual('test'); + expect(result2.current[1].status).toEqual('loaded'); + }); + }); }); From 27c779b435450f546638e2ee9d36161689b4d0f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Tue, 2 Jul 2024 16:23:00 +0100 Subject: [PATCH 05/31] Integrate the connection manager into withOnyx --- lib/OnyxConnectionManager.ts | 39 ++++++--- lib/Str.ts | 16 +++- lib/useOnyx.ts | 2 +- lib/withOnyx/index.tsx | 19 ++-- tests/unit/OnyxConnectionManagerTest.ts | 111 ++++++++++++++++++++---- 5 files changed, 148 insertions(+), 39 deletions(-) diff --git a/lib/OnyxConnectionManager.ts b/lib/OnyxConnectionManager.ts index 9978cfd8..a4d89e89 100644 --- a/lib/OnyxConnectionManager.ts +++ b/lib/OnyxConnectionManager.ts @@ -1,6 +1,7 @@ import bindAll from 'lodash/bindAll'; import type {ConnectOptions} from './Onyx'; import Onyx from './Onyx'; +import * as Str from './Str'; import type {DefaultConnectCallback, DefaultConnectOptions, OnyxKey, OnyxValue} from './types'; type ConnectCallback = DefaultConnectCallback; @@ -29,11 +30,19 @@ class OnyxConnectionManager { this.connectionsMap = new Map(); this.lastCallbackID = 0; - bindAll(this, 'fireCallbacks', 'connectionMapKey', 'connect', 'disconnect', 'disconnectAll'); + bindAll(this, 'fireCallbacks', 'connectionMapKey', 'connect', 'disconnect', 'disconnectKey', 'disconnectAll'); } private connectionMapKey(connectOptions: ConnectOptions): string { - return `key=${connectOptions.key},initWithStoredValues=${connectOptions.initWithStoredValues ?? true},waitForCollectionCallback=${connectOptions.waitForCollectionCallback ?? false}`; + let suffix = ''; + + if ('withOnyxInstance' in connectOptions) { + suffix += `,withOnyxInstance=${Str.guid()}`; + } + + return `key=${connectOptions.key},initWithStoredValues=${connectOptions.initWithStoredValues ?? true},waitForCollectionCallback=${ + connectOptions.waitForCollectionCallback ?? false + }${suffix}`; } private fireCallbacks(mapKey: string): void { @@ -52,9 +61,10 @@ class OnyxConnectionManager { const callbackID = String(this.lastCallbackID++); if (!connection) { - connectionID = Onyx.connect({ - ...(connectOptions as DefaultConnectOptions), - callback: (value, key) => { + let callback: ConnectCallback | undefined; + + if (!('withOnyxInstance' in connectOptions)) { + callback = (value, key) => { const createdConnection = this.connectionsMap.get(mapKey); if (createdConnection) { createdConnection.isConnectionMade = true; @@ -63,7 +73,12 @@ class OnyxConnectionManager { this.fireCallbacks(mapKey); } - }, + }; + } + + connectionID = Onyx.connect({ + ...(connectOptions as DefaultConnectOptions), + callback, }); connection = { @@ -87,7 +102,7 @@ class OnyxConnectionManager { return {key: mapKey, callbackID, connectionID: connection.id}; } - disconnect(key: string, callbackID: string): void { + disconnect({key, callbackID}: ConnectionMetadata, shouldRemoveKeyFromEvictionBlocklist?: boolean): void { const connection = this.connectionsMap.get(key); if (!connection) { @@ -97,25 +112,25 @@ class OnyxConnectionManager { connection.callbacks.delete(callbackID); if (connection.callbacks.size === 0) { - Onyx.disconnect(connection.id); + Onyx.disconnect(connection.id, shouldRemoveKeyFromEvictionBlocklist ? connection.onyxKey : undefined); this.connectionsMap.delete(key); } } - disconnectKey(key: string): void { + disconnectKey(key: string, shouldRemoveKeyFromEvictionBlocklist?: boolean): void { const connection = this.connectionsMap.get(key); if (!connection) { return; } - Onyx.disconnect(connection.id); + Onyx.disconnect(connection.id, shouldRemoveKeyFromEvictionBlocklist ? connection.onyxKey : undefined); this.connectionsMap.delete(key); } - disconnectAll(): void { + disconnectAll(shouldRemoveKeysFromEvictionBlocklist?: boolean): void { Array.from(this.connectionsMap.values()).forEach((connection) => { - Onyx.disconnect(connection.id); + Onyx.disconnect(connection.id, shouldRemoveKeysFromEvictionBlocklist ? connection.onyxKey : undefined); }); this.connectionsMap.clear(); diff --git a/lib/Str.ts b/lib/Str.ts index bb1429b4..8666f2e7 100644 --- a/lib/Str.ts +++ b/lib/Str.ts @@ -21,4 +21,18 @@ function result unknown, TArgs extends unknow return typeof parameter === 'function' ? (parameter(...args) as ReturnType) : parameter; } -export {startsWith, result}; +/** + * A simple GUID generator taken from https://stackoverflow.com/a/32760401/9114791 + * + * @param [prefix] an optional prefix to put in front of the guid + */ +function guid(prefix = ''): string { + function s4() { + return Math.floor((1 + Math.random()) * 0x10000) + .toString(16) + .substring(1); + } + return `${prefix}${s4()}${s4()}-${s4()}-${s4()}-${s4()}-${s4()}${s4()}${s4()}`; +} + +export {guid, result, startsWith}; diff --git a/lib/useOnyx.ts b/lib/useOnyx.ts index 7dadd2d3..fa5521bc 100644 --- a/lib/useOnyx.ts +++ b/lib/useOnyx.ts @@ -209,7 +209,7 @@ function useOnyx>(key: TKey return; } - connectionManager.disconnect(connectionRef.current.key, connectionRef.current.callbackID); + connectionManager.disconnect(connectionRef.current, true); isFirstConnectionRef.current = false; }; }, diff --git a/lib/withOnyx/index.tsx b/lib/withOnyx/index.tsx index 77f26930..8072b754 100644 --- a/lib/withOnyx/index.tsx +++ b/lib/withOnyx/index.tsx @@ -4,13 +4,14 @@ * will automatically change to reflect the new data. */ import React from 'react'; -import Onyx from '../Onyx'; import OnyxUtils from '../OnyxUtils'; import * as Str from '../Str'; import type {GenericFunction, Mapping, OnyxKey, WithOnyxConnectOptions} from '../types'; import utils from '../utils'; import type {MapOnyxToState, WithOnyxInstance, WithOnyxMapping, WithOnyxProps, WithOnyxState} from './types'; import cache from '../OnyxCache'; +import type {ConnectionMetadata} from '../OnyxConnectionManager'; +import connectionManager from '../OnyxConnectionManager'; // This is a list of keys that can exist on a `mapping`, but are not directly related to loading data from Onyx. When the keys of a mapping are looped over to check // if a key has changed, it's a good idea to skip looking at these properties since they would have unexpected results. @@ -67,7 +68,7 @@ export default function ( shouldDelayUpdates: boolean; - activeConnectionIDs: Record; + activeConnections: Record; tempState: WithOnyxState | undefined; @@ -78,9 +79,9 @@ export default function ( this.setWithOnyxState = this.setWithOnyxState.bind(this); this.flushPendingSetStates = this.flushPendingSetStates.bind(this); - // This stores all the Onyx connection IDs to be used when the component unmounts so everything can be - // disconnected. It is a key value store with the format {[mapping.key]: connectionID}. - this.activeConnectionIDs = {}; + // This stores all the Onyx connections to be used when the component unmounts so everything can be + // disconnected. It is a key value store with the format {[mapping.key]: connection metadata object}. + this.activeConnections = {}; const cachedState = mapOnyxToStateEntries(mapOnyxToState).reduce>((resultObj, [propName, mapping]) => { const key = Str.result(mapping.key as GenericFunction, props); @@ -163,8 +164,8 @@ export default function ( const previousKey = isFirstTimeUpdatingAfterLoading ? mapping.previousKey : Str.result(mapping.key as GenericFunction, {...prevProps, ...prevOnyxDataFromState}); const newKey = Str.result(mapping.key as GenericFunction, {...this.props, ...onyxDataFromState}); if (previousKey !== newKey) { - Onyx.disconnect(this.activeConnectionIDs[previousKey], previousKey); - delete this.activeConnectionIDs[previousKey]; + connectionManager.disconnect(this.activeConnections[previousKey], true); + delete this.activeConnections[previousKey]; this.connectMappingToOnyx(mapping, propName, newKey); } }); @@ -176,7 +177,7 @@ export default function ( // Disconnect everything from Onyx mapOnyxToStateEntries(mapOnyxToState).forEach(([, mapping]) => { const key = Str.result(mapping.key as GenericFunction, {...this.props, ...getOnyxDataFromState(this.state, mapOnyxToState)}); - Onyx.disconnect(this.activeConnectionIDs[key], key); + connectionManager.disconnect(this.activeConnections[key], true); }); } @@ -319,7 +320,7 @@ export default function ( onyxMapping.previousKey = key; // eslint-disable-next-line rulesdir/prefer-onyx-connect-in-libs - this.activeConnectionIDs[key] = Onyx.connect({ + this.activeConnections[key] = connectionManager.connect({ ...mapping, key, statePropertyName: statePropertyName as string, diff --git a/tests/unit/OnyxConnectionManagerTest.ts b/tests/unit/OnyxConnectionManagerTest.ts index 0de17853..6fb3818e 100644 --- a/tests/unit/OnyxConnectionManagerTest.ts +++ b/tests/unit/OnyxConnectionManagerTest.ts @@ -1,7 +1,10 @@ +// eslint-disable-next-line max-classes-per-file import {act} from '@testing-library/react-native'; import Onyx from '../../lib'; import connectionManager from '../../lib/OnyxConnectionManager'; import StorageMock from '../../lib/storage'; +import type {OnyxKey, WithOnyxConnectOptions} from '../../lib/types'; +import type {WithOnyxInstance} from '../../lib/withOnyx/types'; import type GenericCollection from '../utils/GenericCollection'; import waitForPromisesToResolve from '../utils/waitForPromisesToResolve'; @@ -34,16 +37,16 @@ describe('OnyxConnectionManager', () => { await StorageMock.setItem(ONYXKEYS.TEST_KEY, 'test'); const callback1 = jest.fn(); - const {key, callbackID} = connectionManager.connect({key: ONYXKEYS.TEST_KEY, callback: callback1}); + const connection = connectionManager.connect({key: ONYXKEYS.TEST_KEY, callback: callback1}); - expect(connectionsMap.has(key)).toBeTruthy(); + expect(connectionsMap.has(connection.key)).toBeTruthy(); await act(async () => waitForPromisesToResolve()); expect(callback1).toHaveBeenCalledTimes(1); expect(callback1).toHaveBeenCalledWith('test', ONYXKEYS.TEST_KEY); - connectionManager.disconnect(key, callbackID); + connectionManager.disconnect(connection); expect(connectionsMap.size).toEqual(0); }); @@ -52,14 +55,14 @@ describe('OnyxConnectionManager', () => { await StorageMock.setItem(ONYXKEYS.TEST_KEY, 'test'); const callback1 = jest.fn(); - const {key: key1, callbackID: callbackID1} = connectionManager.connect({key: ONYXKEYS.TEST_KEY, callback: callback1}); + const connection1 = connectionManager.connect({key: ONYXKEYS.TEST_KEY, callback: callback1}); const callback2 = jest.fn(); - const {key: key2, callbackID: callbackID2} = connectionManager.connect({key: ONYXKEYS.TEST_KEY, callback: callback2}); + const connection2 = connectionManager.connect({key: ONYXKEYS.TEST_KEY, callback: callback2}); - expect(key1).toEqual(key2); + expect(connection1.key).toEqual(connection2.key); expect(connectionsMap.size).toEqual(1); - expect(connectionsMap.has(key1)).toBeTruthy(); + expect(connectionsMap.has(connection1.key)).toBeTruthy(); await act(async () => waitForPromisesToResolve()); @@ -68,8 +71,8 @@ describe('OnyxConnectionManager', () => { expect(callback2).toHaveBeenCalledTimes(1); expect(callback2).toHaveBeenCalledWith('test', ONYXKEYS.TEST_KEY); - connectionManager.disconnect(key1, callbackID1); - connectionManager.disconnect(key1, callbackID2); + connectionManager.disconnect(connection1); + connectionManager.disconnect(connection2); expect(connectionsMap.size).toEqual(0); }); @@ -87,15 +90,15 @@ describe('OnyxConnectionManager', () => { ]); const callback1 = jest.fn(); - const {key: key1, callbackID: callbackID1} = connectionManager.connect({key: ONYXKEYS.COLLECTION.TEST_KEY, callback: callback1}); + const connection1 = connectionManager.connect({key: ONYXKEYS.COLLECTION.TEST_KEY, callback: callback1}); const callback2 = jest.fn(); - const {key: key2, callbackID: callbackID2} = connectionManager.connect({key: ONYXKEYS.COLLECTION.TEST_KEY, callback: callback2, waitForCollectionCallback: true}); + const connection2 = connectionManager.connect({key: ONYXKEYS.COLLECTION.TEST_KEY, callback: callback2, waitForCollectionCallback: true}); - expect(key1).not.toEqual(key2); + expect(connection1.key).not.toEqual(connection2.key); expect(connectionsMap.size).toEqual(2); - expect(connectionsMap.has(key1)).toBeTruthy(); - expect(connectionsMap.has(key2)).toBeTruthy(); + expect(connectionsMap.has(connection1.key)).toBeTruthy(); + expect(connectionsMap.has(connection2.key)).toBeTruthy(); await act(async () => waitForPromisesToResolve()); @@ -106,8 +109,8 @@ describe('OnyxConnectionManager', () => { expect(callback2).toHaveBeenCalledTimes(1); expect(callback2).toHaveBeenCalledWith(collection, undefined); - connectionManager.disconnect(key1, callbackID1); - connectionManager.disconnect(key2, callbackID2); + connectionManager.disconnect(connection1); + connectionManager.disconnect(connection2); expect(connectionsMap.size).toEqual(0); }); @@ -140,4 +143,80 @@ describe('OnyxConnectionManager', () => { expect(callback4).toHaveBeenCalledWith('test', ONYXKEYS.TEST_KEY); }); }); + + describe('withOnyx', () => { + it('should connect to a key two times with withOnyx and create two connections instead of one', async () => { + await StorageMock.setItem(ONYXKEYS.TEST_KEY, 'test'); + + const connection1 = connectionManager.connect({ + key: ONYXKEYS.TEST_KEY, + displayName: 'Component1', + statePropertyName: 'prop1', + withOnyxInstance: new (class { + // eslint-disable-next-line @typescript-eslint/no-empty-function + setStateProxy() {} + + // eslint-disable-next-line @typescript-eslint/no-empty-function + setWithOnyxState() {} + })() as unknown as WithOnyxInstance, + } as WithOnyxConnectOptions); + + const connection2 = connectionManager.connect({ + key: ONYXKEYS.TEST_KEY, + displayName: 'Component2', + statePropertyName: 'prop2', + withOnyxInstance: new (class { + // eslint-disable-next-line @typescript-eslint/no-empty-function + setStateProxy() {} + + // eslint-disable-next-line @typescript-eslint/no-empty-function + setWithOnyxState() {} + })() as unknown as WithOnyxInstance, + } as WithOnyxConnectOptions); + + await act(async () => waitForPromisesToResolve()); + + expect(connection1.key).not.toEqual(connection2.key); + expect(connectionsMap.size).toEqual(2); + expect(connectionsMap.has(connection1.key)).toBeTruthy(); + expect(connectionsMap.has(connection2.key)).toBeTruthy(); + + connectionManager.disconnect(connection1); + connectionManager.disconnect(connection2); + + expect(connectionsMap.size).toEqual(0); + }); + + it('should connect to a key directly, connect again with withOnyx and create another connection instead of reusing the first one', async () => { + await StorageMock.setItem(ONYXKEYS.TEST_KEY, 'test'); + + const callback1 = jest.fn(); + const connection1 = connectionManager.connect({key: ONYXKEYS.TEST_KEY, callback: callback1}); + + await act(async () => waitForPromisesToResolve()); + + const connection2 = connectionManager.connect({ + key: ONYXKEYS.TEST_KEY, + displayName: 'Component2', + statePropertyName: 'prop2', + withOnyxInstance: new (class { + // eslint-disable-next-line @typescript-eslint/no-empty-function + setStateProxy() {} + + // eslint-disable-next-line @typescript-eslint/no-empty-function + setWithOnyxState() {} + })() as unknown as WithOnyxInstance, + } as WithOnyxConnectOptions); + + expect(connection1.key).not.toEqual(connection2.key); + expect(connectionsMap.size).toEqual(2); + expect(connectionsMap.has(connection1.key)).toBeTruthy(); + expect(connectionsMap.has(connection2.key)).toBeTruthy(); + + connectionManager.disconnect(connection1); + connectionManager.disconnect(connection2); + + expect(connectionsMap.size).toEqual(0); + }); + }); }); From 94e6bdc9cd71883a0e1fdf50a1824fa78262f947 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Tue, 2 Jul 2024 19:17:05 +0100 Subject: [PATCH 06/31] Refactor Onyx.connect to use the connection manager --- lib/Onyx.ts | 158 +--------------------- lib/OnyxConnectionManager.ts | 45 ++++++- lib/OnyxUtils.ts | 165 ++++++++++++++++++++++- lib/createDeferredTask.ts | 2 + lib/types.ts | 2 +- tests/unit/onyxCacheTest.tsx | 7 +- tests/unit/onyxClearNativeStorageTest.ts | 9 +- tests/unit/onyxClearWebStorageTest.ts | 31 +++-- tests/unit/onyxTest.ts | 119 ++++++++-------- 9 files changed, 301 insertions(+), 237 deletions(-) diff --git a/lib/Onyx.ts b/lib/Onyx.ts index b5e11638..0769a517 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -3,7 +3,6 @@ import _ from 'underscore'; import lodashPick from 'lodash/pick'; import * as Logger from './Logger'; import cache from './OnyxCache'; -import createDeferredTask from './createDeferredTask'; import * as PerformanceUtils from './PerformanceUtils'; import Storage from './storage'; import utils from './utils'; @@ -27,12 +26,7 @@ import type { } from './types'; import OnyxUtils from './OnyxUtils'; import logMessages from './logMessages'; - -// Keeps track of the last connectionID that was used so we can keep incrementing it -let lastConnectionID = 0; - -// Connections can be made before `Onyx.init`. They would wait for this task before resolving -const deferredInitTask = createDeferredTask(); +import connectionManager from './OnyxConnectionManager'; /** Initialize the store with actions and listening for storage events */ function init({ @@ -64,151 +58,7 @@ function init({ OnyxUtils.initStoreValues(keys, initialKeyStates, safeEvictionKeys); // Initialize all of our keys with data provided then give green light to any pending connections - Promise.all([OnyxUtils.addAllSafeEvictionKeysToRecentlyAccessedList(), OnyxUtils.initializeWithDefaultKeyStates()]).then(deferredInitTask.resolve); -} - -/** - * Subscribes a react component's state directly to a store key - * - * @example - * const connectionID = Onyx.connect({ - * key: ONYXKEYS.SESSION, - * callback: onSessionChange, - * }); - * - * @param mapping the mapping information to connect Onyx to the components state - * @param mapping.key ONYXKEY to subscribe to - * @param [mapping.statePropertyName] the name of the property in the state to connect the data to - * @param [mapping.withOnyxInstance] whose setState() method will be called with any changed data - * This is used by React components to connect to Onyx - * @param [mapping.callback] a method that will be called with changed data - * This is used by any non-React code to connect to Onyx - * @param [mapping.initWithStoredValues] If set to false, then no data will be prefilled into the - * component - * @param [mapping.waitForCollectionCallback] If set to true, it will return the entire collection to the callback as a single object - * @param [mapping.selector] THIS PARAM IS ONLY USED WITH withOnyx(). If included, this will be used to subscribe to a subset of an Onyx key's data. - * The sourceData and withOnyx state are passed to the selector and should return the simplified data. Using this setting on `withOnyx` can have very positive - * performance benefits because the component will only re-render when the subset of data changes. Otherwise, any change of data on any property would normally - * cause the component to re-render (and that can be expensive from a performance standpoint). - * @param [mapping.initialValue] THIS PARAM IS ONLY USED WITH withOnyx(). - * If included, this will be passed to the component so that something can be rendered while data is being fetched from the DB. - * Note that it will not cause the component to have the loading prop set to true. - * @returns an ID to use when calling disconnect - */ -function connect(connectOptions: ConnectOptions): number { - const mapping = connectOptions as Mapping; - const connectionID = lastConnectionID++; - const callbackToStateMapping = OnyxUtils.getCallbackToStateMapping(); - callbackToStateMapping[connectionID] = mapping as Mapping; - callbackToStateMapping[connectionID].connectionID = connectionID; - - if (mapping.initWithStoredValues === false) { - return connectionID; - } - - // Commit connection only after init passes - deferredInitTask.promise - .then(() => OnyxUtils.addKeyToRecentlyAccessedIfNeeded(mapping)) - .then(() => { - // Performance improvement - // If the mapping is connected to an onyx key that is not a collection - // we can skip the call to getAllKeys() and return an array with a single item - if (Boolean(mapping.key) && typeof mapping.key === 'string' && !mapping.key.endsWith('_') && cache.getAllKeys().has(mapping.key)) { - return new Set([mapping.key]); - } - return OnyxUtils.getAllKeys(); - }) - .then((keys) => { - // We search all the keys in storage to see if any are a "match" for the subscriber we are connecting so that we - // can send data back to the subscriber. Note that multiple keys can match as a subscriber could either be - // subscribed to a "collection key" or a single key. - const matchingKeys: string[] = []; - keys.forEach((key) => { - if (!OnyxUtils.isKeyMatch(mapping.key, key)) { - return; - } - matchingKeys.push(key); - }); - // If the key being connected to does not exist we initialize the value with null. For subscribers that connected - // directly via connect() they will simply get a null value sent to them without any information about which key matched - // since there are none matched. In withOnyx() we wait for all connected keys to return a value before rendering the child - // component. This null value will be filtered out so that the connected component can utilize defaultProps. - if (matchingKeys.length === 0) { - if (mapping.key && !OnyxUtils.isCollectionKey(mapping.key)) { - cache.addNullishStorageKey(mapping.key); - } - - // Here we cannot use batching because the nullish value is expected to be set immediately for default props - // or they will be undefined. - OnyxUtils.sendDataToConnection(mapping, null, undefined, false); - return; - } - - // When using a callback subscriber we will either trigger the provided callback for each key we find or combine all values - // into an object and just make a single call. The latter behavior is enabled by providing a waitForCollectionCallback key - // combined with a subscription to a collection key. - if (typeof mapping.callback === 'function') { - if (OnyxUtils.isCollectionKey(mapping.key)) { - if (mapping.waitForCollectionCallback) { - OnyxUtils.getCollectionDataAndSendAsObject(matchingKeys, mapping); - return; - } - - // We did not opt into using waitForCollectionCallback mode so the callback is called for every matching key. - OnyxUtils.multiGet(matchingKeys).then((values) => { - values.forEach((val, key) => { - OnyxUtils.sendDataToConnection(mapping, val as OnyxValue, key as TKey, true); - }); - }); - return; - } - - // If we are not subscribed to a collection key then there's only a single key to send an update for. - OnyxUtils.get(mapping.key).then((val) => OnyxUtils.sendDataToConnection(mapping, val as OnyxValue, mapping.key, true)); - return; - } - - // If we have a withOnyxInstance that means a React component has subscribed via the withOnyx() HOC and we need to - // group collection key member data into an object. - if ('withOnyxInstance' in mapping && mapping.withOnyxInstance) { - if (OnyxUtils.isCollectionKey(mapping.key)) { - OnyxUtils.getCollectionDataAndSendAsObject(matchingKeys, mapping); - return; - } - - // If the subscriber is not using a collection key then we just send a single value back to the subscriber - OnyxUtils.get(mapping.key).then((val) => OnyxUtils.sendDataToConnection(mapping, val as OnyxValue, mapping.key, true)); - return; - } - - console.error('Warning: Onyx.connect() was found without a callback or withOnyxInstance'); - }); - - // The connectionID is returned back to the caller so that it can be used to clean up the connection when it's no longer needed - // by calling Onyx.disconnect(connectionID). - return connectionID; -} - -/** - * Remove the listener for a react component - * @example - * Onyx.disconnect(connectionID); - * - * @param connectionID unique id returned by call to Onyx.connect() - */ -function disconnect(connectionID: number, keyToRemoveFromEvictionBlocklist?: OnyxKey): void { - const callbackToStateMapping = OnyxUtils.getCallbackToStateMapping(); - if (!callbackToStateMapping[connectionID]) { - return; - } - - // Remove this key from the eviction block list as we are no longer - // subscribing to it and it should be safe to delete again - if (keyToRemoveFromEvictionBlocklist) { - OnyxUtils.removeFromEvictionBlockList(keyToRemoveFromEvictionBlocklist, connectionID); - } - - delete callbackToStateMapping[connectionID]; + Promise.all([OnyxUtils.addAllSafeEvictionKeysToRecentlyAccessedList(), OnyxUtils.initializeWithDefaultKeyStates()]).then(OnyxUtils.getDeferredInitTask().resolve); } /** @@ -740,8 +590,8 @@ function update(data: OnyxUpdate[]): Promise { const Onyx = { METHOD: OnyxUtils.METHOD, - connect, - disconnect, + connect: connectionManager.connect, + disconnect: connectionManager.disconnect, set, multiSet, merge, diff --git a/lib/OnyxConnectionManager.ts b/lib/OnyxConnectionManager.ts index a4d89e89..f239f49f 100644 --- a/lib/OnyxConnectionManager.ts +++ b/lib/OnyxConnectionManager.ts @@ -1,6 +1,6 @@ import bindAll from 'lodash/bindAll'; import type {ConnectOptions} from './Onyx'; -import Onyx from './Onyx'; +import OnyxUtils from './OnyxUtils'; import * as Str from './Str'; import type {DefaultConnectCallback, DefaultConnectOptions, OnyxKey, OnyxValue} from './types'; @@ -53,6 +53,34 @@ class OnyxConnectionManager { }); } + /** + * Subscribes a react component's state directly to a store key + * + * @example + * const connection = Onyx.connect({ + * key: ONYXKEYS.SESSION, + * callback: onSessionChange, + * }); + * + * @param mapping the mapping information to connect Onyx to the components state + * @param mapping.key ONYXKEY to subscribe to + * @param [mapping.statePropertyName] the name of the property in the state to connect the data to + * @param [mapping.withOnyxInstance] whose setState() method will be called with any changed data + * This is used by React components to connect to Onyx + * @param [mapping.callback] a method that will be called with changed data + * This is used by any non-React code to connect to Onyx + * @param [mapping.initWithStoredValues] If set to false, then no data will be prefilled into the + * component + * @param [mapping.waitForCollectionCallback] If set to true, it will return the entire collection to the callback as a single object + * @param [mapping.selector] THIS PARAM IS ONLY USED WITH withOnyx(). If included, this will be used to subscribe to a subset of an Onyx key's data. + * The sourceData and withOnyx state are passed to the selector and should return the simplified data. Using this setting on `withOnyx` can have very positive + * performance benefits because the component will only re-render when the subset of data changes. Otherwise, any change of data on any property would normally + * cause the component to re-render (and that can be expensive from a performance standpoint). + * @param [mapping.initialValue] THIS PARAM IS ONLY USED WITH withOnyx(). + * If included, this will be passed to the component so that something can be rendered while data is being fetched from the DB. + * Note that it will not cause the component to have the loading prop set to true. + * @returns a connection metadata object to use when calling `Onyx.disconnect()` + */ connect(connectOptions: ConnectOptions): ConnectionMetadata { const mapKey = this.connectionMapKey(connectOptions); let connection = this.connectionsMap.get(mapKey); @@ -76,7 +104,7 @@ class OnyxConnectionManager { }; } - connectionID = Onyx.connect({ + connectionID = OnyxUtils.connectToKey({ ...(connectOptions as DefaultConnectOptions), callback, }); @@ -102,6 +130,13 @@ class OnyxConnectionManager { return {key: mapKey, callbackID, connectionID: connection.id}; } + /** + * Remove the listener for a react component + * @example + * Onyx.disconnectFromKey(connection); + * + * @param connection connection metadata object returned by call to `Onyx.connect()` + */ disconnect({key, callbackID}: ConnectionMetadata, shouldRemoveKeyFromEvictionBlocklist?: boolean): void { const connection = this.connectionsMap.get(key); @@ -112,7 +147,7 @@ class OnyxConnectionManager { connection.callbacks.delete(callbackID); if (connection.callbacks.size === 0) { - Onyx.disconnect(connection.id, shouldRemoveKeyFromEvictionBlocklist ? connection.onyxKey : undefined); + OnyxUtils.disconnectFromKey(connection.id, shouldRemoveKeyFromEvictionBlocklist ? connection.onyxKey : undefined); this.connectionsMap.delete(key); } } @@ -124,13 +159,13 @@ class OnyxConnectionManager { return; } - Onyx.disconnect(connection.id, shouldRemoveKeyFromEvictionBlocklist ? connection.onyxKey : undefined); + OnyxUtils.disconnectFromKey(connection.id, shouldRemoveKeyFromEvictionBlocklist ? connection.onyxKey : undefined); this.connectionsMap.delete(key); } disconnectAll(shouldRemoveKeysFromEvictionBlocklist?: boolean): void { Array.from(this.connectionsMap.values()).forEach((connection) => { - Onyx.disconnect(connection.id, shouldRemoveKeysFromEvictionBlocklist ? connection.onyxKey : undefined); + OnyxUtils.disconnectFromKey(connection.id, shouldRemoveKeysFromEvictionBlocklist ? connection.onyxKey : undefined); }); this.connectionsMap.clear(); diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index 01f7f220..a07af446 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -14,6 +14,7 @@ import Storage from './storage'; import type { CollectionKey, CollectionKeyBase, + ConnectOptions, DeepRecord, DefaultConnectCallback, DefaultConnectOptions, @@ -28,6 +29,8 @@ import type { } from './types'; import utils from './utils'; import type {WithOnyxState} from './withOnyx/types'; +import type {DeferredTask} from './createDeferredTask'; +import createDeferredTask from './createDeferredTask'; // Method constants const METHOD = { @@ -69,6 +72,12 @@ let batchUpdatesQueue: Array<() => void> = []; let snapshotKey: OnyxKey | null = null; +// Keeps track of the last connectionID that was used so we can keep incrementing it +let lastConnectionID = 0; + +// Connections can be made before `Onyx.init`. They would wait for this task before resolving +const deferredInitTask = createDeferredTask(); + function getSnapshotKey(): OnyxKey | null { return snapshotKey; } @@ -101,6 +110,13 @@ function getDefaultKeyStates(): Record> { return defaultKeyStates; } +/** + * Getter - returns the deffered init task. + */ +function getDeferredInitTask(): DeferredTask { + return deferredInitTask; +} + /** * Sets the initial values for the Onyx store * @@ -563,7 +579,7 @@ function keysChanged( // send the whole cached collection. if (isSubscribedToCollectionKey) { if (subscriber.waitForCollectionCallback) { - subscriber.callback(cachedCollection); + subscriber.callback(cachedCollection, undefined); continue; } @@ -743,7 +759,7 @@ function keyChanged( const cachedCollection = getCachedCollection(subscriber.key); cachedCollection[key] = value; - subscriber.callback(cachedCollection); + subscriber.callback(cachedCollection, undefined); continue; } @@ -1117,12 +1133,155 @@ function initializeWithDefaultKeyStates(): Promise { }); } +/** + * Subscribes a react component's state directly to a store key + * + * @example + * const connectionID = OnyxUtils.connectToKey({ + * key: ONYXKEYS.SESSION, + * callback: onSessionChange, + * }); + * + * @param mapping the mapping information to connect Onyx to the components state + * @param mapping.key ONYXKEY to subscribe to + * @param [mapping.statePropertyName] the name of the property in the state to connect the data to + * @param [mapping.withOnyxInstance] whose setState() method will be called with any changed data + * This is used by React components to connect to Onyx + * @param [mapping.callback] a method that will be called with changed data + * This is used by any non-React code to connect to Onyx + * @param [mapping.initWithStoredValues] If set to false, then no data will be prefilled into the + * component + * @param [mapping.waitForCollectionCallback] If set to true, it will return the entire collection to the callback as a single object + * @param [mapping.selector] THIS PARAM IS ONLY USED WITH withOnyx(). If included, this will be used to subscribe to a subset of an Onyx key's data. + * The sourceData and withOnyx state are passed to the selector and should return the simplified data. Using this setting on `withOnyx` can have very positive + * performance benefits because the component will only re-render when the subset of data changes. Otherwise, any change of data on any property would normally + * cause the component to re-render (and that can be expensive from a performance standpoint). + * @param [mapping.initialValue] THIS PARAM IS ONLY USED WITH withOnyx(). + * If included, this will be passed to the component so that something can be rendered while data is being fetched from the DB. + * Note that it will not cause the component to have the loading prop set to true. + * @returns an ID to use when calling `OnyxUtils.disconnectFromKey()` + */ +function connectToKey(connectOptions: ConnectOptions): number { + const mapping = connectOptions as Mapping; + const connectionID = lastConnectionID++; + callbackToStateMapping[connectionID] = mapping as Mapping; + callbackToStateMapping[connectionID].connectionID = connectionID; + + if (mapping.initWithStoredValues === false) { + return connectionID; + } + + // Commit connection only after init passes + deferredInitTask.promise + .then(() => OnyxUtils.addKeyToRecentlyAccessedIfNeeded(mapping)) + .then(() => { + // Performance improvement + // If the mapping is connected to an onyx key that is not a collection + // we can skip the call to getAllKeys() and return an array with a single item + if (Boolean(mapping.key) && typeof mapping.key === 'string' && !mapping.key.endsWith('_') && cache.getAllKeys().has(mapping.key)) { + return new Set([mapping.key]); + } + return OnyxUtils.getAllKeys(); + }) + .then((keys) => { + // We search all the keys in storage to see if any are a "match" for the subscriber we are connecting so that we + // can send data back to the subscriber. Note that multiple keys can match as a subscriber could either be + // subscribed to a "collection key" or a single key. + const matchingKeys: string[] = []; + keys.forEach((key) => { + if (!OnyxUtils.isKeyMatch(mapping.key, key)) { + return; + } + matchingKeys.push(key); + }); + // If the key being connected to does not exist we initialize the value with null. For subscribers that connected + // directly via connect() they will simply get a null value sent to them without any information about which key matched + // since there are none matched. In withOnyx() we wait for all connected keys to return a value before rendering the child + // component. This null value will be filtered out so that the connected component can utilize defaultProps. + if (matchingKeys.length === 0) { + if (mapping.key && !OnyxUtils.isCollectionKey(mapping.key)) { + cache.addNullishStorageKey(mapping.key); + } + + // Here we cannot use batching because the nullish value is expected to be set immediately for default props + // or they will be undefined. + OnyxUtils.sendDataToConnection(mapping, null, undefined, false); + return; + } + + // When using a callback subscriber we will either trigger the provided callback for each key we find or combine all values + // into an object and just make a single call. The latter behavior is enabled by providing a waitForCollectionCallback key + // combined with a subscription to a collection key. + if (typeof mapping.callback === 'function') { + if (OnyxUtils.isCollectionKey(mapping.key)) { + if (mapping.waitForCollectionCallback) { + OnyxUtils.getCollectionDataAndSendAsObject(matchingKeys, mapping); + return; + } + + // We did not opt into using waitForCollectionCallback mode so the callback is called for every matching key. + OnyxUtils.multiGet(matchingKeys).then((values) => { + values.forEach((val, key) => { + OnyxUtils.sendDataToConnection(mapping, val as OnyxValue, key as TKey, true); + }); + }); + return; + } + + // If we are not subscribed to a collection key then there's only a single key to send an update for. + OnyxUtils.get(mapping.key).then((val) => OnyxUtils.sendDataToConnection(mapping, val as OnyxValue, mapping.key, true)); + return; + } + + // If we have a withOnyxInstance that means a React component has subscribed via the withOnyx() HOC and we need to + // group collection key member data into an object. + if ('withOnyxInstance' in mapping && mapping.withOnyxInstance) { + if (OnyxUtils.isCollectionKey(mapping.key)) { + OnyxUtils.getCollectionDataAndSendAsObject(matchingKeys, mapping); + return; + } + + // If the subscriber is not using a collection key then we just send a single value back to the subscriber + OnyxUtils.get(mapping.key).then((val) => OnyxUtils.sendDataToConnection(mapping, val as OnyxValue, mapping.key, true)); + return; + } + + console.error('Warning: Onyx.connect() was found without a callback or withOnyxInstance'); + }); + + // The connectionID is returned back to the caller so that it can be used to clean up the connection when it's no longer needed + // by calling Onyx.disconnect(connectionID). + return connectionID; +} + +/** + * Remove the listener for a react component + * @example + * Onyx.disconnectFromKey(connectionID); + * + * @param connectionID unique id returned by call to `OnyxUtils.connectToKey()` + */ +function disconnectFromKey(connectionID: number, keyToRemoveFromEvictionBlocklist?: OnyxKey): void { + if (!callbackToStateMapping[connectionID]) { + return; + } + + // Remove this key from the eviction block list as we are no longer + // subscribing to it and it should be safe to delete again + if (keyToRemoveFromEvictionBlocklist) { + OnyxUtils.removeFromEvictionBlockList(keyToRemoveFromEvictionBlocklist, connectionID); + } + + delete callbackToStateMapping[connectionID]; +} + const OnyxUtils = { METHOD, getMergeQueue, getMergeQueuePromise, getCallbackToStateMapping, getDefaultKeyStates, + getDeferredInitTask, initStoreValues, sendActionToDevTools, maybeFlushBatchUpdates, @@ -1159,6 +1318,8 @@ const OnyxUtils = { initializeWithDefaultKeyStates, getSnapshotKey, multiGet, + connectToKey, + disconnectFromKey, }; export default OnyxUtils; diff --git a/lib/createDeferredTask.ts b/lib/createDeferredTask.ts index 3d37c91f..e697e1a7 100644 --- a/lib/createDeferredTask.ts +++ b/lib/createDeferredTask.ts @@ -17,3 +17,5 @@ export default function createDeferredTask(): DeferredTask { return deferred; } + +export type {DeferredTask}; diff --git a/lib/types.ts b/lib/types.ts index 4c7ea8f8..10718a75 100644 --- a/lib/types.ts +++ b/lib/types.ts @@ -269,7 +269,7 @@ type BaseConnectOptions = { type DefaultConnectCallback = (value: OnyxEntry, key: TKey) => void; /** Represents the callback function used in `Onyx.connect()` method with a collection key. */ -type CollectionConnectCallback = (value: NonUndefined>) => void; +type CollectionConnectCallback = (value: NonUndefined>, key: undefined) => void; /** Represents the options used in `Onyx.connect()` method with a regular key. */ type DefaultConnectOptions = BaseConnectOptions & { diff --git a/tests/unit/onyxCacheTest.tsx b/tests/unit/onyxCacheTest.tsx index 9487b0b4..a7624cfe 100644 --- a/tests/unit/onyxCacheTest.tsx +++ b/tests/unit/onyxCacheTest.tsx @@ -13,6 +13,7 @@ import type OnyxInstance from '../../lib/Onyx'; import type withOnyxType from '../../lib/withOnyx'; import type {InitOptions} from '../../lib/types'; import generateRange from '../utils/generateRange'; +import type {ConnectionMetadata} from '../../lib/OnyxConnectionManager'; describe('Onyx', () => { describe('Cache Service', () => { @@ -528,7 +529,7 @@ describe('Onyx', () => { StorageMock.getItem.mockResolvedValue('"mockValue"'); const range = generateRange(0, 10); StorageMock.getAllKeys.mockResolvedValue(range.map((number) => `${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}${number}`)); - let connections: Array<{key: string; connectionId: number}> = []; + let connections: Array<{key: string; connection: ConnectionMetadata}> = []; // Given Onyx is configured with max 5 keys in cache return initOnyx({ @@ -540,14 +541,14 @@ describe('Onyx', () => { const key = `${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}${number}`; return { key, - connectionId: Onyx.connect({key, callback: jest.fn()}), + connection: Onyx.connect({key, callback: jest.fn()}), }; }); }) .then(waitForPromisesToResolve) .then(() => { // When a new connection for a safe eviction key happens - Onyx.connect({key: `${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}9`, callback: jest.fn()}); + Onyx.connect({key: `${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}10`, callback: jest.fn()}); }) .then(() => { // Then the most recent 5 keys should remain in cache diff --git a/tests/unit/onyxClearNativeStorageTest.ts b/tests/unit/onyxClearNativeStorageTest.ts index 28605adc..5c6aea9d 100644 --- a/tests/unit/onyxClearNativeStorageTest.ts +++ b/tests/unit/onyxClearNativeStorageTest.ts @@ -2,6 +2,7 @@ import waitForPromisesToResolve from '../utils/waitForPromisesToResolve'; import StorageMock from '../../lib/storage'; import Onyx from '../../lib/Onyx'; import type OnyxCache from '../../lib/OnyxCache'; +import type {ConnectionMetadata} from '../../lib/OnyxConnectionManager'; const ONYX_KEYS = { DEFAULT_KEY: 'defaultKey', @@ -14,7 +15,7 @@ const MERGED_VALUE = 1; const DEFAULT_VALUE = 0; describe('Set data while storage is clearing', () => { - let connectionID: number; + let connection: ConnectionMetadata | undefined; let onyxValue: unknown; /** @type OnyxCache */ @@ -31,7 +32,7 @@ describe('Set data while storage is clearing', () => { [ONYX_KEYS.DEFAULT_KEY]: DEFAULT_VALUE, }, }); - connectionID = Onyx.connect({ + connection = Onyx.connect({ key: ONYX_KEYS.DEFAULT_KEY, initWithStoredValues: false, callback: (val) => (onyxValue = val), @@ -40,7 +41,9 @@ describe('Set data while storage is clearing', () => { }); afterEach(() => { - Onyx.disconnect(connectionID); + if (connection) { + Onyx.disconnect(connection); + } return Onyx.clear(); }); diff --git a/tests/unit/onyxClearWebStorageTest.ts b/tests/unit/onyxClearWebStorageTest.ts index 6fb52fbc..b97338ce 100644 --- a/tests/unit/onyxClearWebStorageTest.ts +++ b/tests/unit/onyxClearWebStorageTest.ts @@ -3,6 +3,7 @@ import StorageMock from '../../lib/storage'; import Onyx from '../../lib/Onyx'; import type OnyxCache from '../../lib/OnyxCache'; import type GenericCollection from '../utils/GenericCollection'; +import type {ConnectionMetadata} from '../../lib/OnyxConnectionManager'; const ONYX_KEYS = { DEFAULT_KEY: 'defaultKey', @@ -16,7 +17,7 @@ const MERGED_VALUE = 'merged'; const DEFAULT_VALUE = 'default'; describe('Set data while storage is clearing', () => { - let connectionID: number; + let connection: ConnectionMetadata | undefined; let onyxValue: unknown; /** @type OnyxCache */ @@ -33,7 +34,7 @@ describe('Set data while storage is clearing', () => { [ONYX_KEYS.DEFAULT_KEY]: DEFAULT_VALUE, }, }); - connectionID = Onyx.connect({ + connection = Onyx.connect({ key: ONYX_KEYS.DEFAULT_KEY, initWithStoredValues: false, callback: (val) => (onyxValue = val), @@ -42,7 +43,9 @@ describe('Set data while storage is clearing', () => { }); afterEach(() => { - Onyx.disconnect(connectionID); + if (connection) { + Onyx.disconnect(connection); + } return Onyx.clear(); }); @@ -129,7 +132,7 @@ describe('Set data while storage is clearing', () => { // Given a mocked callback function and a collection with four items in it const collectionCallback = jest.fn(); - const testConnectionID = Onyx.connect({ + const testConnection = Onyx.connect({ key: ONYX_KEYS.COLLECTION.TEST, waitForCollectionCallback: true, callback: collectionCallback, @@ -148,7 +151,7 @@ describe('Set data while storage is clearing', () => { // When onyx is cleared .then(() => Onyx.clear()) .then(() => { - Onyx.disconnect(testConnectionID); + Onyx.disconnect(testConnection); }) .then(() => { // Then the collection callback should only have been called three times: @@ -159,13 +162,17 @@ describe('Set data while storage is clearing', () => { // And it should be called with the expected parameters each time expect(collectionCallback).toHaveBeenNthCalledWith(1, undefined, undefined); - expect(collectionCallback).toHaveBeenNthCalledWith(2, { - test_1: 1, - test_2: 2, - test_3: 3, - test_4: 4, - }); - expect(collectionCallback).toHaveBeenLastCalledWith({}); + expect(collectionCallback).toHaveBeenNthCalledWith( + 2, + { + test_1: 1, + test_2: 2, + test_3: 3, + test_4: 4, + }, + undefined, + ); + expect(collectionCallback).toHaveBeenLastCalledWith({}, undefined); }) ); }); diff --git a/tests/unit/onyxTest.ts b/tests/unit/onyxTest.ts index 07ecc7df..a6a2a5f7 100644 --- a/tests/unit/onyxTest.ts +++ b/tests/unit/onyxTest.ts @@ -5,6 +5,7 @@ import OnyxUtils from '../../lib/OnyxUtils'; import type OnyxCache from '../../lib/OnyxCache'; import type {OnyxCollection, OnyxUpdate} from '../../lib/types'; import type GenericCollection from '../utils/GenericCollection'; +import type {ConnectionMetadata} from '../../lib/OnyxConnectionManager'; const ONYX_KEYS = { TEST_KEY: 'test', @@ -27,7 +28,7 @@ Onyx.init({ }); describe('Onyx', () => { - let connectionID: number; + let connection: ConnectionMetadata | undefined; /** @type OnyxCache */ let cache: typeof OnyxCache; @@ -38,7 +39,9 @@ describe('Onyx', () => { }); afterEach(() => { - Onyx.disconnect(connectionID); + if (connection) { + Onyx.disconnect(connection); + } return Onyx.clear(); }); @@ -63,7 +66,7 @@ describe('Onyx', () => { it('should set a simple key', () => { let testKeyValue: unknown; - connectionID = Onyx.connect({ + connection = Onyx.connect({ key: ONYX_KEYS.TEST_KEY, initWithStoredValues: false, callback: (value) => { @@ -80,7 +83,7 @@ describe('Onyx', () => { it('should not set the key if the value is incompatible (array vs object)', () => { let testKeyValue: unknown; - connectionID = Onyx.connect({ + connection = Onyx.connect({ key: ONYX_KEYS.TEST_KEY, initWithStoredValues: false, callback: (value) => { @@ -101,7 +104,7 @@ describe('Onyx', () => { it('should merge an object with another object', () => { let testKeyValue: unknown; - connectionID = Onyx.connect({ + connection = Onyx.connect({ key: ONYX_KEYS.TEST_KEY, initWithStoredValues: false, callback: (value) => { @@ -122,7 +125,7 @@ describe('Onyx', () => { it('should not merge if the value is incompatible (array vs object)', () => { let testKeyValue: unknown; - connectionID = Onyx.connect({ + connection = Onyx.connect({ key: ONYX_KEYS.TEST_KEY, initWithStoredValues: false, callback: (value) => { @@ -142,7 +145,7 @@ describe('Onyx', () => { it('should notify subscribers when data has been cleared', () => { let testKeyValue: unknown; - connectionID = Onyx.connect({ + connection = Onyx.connect({ key: ONYX_KEYS.TEST_KEY, initWithStoredValues: false, callback: (value) => { @@ -151,7 +154,7 @@ describe('Onyx', () => { }); let otherTestValue: unknown; - const otherTestConnectionID = Onyx.connect({ + const otherTestConnection = Onyx.connect({ key: ONYX_KEYS.OTHER_TEST, callback: (value) => { otherTestValue = value; @@ -171,13 +174,13 @@ describe('Onyx', () => { // Other test key should be returned to its default state expect(otherTestValue).toBe(42); - return Onyx.disconnect(otherTestConnectionID); + return Onyx.disconnect(otherTestConnection); }); }); it('should not notify subscribers after they have disconnected', () => { let testKeyValue: unknown; - connectionID = Onyx.connect({ + connection = Onyx.connect({ key: ONYX_KEYS.TEST_KEY, initWithStoredValues: false, callback: (value) => { @@ -188,7 +191,9 @@ describe('Onyx', () => { return Onyx.set(ONYX_KEYS.TEST_KEY, 'test') .then(() => { expect(testKeyValue).toBe('test'); - Onyx.disconnect(connectionID); + if (connection) { + Onyx.disconnect(connection); + } return Onyx.set(ONYX_KEYS.TEST_KEY, 'test updated'); }) .then(() => { @@ -199,7 +204,7 @@ describe('Onyx', () => { it('should merge arrays by replacing previous value with new value', () => { let testKeyValue: unknown; - connectionID = Onyx.connect({ + connection = Onyx.connect({ key: ONYX_KEYS.TEST_KEY, initWithStoredValues: false, callback: (value) => { @@ -220,7 +225,7 @@ describe('Onyx', () => { it('should merge 2 objects when it has no initial stored value for test key', () => { let testKeyValue: unknown; - connectionID = Onyx.connect({ + connection = Onyx.connect({ key: ONYX_KEYS.TEST_KEY, initWithStoredValues: false, callback: (value) => { @@ -236,7 +241,7 @@ describe('Onyx', () => { it('should merge 2 arrays when it has no initial stored value for test key', () => { let testKeyValue: unknown; - connectionID = Onyx.connect({ + connection = Onyx.connect({ key: ONYX_KEYS.TEST_KEY, initWithStoredValues: false, callback: (value) => { @@ -253,7 +258,7 @@ describe('Onyx', () => { it('should remove keys that are set to null when merging', () => { let testKeyValue: unknown; - connectionID = Onyx.connect({ + connection = Onyx.connect({ key: ONYX_KEYS.TEST_KEY, initWithStoredValues: false, callback: (value) => { @@ -314,7 +319,7 @@ describe('Onyx', () => { it('should ignore top-level and remove nested `undefined` values in Onyx.set', () => { let testKeyValue: unknown; - connectionID = Onyx.connect({ + connection = Onyx.connect({ key: ONYX_KEYS.TEST_KEY, initWithStoredValues: false, callback: (value) => { @@ -362,7 +367,7 @@ describe('Onyx', () => { it('should ignore top-level and remove nested `undefined` values in Onyx.multiSet', () => { let testKeyValue: unknown; - connectionID = Onyx.connect({ + connection = Onyx.connect({ key: ONYX_KEYS.TEST_KEY, initWithStoredValues: false, callback: (value) => { @@ -371,7 +376,7 @@ describe('Onyx', () => { }); let otherTestKeyValue: unknown; - connectionID = Onyx.connect({ + connection = Onyx.connect({ key: ONYX_KEYS.OTHER_TEST, initWithStoredValues: false, callback: (value) => { @@ -421,7 +426,7 @@ describe('Onyx', () => { it('should ignore top-level and remove nested `undefined` values in Onyx.merge', () => { let testKeyValue: unknown; - connectionID = Onyx.connect({ + connection = Onyx.connect({ key: ONYX_KEYS.TEST_KEY, initWithStoredValues: false, callback: (value) => { @@ -468,7 +473,7 @@ describe('Onyx', () => { const holidayRoute = `${ONYX_KEYS.COLLECTION.TEST_KEY}holiday`; const workRoute = `${ONYX_KEYS.COLLECTION.TEST_KEY}work`; - connectionID = Onyx.connect({ + connection = Onyx.connect({ key: ONYX_KEYS.COLLECTION.TEST_KEY, initWithStoredValues: false, callback: (value) => (result = value), @@ -503,7 +508,7 @@ describe('Onyx', () => { it('should overwrite an array key nested inside an object', () => { let testKeyValue: unknown; - connectionID = Onyx.connect({ + connection = Onyx.connect({ key: ONYX_KEYS.TEST_KEY, initWithStoredValues: false, callback: (value) => { @@ -519,7 +524,7 @@ describe('Onyx', () => { it('should overwrite an array key nested inside an object when using merge on a collection', () => { let testKeyValue: unknown; - connectionID = Onyx.connect({ + connection = Onyx.connect({ key: ONYX_KEYS.COLLECTION.TEST_KEY, initWithStoredValues: false, callback: (value) => { @@ -536,7 +541,7 @@ describe('Onyx', () => { it('should properly set and merge when using mergeCollection', () => { const valuesReceived: Record = {}; const mockCallback = jest.fn((data) => (valuesReceived[data.ID] = data.value)); - connectionID = Onyx.connect({ + connection = Onyx.connect({ key: ONYX_KEYS.COLLECTION.TEST_KEY, initWithStoredValues: false, callback: mockCallback, @@ -600,7 +605,7 @@ describe('Onyx', () => { it('should skip the update when a key not belonging to collection key is present in mergeCollection', () => { const valuesReceived: Record = {}; - connectionID = Onyx.connect({ + connection = Onyx.connect({ key: ONYX_KEYS.COLLECTION.TEST_KEY, initWithStoredValues: false, callback: (data, key) => (valuesReceived[key] = data), @@ -613,7 +618,7 @@ describe('Onyx', () => { it('should return full object to callback when calling mergeCollection()', () => { const valuesReceived: Record = {}; - connectionID = Onyx.connect({ + connection = Onyx.connect({ key: ONYX_KEYS.COLLECTION.TEST_KEY, initWithStoredValues: false, callback: (data, key) => (valuesReceived[key] = data), @@ -657,7 +662,7 @@ describe('Onyx', () => { it('should use update data object to set/merge keys', () => { let testKeyValue: unknown; - connectionID = Onyx.connect({ + connection = Onyx.connect({ key: ONYX_KEYS.TEST_KEY, initWithStoredValues: false, callback: (value) => { @@ -666,7 +671,7 @@ describe('Onyx', () => { }); let otherTestKeyValue: unknown; - connectionID = Onyx.connect({ + connection = Onyx.connect({ key: ONYX_KEYS.OTHER_TEST, initWithStoredValues: false, callback: (value) => { @@ -709,7 +714,7 @@ describe('Onyx', () => { it('should use update data object to merge a collection of keys', () => { const valuesReceived: Record = {}; const mockCallback = jest.fn((data) => (valuesReceived[data.ID] = data.value)); - connectionID = Onyx.connect({ + connection = Onyx.connect({ key: ONYX_KEYS.COLLECTION.TEST_KEY, initWithStoredValues: false, callback: mockCallback, @@ -821,7 +826,7 @@ describe('Onyx', () => { it('should properly set all keys provided in a multiSet called via update', () => { const valuesReceived: Record = {}; - connectionID = Onyx.connect({ + connection = Onyx.connect({ key: ONYX_KEYS.COLLECTION.TEST_KEY, initWithStoredValues: false, callback: (data, key) => (valuesReceived[key] = data), @@ -914,7 +919,7 @@ describe('Onyx', () => { return Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_CONNECT_COLLECTION, initialCollectionData as GenericCollection) .then(() => { // When we connect to that collection with waitForCollectionCallback = true - connectionID = Onyx.connect({ + connection = Onyx.connect({ key: ONYX_KEYS.COLLECTION.TEST_CONNECT_COLLECTION, waitForCollectionCallback: true, callback: mockCallback, @@ -936,7 +941,7 @@ describe('Onyx', () => { }; // Given an Onyx.connect call with waitForCollectionCallback=true - connectionID = Onyx.connect({ + connection = Onyx.connect({ key: ONYX_KEYS.COLLECTION.TEST_POLICY, waitForCollectionCallback: true, callback: mockCallback, @@ -953,7 +958,7 @@ describe('Onyx', () => { expect(mockCallback).toHaveBeenNthCalledWith(1, undefined, undefined); // AND the value for the second call should be collectionUpdate since the collection was updated - expect(mockCallback).toHaveBeenNthCalledWith(2, collectionUpdate); + expect(mockCallback).toHaveBeenNthCalledWith(2, collectionUpdate, undefined); }) ); }); @@ -966,7 +971,7 @@ describe('Onyx', () => { }; // Given an Onyx.connect call with waitForCollectionCallback=false - connectionID = Onyx.connect({ + connection = Onyx.connect({ key: `${ONYX_KEYS.COLLECTION.TEST_POLICY}${1}`, callback: mockCallback, }); @@ -994,7 +999,7 @@ describe('Onyx', () => { }; // Given an Onyx.connect call with waitForCollectionCallback=true - connectionID = Onyx.connect({ + connection = Onyx.connect({ key: ONYX_KEYS.COLLECTION.TEST_POLICY, waitForCollectionCallback: true, callback: mockCallback, @@ -1008,7 +1013,7 @@ describe('Onyx', () => { expect(mockCallback).toHaveBeenCalledTimes(2); // AND the value for the second call should be collectionUpdate - expect(mockCallback).toHaveBeenLastCalledWith(collectionUpdate); + expect(mockCallback).toHaveBeenLastCalledWith(collectionUpdate, undefined); }) ); }); @@ -1029,7 +1034,7 @@ describe('Onyx', () => { }; // Given an Onyx.connect call with waitForCollectionCallback=true - connectionID = Onyx.connect({ + connection = Onyx.connect({ key: ONYX_KEYS.COLLECTION.TEST_POLICY, waitForCollectionCallback: true, callback: mockCallback, @@ -1043,7 +1048,7 @@ describe('Onyx', () => { expect(mockCallback).toHaveBeenCalledTimes(2); // And the value for the second call should be collectionUpdate - expect(mockCallback).toHaveBeenNthCalledWith(2, collectionUpdate); + expect(mockCallback).toHaveBeenNthCalledWith(2, collectionUpdate, undefined); }) // When merge is called again with the same collection not modified @@ -1069,7 +1074,7 @@ describe('Onyx', () => { }; // Given an Onyx.connect call with waitForCollectionCallback=true - connectionID = Onyx.connect({ + connection = Onyx.connect({ key: ONYX_KEYS.COLLECTION.TEST_POLICY, waitForCollectionCallback: true, callback: mockCallback, @@ -1084,7 +1089,7 @@ describe('Onyx', () => { expect(mockCallback).toHaveBeenCalledTimes(1); // And the value for the second call should be collectionUpdate - expect(mockCallback).toHaveBeenNthCalledWith(1, collectionUpdate); + expect(mockCallback).toHaveBeenNthCalledWith(1, collectionUpdate, undefined); }) // When merge is called again with the same collection not modified @@ -1104,16 +1109,16 @@ describe('Onyx', () => { }); it('should return a promise that completes when all update() operations are done', () => { - const connectionIDs: number[] = []; + const connections: ConnectionMetadata[] = []; const testCallback = jest.fn(); const otherTestCallback = jest.fn(); const collectionCallback = jest.fn(); const itemKey = `${ONYX_KEYS.COLLECTION.TEST_UPDATE}1`; - connectionIDs.push(Onyx.connect({key: ONYX_KEYS.TEST_KEY, callback: testCallback})); - connectionIDs.push(Onyx.connect({key: ONYX_KEYS.OTHER_TEST, callback: otherTestCallback})); - connectionIDs.push(Onyx.connect({key: ONYX_KEYS.COLLECTION.TEST_UPDATE, callback: collectionCallback, waitForCollectionCallback: true})); + connections.push(Onyx.connect({key: ONYX_KEYS.TEST_KEY, callback: testCallback})); + connections.push(Onyx.connect({key: ONYX_KEYS.OTHER_TEST, callback: otherTestCallback})); + connections.push(Onyx.connect({key: ONYX_KEYS.COLLECTION.TEST_UPDATE, callback: collectionCallback, waitForCollectionCallback: true})); return waitForPromisesToResolve().then(() => Onyx.update([ {onyxMethod: Onyx.METHOD.SET, key: ONYX_KEYS.TEST_KEY, value: 'taco'}, @@ -1122,7 +1127,7 @@ describe('Onyx', () => { ]).then(() => { expect(collectionCallback).toHaveBeenCalledTimes(2); expect(collectionCallback).toHaveBeenNthCalledWith(1, undefined, undefined); - expect(collectionCallback).toHaveBeenNthCalledWith(2, {[itemKey]: {a: 'a'}}); + expect(collectionCallback).toHaveBeenNthCalledWith(2, {[itemKey]: {a: 'a'}}, undefined); expect(testCallback).toHaveBeenCalledTimes(2); expect(testCallback).toHaveBeenNthCalledWith(1, undefined, undefined); @@ -1132,7 +1137,7 @@ describe('Onyx', () => { // We set an initial value of 42 for ONYX_KEYS.OTHER_TEST in Onyx.init() expect(otherTestCallback).toHaveBeenNthCalledWith(1, 42, ONYX_KEYS.OTHER_TEST); expect(otherTestCallback).toHaveBeenNthCalledWith(2, 'pizza', ONYX_KEYS.OTHER_TEST); - connectionIDs.forEach((id) => Onyx.disconnect(id)); + connections.forEach((id) => Onyx.disconnect(id)); }), ); }); @@ -1140,7 +1145,7 @@ describe('Onyx', () => { it('should merge an object with a batch of objects and undefined', () => { let testKeyValue: unknown; - connectionID = Onyx.connect({ + connection = Onyx.connect({ key: ONYX_KEYS.TEST_KEY, initWithStoredValues: false, callback: (value) => { @@ -1171,7 +1176,7 @@ describe('Onyx', () => { it('should merge an object with null and overwrite the value', () => { let testKeyValue: unknown; - connectionID = Onyx.connect({ + connection = Onyx.connect({ key: ONYX_KEYS.TEST_KEY, initWithStoredValues: false, callback: (value) => { @@ -1198,7 +1203,7 @@ describe('Onyx', () => { it('should merge a key with null and allow subsequent updates', () => { let testKeyValue: unknown; - connectionID = Onyx.connect({ + connection = Onyx.connect({ key: ONYX_KEYS.TEST_KEY, initWithStoredValues: false, callback: (value) => { @@ -1224,7 +1229,7 @@ describe('Onyx', () => { it("should not set null values in Onyx.merge, when the key doesn't exist yet", () => { let testKeyValue: unknown; - connectionID = Onyx.connect({ + connection = Onyx.connect({ key: ONYX_KEYS.TEST_KEY, initWithStoredValues: false, callback: (value) => { @@ -1250,7 +1255,7 @@ describe('Onyx', () => { it('should apply updates in order with Onyx.update', () => { let testKeyValue: unknown; - connectionID = Onyx.connect({ + connection = Onyx.connect({ key: ONYX_KEYS.TEST_KEY, initWithStoredValues: false, callback: (value) => { @@ -1286,7 +1291,7 @@ describe('Onyx', () => { const routineRoute = `${ONYX_KEYS.COLLECTION.TEST_KEY}routine`; const holidayRoute = `${ONYX_KEYS.COLLECTION.TEST_KEY}holiday`; - connectionID = Onyx.connect({ + connection = Onyx.connect({ key: ONYX_KEYS.COLLECTION.TEST_KEY, initWithStoredValues: false, callback: (value) => (result = value), @@ -1328,7 +1333,7 @@ describe('Onyx', () => { }); it('should not call a collection item subscriber if the value did not change', () => { - const connectionIDs: number[] = []; + const connections: ConnectionMetadata[] = []; const cat = `${ONYX_KEYS.COLLECTION.ANIMALS}cat`; const dog = `${ONYX_KEYS.COLLECTION.ANIMALS}dog`; @@ -1337,15 +1342,15 @@ describe('Onyx', () => { const catCallback = jest.fn(); const dogCallback = jest.fn(); - connectionIDs.push( + connections.push( Onyx.connect({ key: ONYX_KEYS.COLLECTION.ANIMALS, callback: collectionCallback, waitForCollectionCallback: true, }), ); - connectionIDs.push(Onyx.connect({key: cat, callback: catCallback})); - connectionIDs.push(Onyx.connect({key: dog, callback: dogCallback})); + connections.push(Onyx.connect({key: cat, callback: catCallback})); + connections.push(Onyx.connect({key: dog, callback: dogCallback})); const initialValue = {name: 'Fluffy'}; @@ -1361,12 +1366,12 @@ describe('Onyx', () => { }) .then(() => { expect(collectionCallback).toHaveBeenCalledTimes(3); - expect(collectionCallback).toHaveBeenCalledWith(collectionDiff); + expect(collectionCallback).toHaveBeenCalledWith(collectionDiff, undefined); expect(catCallback).toHaveBeenCalledTimes(2); expect(dogCallback).toHaveBeenCalledTimes(2); - connectionIDs.map((id) => Onyx.disconnect(id)); + connections.map((id) => Onyx.disconnect(id)); }); }); From 4fa813d62db4c2c1dfef02bbf35bb2977649c746 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Wed, 3 Jul 2024 10:35:52 +0100 Subject: [PATCH 07/31] Improve connection manager --- lib/OnyxConnectionManager.ts | 16 +++++++++----- lib/index.ts | 2 ++ lib/types.ts | 2 ++ tests/unit/OnyxConnectionManagerTest.ts | 29 +++++++++++++++++++++++++ 4 files changed, 44 insertions(+), 5 deletions(-) diff --git a/lib/OnyxConnectionManager.ts b/lib/OnyxConnectionManager.ts index f239f49f..72c7152f 100644 --- a/lib/OnyxConnectionManager.ts +++ b/lib/OnyxConnectionManager.ts @@ -36,6 +36,10 @@ class OnyxConnectionManager { private connectionMapKey(connectOptions: ConnectOptions): string { let suffix = ''; + if (connectOptions.reuseConnection === false) { + suffix += `,uniqueID=${Str.guid()}`; + } + if ('withOnyxInstance' in connectOptions) { suffix += `,withOnyxInstance=${Str.guid()}`; } @@ -124,7 +128,9 @@ class OnyxConnectionManager { } if (connection.isConnectionMade) { - (connectOptions as DefaultConnectOptions).callback?.(connection.cachedCallbackValue, connection.cachedCallbackKey as OnyxKey); + Promise.resolve().then(() => { + (connectOptions as DefaultConnectOptions).callback?.(connection.cachedCallbackValue, connection.cachedCallbackKey as OnyxKey); + }); } return {key: mapKey, callbackID, connectionID: connection.id}; @@ -137,18 +143,18 @@ class OnyxConnectionManager { * * @param connection connection metadata object returned by call to `Onyx.connect()` */ - disconnect({key, callbackID}: ConnectionMetadata, shouldRemoveKeyFromEvictionBlocklist?: boolean): void { - const connection = this.connectionsMap.get(key); + disconnect(connectionMetadada: ConnectionMetadata, shouldRemoveKeyFromEvictionBlocklist?: boolean): void { + const connection = this.connectionsMap.get(connectionMetadada.key); if (!connection) { return; } - connection.callbacks.delete(callbackID); + connection.callbacks.delete(connectionMetadada.callbackID); if (connection.callbacks.size === 0) { OnyxUtils.disconnectFromKey(connection.id, shouldRemoveKeyFromEvictionBlocklist ? connection.onyxKey : undefined); - this.connectionsMap.delete(key); + this.connectionsMap.delete(connectionMetadada.key); } } diff --git a/lib/index.ts b/lib/index.ts index 04ec3fb9..65e37d1a 100644 --- a/lib/index.ts +++ b/lib/index.ts @@ -18,6 +18,7 @@ import type { OnyxMergeCollectionInput, } from './types'; import type {FetchStatus, ResultMetadata, UseOnyxResult} from './useOnyx'; +import type {ConnectionMetadata} from './OnyxConnectionManager'; import useOnyx from './useOnyx'; import withOnyx from './withOnyx'; import type {WithOnyxState} from './withOnyx/types'; @@ -46,4 +47,5 @@ export type { Selector, UseOnyxResult, WithOnyxState, + ConnectionMetadata, }; diff --git a/lib/types.ts b/lib/types.ts index 10718a75..074e44fe 100644 --- a/lib/types.ts +++ b/lib/types.ts @@ -276,6 +276,7 @@ type DefaultConnectOptions = BaseConnectOptions & { key: TKey; callback?: DefaultConnectCallback; waitForCollectionCallback?: false; + reuseConnection?: boolean; }; /** Represents the options used in `Onyx.connect()` method with a collection key. */ @@ -283,6 +284,7 @@ type CollectionConnectOptions = BaseConnectOptions & { key: TKey extends CollectionKeyBase ? TKey : never; callback?: CollectionConnectCallback; waitForCollectionCallback: true; + reuseConnection?: boolean; }; /** diff --git a/tests/unit/OnyxConnectionManagerTest.ts b/tests/unit/OnyxConnectionManagerTest.ts index 6fb3818e..64c103b7 100644 --- a/tests/unit/OnyxConnectionManagerTest.ts +++ b/tests/unit/OnyxConnectionManagerTest.ts @@ -135,6 +135,8 @@ describe('OnyxConnectionManager', () => { const callback4 = jest.fn(); connectionManager.connect({key: ONYXKEYS.TEST_KEY, callback: callback4}); + await act(async () => waitForPromisesToResolve()); + expect(callback2).toHaveBeenCalledTimes(1); expect(callback2).toHaveBeenCalledWith('test', ONYXKEYS.TEST_KEY); expect(callback3).toHaveBeenCalledTimes(1); @@ -142,6 +144,33 @@ describe('OnyxConnectionManager', () => { expect(callback4).toHaveBeenCalledTimes(1); expect(callback4).toHaveBeenCalledWith('test', ONYXKEYS.TEST_KEY); }); + + it('should have the connection object already defined when triggering the callback of the second connection to the same key', async () => { + await StorageMock.setItem(ONYXKEYS.TEST_KEY, 'test'); + + const callback1 = jest.fn(); + connectionManager.connect({key: ONYXKEYS.TEST_KEY, callback: callback1}); + + await act(async () => waitForPromisesToResolve()); + + expect(callback1).toHaveBeenCalledTimes(1); + expect(callback1).toHaveBeenCalledWith('test', ONYXKEYS.TEST_KEY); + + const callback2 = jest.fn(); + const connection2 = connectionManager.connect({ + key: ONYXKEYS.TEST_KEY, + callback: (...params) => { + callback2(...params); + connectionManager.disconnect(connection2); + }, + }); + + await act(async () => waitForPromisesToResolve()); + + expect(callback2).toHaveBeenCalledTimes(1); + expect(callback2).toHaveBeenCalledWith('test', ONYXKEYS.TEST_KEY); + expect(connectionsMap.size).toEqual(1); + }); }); describe('withOnyx', () => { From 86a53473544175df9633ff8b604abcaf731e76fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Wed, 3 Jul 2024 10:43:07 +0100 Subject: [PATCH 08/31] Add fail safe logic to disconnect --- lib/OnyxConnectionManager.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/lib/OnyxConnectionManager.ts b/lib/OnyxConnectionManager.ts index 72c7152f..8731e9b8 100644 --- a/lib/OnyxConnectionManager.ts +++ b/lib/OnyxConnectionManager.ts @@ -1,4 +1,5 @@ import bindAll from 'lodash/bindAll'; +import * as Logger from './Logger'; import type {ConnectOptions} from './Onyx'; import OnyxUtils from './OnyxUtils'; import * as Str from './Str'; @@ -139,11 +140,16 @@ class OnyxConnectionManager { /** * Remove the listener for a react component * @example - * Onyx.disconnectFromKey(connection); + * Onyx.disconnect(connection); * * @param connection connection metadata object returned by call to `Onyx.connect()` */ disconnect(connectionMetadada: ConnectionMetadata, shouldRemoveKeyFromEvictionBlocklist?: boolean): void { + if (!connectionMetadada) { + Logger.logAlert(`[ConnectionManager] Attempted to disconnect passing an undefined metadata object.`); + return; + } + const connection = this.connectionsMap.get(connectionMetadada.key); if (!connection) { From a45fa57ce5a152e457ce07ed1c4f35f61dfad190 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Wed, 3 Jul 2024 10:46:18 +0100 Subject: [PATCH 09/31] Unify reuseConnection and withOnyxInstance conditions --- lib/OnyxConnectionManager.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/lib/OnyxConnectionManager.ts b/lib/OnyxConnectionManager.ts index 8731e9b8..50428710 100644 --- a/lib/OnyxConnectionManager.ts +++ b/lib/OnyxConnectionManager.ts @@ -37,14 +37,10 @@ class OnyxConnectionManager { private connectionMapKey(connectOptions: ConnectOptions): string { let suffix = ''; - if (connectOptions.reuseConnection === false) { + if (connectOptions.reuseConnection === false || 'withOnyxInstance' in connectOptions) { suffix += `,uniqueID=${Str.guid()}`; } - if ('withOnyxInstance' in connectOptions) { - suffix += `,withOnyxInstance=${Str.guid()}`; - } - return `key=${connectOptions.key},initWithStoredValues=${connectOptions.initWithStoredValues ?? true},waitForCollectionCallback=${ connectOptions.waitForCollectionCallback ?? false }${suffix}`; From d0753f6288cf30664c19314f9bef9ef4570f3905 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Wed, 3 Jul 2024 11:21:52 +0100 Subject: [PATCH 10/31] Keep connect and disconnect functions to avoid TS error --- lib/Onyx.ts | 48 ++++++++++++++++++++++++++++++++++-- lib/OnyxConnectionManager.ts | 28 --------------------- 2 files changed, 46 insertions(+), 30 deletions(-) diff --git a/lib/Onyx.ts b/lib/Onyx.ts index 0769a517..962164fd 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -26,6 +26,7 @@ import type { } from './types'; import OnyxUtils from './OnyxUtils'; import logMessages from './logMessages'; +import type {ConnectionMetadata} from './OnyxConnectionManager'; import connectionManager from './OnyxConnectionManager'; /** Initialize the store with actions and listening for storage events */ @@ -61,6 +62,49 @@ function init({ Promise.all([OnyxUtils.addAllSafeEvictionKeysToRecentlyAccessedList(), OnyxUtils.initializeWithDefaultKeyStates()]).then(OnyxUtils.getDeferredInitTask().resolve); } +/** + * Subscribes a react component's state directly to a store key + * + * @example + * const connection = Onyx.connect({ + * key: ONYXKEYS.SESSION, + * callback: onSessionChange, + * }); + * + * @param mapping the mapping information to connect Onyx to the components state + * @param mapping.key ONYXKEY to subscribe to + * @param [mapping.statePropertyName] the name of the property in the state to connect the data to + * @param [mapping.withOnyxInstance] whose setState() method will be called with any changed data + * This is used by React components to connect to Onyx + * @param [mapping.callback] a method that will be called with changed data + * This is used by any non-React code to connect to Onyx + * @param [mapping.initWithStoredValues] If set to false, then no data will be prefilled into the + * component + * @param [mapping.waitForCollectionCallback] If set to true, it will return the entire collection to the callback as a single object + * @param [mapping.selector] THIS PARAM IS ONLY USED WITH withOnyx(). If included, this will be used to subscribe to a subset of an Onyx key's data. + * The sourceData and withOnyx state are passed to the selector and should return the simplified data. Using this setting on `withOnyx` can have very positive + * performance benefits because the component will only re-render when the subset of data changes. Otherwise, any change of data on any property would normally + * cause the component to re-render (and that can be expensive from a performance standpoint). + * @param [mapping.initialValue] THIS PARAM IS ONLY USED WITH withOnyx(). + * If included, this will be passed to the component so that something can be rendered while data is being fetched from the DB. + * Note that it will not cause the component to have the loading prop set to true. + * @returns a connection metadata object to use when calling `Onyx.disconnect()` + */ +function connect(connectOptions: ConnectOptions): ConnectionMetadata { + return connectionManager.connect(connectOptions); +} + +/** + * Remove the listener for a react component + * @example + * Onyx.disconnect(connection); + * + * @param connection connection metadata object returned by call to `Onyx.connect()` + */ +function disconnect(connectionMetadada: ConnectionMetadata, shouldRemoveKeyFromEvictionBlocklist?: boolean): void { + connectionManager.disconnect(connectionMetadada, shouldRemoveKeyFromEvictionBlocklist); +} + /** * Write a value to our store with the given key * @@ -590,8 +634,8 @@ function update(data: OnyxUpdate[]): Promise { const Onyx = { METHOD: OnyxUtils.METHOD, - connect: connectionManager.connect, - disconnect: connectionManager.disconnect, + connect, + disconnect, set, multiSet, merge, diff --git a/lib/OnyxConnectionManager.ts b/lib/OnyxConnectionManager.ts index 50428710..7cf8d225 100644 --- a/lib/OnyxConnectionManager.ts +++ b/lib/OnyxConnectionManager.ts @@ -54,34 +54,6 @@ class OnyxConnectionManager { }); } - /** - * Subscribes a react component's state directly to a store key - * - * @example - * const connection = Onyx.connect({ - * key: ONYXKEYS.SESSION, - * callback: onSessionChange, - * }); - * - * @param mapping the mapping information to connect Onyx to the components state - * @param mapping.key ONYXKEY to subscribe to - * @param [mapping.statePropertyName] the name of the property in the state to connect the data to - * @param [mapping.withOnyxInstance] whose setState() method will be called with any changed data - * This is used by React components to connect to Onyx - * @param [mapping.callback] a method that will be called with changed data - * This is used by any non-React code to connect to Onyx - * @param [mapping.initWithStoredValues] If set to false, then no data will be prefilled into the - * component - * @param [mapping.waitForCollectionCallback] If set to true, it will return the entire collection to the callback as a single object - * @param [mapping.selector] THIS PARAM IS ONLY USED WITH withOnyx(). If included, this will be used to subscribe to a subset of an Onyx key's data. - * The sourceData and withOnyx state are passed to the selector and should return the simplified data. Using this setting on `withOnyx` can have very positive - * performance benefits because the component will only re-render when the subset of data changes. Otherwise, any change of data on any property would normally - * cause the component to re-render (and that can be expensive from a performance standpoint). - * @param [mapping.initialValue] THIS PARAM IS ONLY USED WITH withOnyx(). - * If included, this will be passed to the component so that something can be rendered while data is being fetched from the DB. - * Note that it will not cause the component to have the loading prop set to true. - * @returns a connection metadata object to use when calling `Onyx.disconnect()` - */ connect(connectOptions: ConnectOptions): ConnectionMetadata { const mapKey = this.connectionMapKey(connectOptions); let connection = this.connectionsMap.get(mapKey); From 5819394478c2d6007779f56676d778cf0b0e9104 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Fri, 5 Jul 2024 18:54:09 +0100 Subject: [PATCH 11/31] Add tests for useOnyx multiple usage --- tests/unit/useOnyxTest.ts | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/tests/unit/useOnyxTest.ts b/tests/unit/useOnyxTest.ts index 41608b94..1a27b579 100644 --- a/tests/unit/useOnyxTest.ts +++ b/tests/unit/useOnyxTest.ts @@ -485,11 +485,10 @@ describe('useOnyx', () => { }); describe('multiple usage', () => { - it('should 1', async () => { + it('should connect to a key and load the value into cache, and return the value loaded in the next hook call', async () => { await StorageMock.setItem(ONYXKEYS.TEST_KEY, 'test'); const {result: result1} = renderHook(() => useOnyx(ONYXKEYS.TEST_KEY)); - // const {result: result2} = renderHook(() => useOnyx(ONYXKEYS.TEST_KEY)); expect(result1.current[0]).toBeUndefined(); expect(result1.current[1].status).toEqual('loading'); @@ -504,5 +503,26 @@ describe('useOnyx', () => { expect(result2.current[0]).toEqual('test'); expect(result2.current[1].status).toEqual('loaded'); }); + + it('should connect to a key two times while data is loading from the cache, and return the value loaded to both of them', async () => { + await StorageMock.setItem(ONYXKEYS.TEST_KEY, 'test'); + + const {result: result1} = renderHook(() => useOnyx(ONYXKEYS.TEST_KEY)); + const {result: result2} = renderHook(() => useOnyx(ONYXKEYS.TEST_KEY)); + + expect(result1.current[0]).toBeUndefined(); + expect(result1.current[1].status).toEqual('loading'); + + expect(result2.current[0]).toBeUndefined(); + expect(result2.current[1].status).toEqual('loading'); + + await act(async () => waitForPromisesToResolve()); + + expect(result1.current[0]).toEqual('test'); + expect(result1.current[1].status).toEqual('loaded'); + + expect(result2.current[0]).toEqual('test'); + expect(result2.current[1].status).toEqual('loaded'); + }); }); }); From d9ac00b09e840b5e8b88647fcb3e846c8005ce21 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Sun, 7 Jul 2024 19:51:32 +0100 Subject: [PATCH 12/31] Fix evictable keys implementation --- lib/Onyx.ts | 4 +- lib/OnyxConnectionManager.ts | 81 +++++++++++++++++++------ lib/OnyxUtils.ts | 46 ++++---------- lib/useOnyx.ts | 40 ++++++------ lib/withOnyx/index.tsx | 8 +-- tests/unit/OnyxConnectionManagerTest.ts | 65 +++++++++++++++----- tests/unit/useOnyxTest.ts | 78 ++++++++++++++++++++++++ 7 files changed, 232 insertions(+), 90 deletions(-) diff --git a/lib/Onyx.ts b/lib/Onyx.ts index 962164fd..6ad9c348 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -101,8 +101,8 @@ function connect(connectOptions: ConnectOptions): Co * * @param connection connection metadata object returned by call to `Onyx.connect()` */ -function disconnect(connectionMetadada: ConnectionMetadata, shouldRemoveKeyFromEvictionBlocklist?: boolean): void { - connectionManager.disconnect(connectionMetadada, shouldRemoveKeyFromEvictionBlocklist); +function disconnect(connectionMetadada: ConnectionMetadata): void { + connectionManager.disconnect(connectionMetadada); } /** diff --git a/lib/OnyxConnectionManager.ts b/lib/OnyxConnectionManager.ts index 7cf8d225..3f8489ae 100644 --- a/lib/OnyxConnectionManager.ts +++ b/lib/OnyxConnectionManager.ts @@ -8,7 +8,7 @@ import type {DefaultConnectCallback, DefaultConnectOptions, OnyxKey, OnyxValue} type ConnectCallback = DefaultConnectCallback; type Connection = { - id: number; + subscriptionID: number; onyxKey: OnyxKey; isConnectionMade: boolean; callbacks: Map; @@ -17,9 +17,9 @@ type Connection = { }; type ConnectionMetadata = { - key: string; + id: string; callbackID: string; - connectionID: number; + subscriptionID: number; }; class OnyxConnectionManager { @@ -31,7 +31,7 @@ class OnyxConnectionManager { this.connectionsMap = new Map(); this.lastCallbackID = 0; - bindAll(this, 'fireCallbacks', 'connectionMapKey', 'connect', 'disconnect', 'disconnectKey', 'disconnectAll'); + bindAll(this, 'fireCallbacks', 'connectionMapKey', 'connect', 'disconnect', 'disconnectKey', 'disconnectAll', 'addToEvictionBlockList', 'removeFromEvictionBlockList'); } private connectionMapKey(connectOptions: ConnectOptions): string { @@ -57,7 +57,7 @@ class OnyxConnectionManager { connect(connectOptions: ConnectOptions): ConnectionMetadata { const mapKey = this.connectionMapKey(connectOptions); let connection = this.connectionsMap.get(mapKey); - let connectionID: number | undefined; + let subscriptionID: number | undefined; const callbackID = String(this.lastCallbackID++); @@ -77,13 +77,13 @@ class OnyxConnectionManager { }; } - connectionID = OnyxUtils.connectToKey({ + subscriptionID = OnyxUtils.connectToKey({ ...(connectOptions as DefaultConnectOptions), callback, }); connection = { - id: connectionID, + subscriptionID, onyxKey: connectOptions.key, isConnectionMade: false, callbacks: new Map(), @@ -102,7 +102,7 @@ class OnyxConnectionManager { }); } - return {key: mapKey, callbackID, connectionID: connection.id}; + return {id: mapKey, callbackID, subscriptionID: connection.subscriptionID}; } /** @@ -112,14 +112,13 @@ class OnyxConnectionManager { * * @param connection connection metadata object returned by call to `Onyx.connect()` */ - disconnect(connectionMetadada: ConnectionMetadata, shouldRemoveKeyFromEvictionBlocklist?: boolean): void { + disconnect(connectionMetadada: ConnectionMetadata): void { if (!connectionMetadada) { Logger.logAlert(`[ConnectionManager] Attempted to disconnect passing an undefined metadata object.`); return; } - const connection = this.connectionsMap.get(connectionMetadada.key); - + const connection = this.connectionsMap.get(connectionMetadada.id); if (!connection) { return; } @@ -127,29 +126,75 @@ class OnyxConnectionManager { connection.callbacks.delete(connectionMetadada.callbackID); if (connection.callbacks.size === 0) { - OnyxUtils.disconnectFromKey(connection.id, shouldRemoveKeyFromEvictionBlocklist ? connection.onyxKey : undefined); - this.connectionsMap.delete(connectionMetadada.key); + OnyxUtils.disconnectFromKey(connection.subscriptionID); + this.removeFromEvictionBlockList(connectionMetadada); + + this.connectionsMap.delete(connectionMetadada.id); } } - disconnectKey(key: string, shouldRemoveKeyFromEvictionBlocklist?: boolean): void { + disconnectKey(key: string): void { const connection = this.connectionsMap.get(key); if (!connection) { return; } - OnyxUtils.disconnectFromKey(connection.id, shouldRemoveKeyFromEvictionBlocklist ? connection.onyxKey : undefined); + OnyxUtils.disconnectFromKey(connection.subscriptionID); + Array.from(connection.callbacks.keys()).forEach((callbackID) => { + this.removeFromEvictionBlockList({id: key, callbackID, subscriptionID: connection.subscriptionID}); + }); + this.connectionsMap.delete(key); } - disconnectAll(shouldRemoveKeysFromEvictionBlocklist?: boolean): void { - Array.from(this.connectionsMap.values()).forEach((connection) => { - OnyxUtils.disconnectFromKey(connection.id, shouldRemoveKeysFromEvictionBlocklist ? connection.onyxKey : undefined); + disconnectAll(): void { + Array.from(this.connectionsMap.entries()).forEach(([connectionID, connection]) => { + OnyxUtils.disconnectFromKey(connection.subscriptionID); + Array.from(connection.callbacks.keys()).forEach((callbackID) => { + this.removeFromEvictionBlockList({id: connectionID, callbackID, subscriptionID: connection.subscriptionID}); + }); }); this.connectionsMap.clear(); } + + /** Keys added to this list can never be deleted. */ + addToEvictionBlockList(connectionMetadada: ConnectionMetadata): void { + const connection = this.connectionsMap.get(connectionMetadada.id); + if (!connection) { + return; + } + + this.removeFromEvictionBlockList(connectionMetadada); + + const evictionBlocklist = OnyxUtils.getEvictionBlocklist(); + if (!evictionBlocklist[connection.onyxKey]) { + evictionBlocklist[connection.onyxKey] = []; + } + + evictionBlocklist[connection.onyxKey]?.push(`${connectionMetadada.id}_${connectionMetadada.callbackID}`); + } + + /** + * Removes a key previously added to this list + * which will enable it to be deleted again. + */ + removeFromEvictionBlockList(connectionMetadada: ConnectionMetadata): void { + const connection = this.connectionsMap.get(connectionMetadada.id); + if (!connection) { + return; + } + + const evictionBlocklist = OnyxUtils.getEvictionBlocklist(); + evictionBlocklist[connection.onyxKey] = + evictionBlocklist[connection.onyxKey]?.filter((evictionKey) => evictionKey !== `${connectionMetadada.id}_${connectionMetadada.callbackID}`) ?? []; + + // Remove the key if there are no more subscribers. + if (evictionBlocklist[connection.onyxKey]?.length === 0) { + delete evictionBlocklist[connection.onyxKey]; + } + } } const connectionManager = new OnyxConnectionManager(); diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index a07af446..03ddce7a 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -60,9 +60,9 @@ let recentlyAccessedKeys: OnyxKey[] = []; // whatever appears in this list it will NEVER be a candidate for eviction. let evictionAllowList: OnyxKey[] = []; -// Holds a map of keys and connectionID arrays whose keys will never be automatically evicted as +// Holds a map of keys and connection arrays whose keys will never be automatically evicted as // long as we have at least one subscriber that returns false for the canEvict property. -const evictionBlocklist: Record = {}; +const evictionBlocklist: Record = {}; // Optional user-provided key value states set when Onyx initializes or clears let defaultKeyStates: Record> = {}; @@ -117,6 +117,13 @@ function getDeferredInitTask(): DeferredTask { return deferredInitTask; } +/** + * Getter - returns the eviction block list. + */ +function getEvictionBlocklist(): Record { + return evictionBlocklist; +} + /** * Sets the initial values for the Onyx store * @@ -460,30 +467,6 @@ function addLastAccessedKey(key: OnyxKey): void { recentlyAccessedKeys.push(key); } -/** - * Removes a key previously added to this list - * which will enable it to be deleted again. - */ -function removeFromEvictionBlockList(key: OnyxKey, connectionID: number): void { - evictionBlocklist[key] = evictionBlocklist[key]?.filter((evictionKey) => evictionKey !== connectionID) ?? []; - - // Remove the key if there are no more subscribers - if (evictionBlocklist[key]?.length === 0) { - delete evictionBlocklist[key]; - } -} - -/** Keys added to this list can never be deleted. */ -function addToEvictionBlockList(key: OnyxKey, connectionID: number): void { - removeFromEvictionBlockList(key, connectionID); - - if (!evictionBlocklist[key]) { - evictionBlocklist[key] = []; - } - - evictionBlocklist[key].push(connectionID); -} - /** * Take all the keys that are safe to evict and add them to * the recently accessed list when initializing the app. This @@ -1261,17 +1244,11 @@ function connectToKey(connectOptions: ConnectOptions * * @param connectionID unique id returned by call to `OnyxUtils.connectToKey()` */ -function disconnectFromKey(connectionID: number, keyToRemoveFromEvictionBlocklist?: OnyxKey): void { +function disconnectFromKey(connectionID: number): void { if (!callbackToStateMapping[connectionID]) { return; } - // Remove this key from the eviction block list as we are no longer - // subscribing to it and it should be safe to delete again - if (keyToRemoveFromEvictionBlocklist) { - OnyxUtils.removeFromEvictionBlockList(keyToRemoveFromEvictionBlocklist, connectionID); - } - delete callbackToStateMapping[connectionID]; } @@ -1296,8 +1273,6 @@ const OnyxUtils = { tryGetCachedValue, removeLastAccessedKey, addLastAccessedKey, - removeFromEvictionBlockList, - addToEvictionBlockList, addAllSafeEvictionKeysToRecentlyAccessedList, getCachedCollection, keysChanged, @@ -1320,6 +1295,7 @@ const OnyxUtils = { multiGet, connectToKey, disconnectFromKey, + getEvictionBlocklist, }; export default OnyxUtils; diff --git a/lib/useOnyx.ts b/lib/useOnyx.ts index fa5521bc..f58bbabb 100644 --- a/lib/useOnyx.ts +++ b/lib/useOnyx.ts @@ -124,6 +124,23 @@ function useOnyx>(key: TKey ); }, [previousKey, key]); + // Mimics withOnyx's checkEvictableKeys() behavior. + const checkEvictableKey = useCallback(() => { + if (options?.canEvict === undefined || !connectionRef.current) { + return; + } + + if (!OnyxUtils.isSafeEvictionKey(key)) { + throw new Error(`canEvict can't be used on key '${key}'. This key must explicitly be flagged as safe for removal by adding it to Onyx.init({safeEvictionKeys: []}).`); + } + + if (options.canEvict) { + connectionManager.removeFromEvictionBlockList(connectionRef.current); + } else { + connectionManager.addToEvictionBlockList(connectionRef.current); + } + }, [key, options?.canEvict]); + const getSnapshot = useCallback(() => { // We get the value from cache while the first connection to Onyx is being made, // so we can return any cached value right away. After the connection is made, we only @@ -204,34 +221,23 @@ function useOnyx>(key: TKey waitForCollectionCallback: OnyxUtils.isCollectionKey(key) as true, }); + checkEvictableKey(); + return () => { if (!connectionRef.current) { return; } - connectionManager.disconnect(connectionRef.current, true); + connectionManager.disconnect(connectionRef.current); isFirstConnectionRef.current = false; }; }, - [key, options?.initWithStoredValues], + [key, options?.initWithStoredValues, checkEvictableKey], ); - // Mimics withOnyx's checkEvictableKeys() behavior. useEffect(() => { - if (options?.canEvict === undefined || !connectionRef.current) { - return; - } - - if (!OnyxUtils.isSafeEvictionKey(key)) { - throw new Error(`canEvict can't be used on key '${key}'. This key must explicitly be flagged as safe for removal by adding it to Onyx.init({safeEvictionKeys: []}).`); - } - - if (options.canEvict) { - OnyxUtils.removeFromEvictionBlockList(key, connectionRef.current.connectionID); - } else { - OnyxUtils.addToEvictionBlockList(key, connectionRef.current.connectionID); - } - }, [key, options?.canEvict]); + checkEvictableKey(); + }, [checkEvictableKey]); const result = useSyncExternalStore>(subscribe, getSnapshot); diff --git a/lib/withOnyx/index.tsx b/lib/withOnyx/index.tsx index 8072b754..0564ff89 100644 --- a/lib/withOnyx/index.tsx +++ b/lib/withOnyx/index.tsx @@ -164,7 +164,7 @@ export default function ( const previousKey = isFirstTimeUpdatingAfterLoading ? mapping.previousKey : Str.result(mapping.key as GenericFunction, {...prevProps, ...prevOnyxDataFromState}); const newKey = Str.result(mapping.key as GenericFunction, {...this.props, ...onyxDataFromState}); if (previousKey !== newKey) { - connectionManager.disconnect(this.activeConnections[previousKey], true); + connectionManager.disconnect(this.activeConnections[previousKey]); delete this.activeConnections[previousKey]; this.connectMappingToOnyx(mapping, propName, newKey); } @@ -177,7 +177,7 @@ export default function ( // Disconnect everything from Onyx mapOnyxToStateEntries(mapOnyxToState).forEach(([, mapping]) => { const key = Str.result(mapping.key as GenericFunction, {...this.props, ...getOnyxDataFromState(this.state, mapOnyxToState)}); - connectionManager.disconnect(this.activeConnections[key], true); + connectionManager.disconnect(this.activeConnections[key]); }); } @@ -294,9 +294,9 @@ export default function ( } if (canEvict) { - OnyxUtils.removeFromEvictionBlockList(key, mapping.connectionID); + connectionManager.removeFromEvictionBlockList(this.activeConnections[key]); } else { - OnyxUtils.addToEvictionBlockList(key, mapping.connectionID); + connectionManager.addToEvictionBlockList(this.activeConnections[key]); } }); } diff --git a/tests/unit/OnyxConnectionManagerTest.ts b/tests/unit/OnyxConnectionManagerTest.ts index 64c103b7..8eed2367 100644 --- a/tests/unit/OnyxConnectionManagerTest.ts +++ b/tests/unit/OnyxConnectionManagerTest.ts @@ -7,6 +7,7 @@ import type {OnyxKey, WithOnyxConnectOptions} from '../../lib/types'; import type {WithOnyxInstance} from '../../lib/withOnyx/types'; import type GenericCollection from '../utils/GenericCollection'; import waitForPromisesToResolve from '../utils/waitForPromisesToResolve'; +import OnyxUtils from '../../lib/OnyxUtils'; // eslint-disable-next-line dot-notation const connectionsMap = connectionManager['connectionsMap']; @@ -32,14 +33,14 @@ describe('OnyxConnectionManager', () => { connectionManager.disconnectAll(); }); - describe('connect', () => { + describe('connect / disconnect', () => { it('should connect to a key and fire the callback with its value', async () => { await StorageMock.setItem(ONYXKEYS.TEST_KEY, 'test'); const callback1 = jest.fn(); const connection = connectionManager.connect({key: ONYXKEYS.TEST_KEY, callback: callback1}); - expect(connectionsMap.has(connection.key)).toBeTruthy(); + expect(connectionsMap.has(connection.id)).toBeTruthy(); await act(async () => waitForPromisesToResolve()); @@ -60,9 +61,9 @@ describe('OnyxConnectionManager', () => { const callback2 = jest.fn(); const connection2 = connectionManager.connect({key: ONYXKEYS.TEST_KEY, callback: callback2}); - expect(connection1.key).toEqual(connection2.key); + expect(connection1.id).toEqual(connection2.id); expect(connectionsMap.size).toEqual(1); - expect(connectionsMap.has(connection1.key)).toBeTruthy(); + expect(connectionsMap.has(connection1.id)).toBeTruthy(); await act(async () => waitForPromisesToResolve()); @@ -95,10 +96,10 @@ describe('OnyxConnectionManager', () => { const callback2 = jest.fn(); const connection2 = connectionManager.connect({key: ONYXKEYS.COLLECTION.TEST_KEY, callback: callback2, waitForCollectionCallback: true}); - expect(connection1.key).not.toEqual(connection2.key); + expect(connection1.id).not.toEqual(connection2.id); expect(connectionsMap.size).toEqual(2); - expect(connectionsMap.has(connection1.key)).toBeTruthy(); - expect(connectionsMap.has(connection2.key)).toBeTruthy(); + expect(connectionsMap.has(connection1.id)).toBeTruthy(); + expect(connectionsMap.has(connection2.id)).toBeTruthy(); await act(async () => waitForPromisesToResolve()); @@ -205,10 +206,10 @@ describe('OnyxConnectionManager', () => { await act(async () => waitForPromisesToResolve()); - expect(connection1.key).not.toEqual(connection2.key); + expect(connection1.id).not.toEqual(connection2.id); expect(connectionsMap.size).toEqual(2); - expect(connectionsMap.has(connection1.key)).toBeTruthy(); - expect(connectionsMap.has(connection2.key)).toBeTruthy(); + expect(connectionsMap.has(connection1.id)).toBeTruthy(); + expect(connectionsMap.has(connection2.id)).toBeTruthy(); connectionManager.disconnect(connection1); connectionManager.disconnect(connection2); @@ -216,7 +217,7 @@ describe('OnyxConnectionManager', () => { expect(connectionsMap.size).toEqual(0); }); - it('should connect to a key directly, connect again with withOnyx and create another connection instead of reusing the first one', async () => { + it('should connect to a key directly, connect again with withOnyx but create another connection instead of reusing the first one', async () => { await StorageMock.setItem(ONYXKEYS.TEST_KEY, 'test'); const callback1 = jest.fn(); @@ -237,10 +238,10 @@ describe('OnyxConnectionManager', () => { })() as unknown as WithOnyxInstance, } as WithOnyxConnectOptions); - expect(connection1.key).not.toEqual(connection2.key); + expect(connection1.id).not.toEqual(connection2.id); expect(connectionsMap.size).toEqual(2); - expect(connectionsMap.has(connection1.key)).toBeTruthy(); - expect(connectionsMap.has(connection2.key)).toBeTruthy(); + expect(connectionsMap.has(connection1.id)).toBeTruthy(); + expect(connectionsMap.has(connection2.id)).toBeTruthy(); connectionManager.disconnect(connection1); connectionManager.disconnect(connection2); @@ -248,4 +249,40 @@ describe('OnyxConnectionManager', () => { expect(connectionsMap.size).toEqual(0); }); }); + + describe('addToEvictionBlockList / removeFromEvictionBlockList', () => { + it('should add and remove connections from the eviction block list correctly', async () => { + const evictionBlocklist = OnyxUtils.getEvictionBlocklist(); + + connectionsMap.set('connectionID1', {subscriptionID: 0, onyxKey: ONYXKEYS.TEST_KEY, callbacks: new Map(), isConnectionMade: true}); + connectionsMap.get('connectionID1')?.callbacks.set('callbackID1', () => undefined); + connectionManager.addToEvictionBlockList({id: 'connectionID1', callbackID: 'callbackID1', subscriptionID: 0}); + expect(evictionBlocklist[ONYXKEYS.TEST_KEY]).toEqual(['connectionID1_callbackID1']); + + connectionsMap.get('connectionID1')?.callbacks.set('callbackID2', () => undefined); + connectionManager.addToEvictionBlockList({id: 'connectionID1', callbackID: 'callbackID2', subscriptionID: 0}); + expect(evictionBlocklist[ONYXKEYS.TEST_KEY]).toEqual(['connectionID1_callbackID1', 'connectionID1_callbackID2']); + + connectionsMap.set('connectionID2', {subscriptionID: 1, onyxKey: `${ONYXKEYS.COLLECTION.TEST_KEY}entry1`, callbacks: new Map(), isConnectionMade: true}); + connectionsMap.get('connectionID2')?.callbacks.set('callbackID3', () => undefined); + connectionManager.addToEvictionBlockList({id: 'connectionID2', callbackID: 'callbackID3', subscriptionID: 1}); + expect(evictionBlocklist[`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`]).toEqual(['connectionID2_callbackID3']); + + connectionManager.removeFromEvictionBlockList({id: 'connectionID2', callbackID: 'callbackID3', subscriptionID: 1}); + expect(evictionBlocklist[`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`]).toBeUndefined(); + + // inexistent callback ID, shouldn't do anything + connectionManager.removeFromEvictionBlockList({id: 'connectionID1', callbackID: 'callbackID1000', subscriptionID: 0}); + expect(evictionBlocklist[ONYXKEYS.TEST_KEY]).toEqual(['connectionID1_callbackID1', 'connectionID1_callbackID2']); + + connectionManager.removeFromEvictionBlockList({id: 'connectionID1', callbackID: 'callbackID2', subscriptionID: 0}); + expect(evictionBlocklist[ONYXKEYS.TEST_KEY]).toEqual(['connectionID1_callbackID1']); + + connectionManager.removeFromEvictionBlockList({id: 'connectionID1', callbackID: 'callbackID1', subscriptionID: 0}); + expect(evictionBlocklist[ONYXKEYS.TEST_KEY]).toBeUndefined(); + + // inexistent connection ID, shouldn't do anything + expect(() => connectionManager.removeFromEvictionBlockList({id: 'connectionID0', callbackID: 'callbackID0', subscriptionID: 0})).not.toThrow(); + }); + }); }); diff --git a/tests/unit/useOnyxTest.ts b/tests/unit/useOnyxTest.ts index 1a27b579..0b054a67 100644 --- a/tests/unit/useOnyxTest.ts +++ b/tests/unit/useOnyxTest.ts @@ -3,6 +3,7 @@ import type {OnyxEntry} from '../../lib'; import Onyx, {useOnyx} from '../../lib'; import waitForPromisesToResolve from '../utils/waitForPromisesToResolve'; import StorageMock from '../../lib/storage'; +import OnyxUtils from '../../lib/OnyxUtils'; const ONYXKEYS = { TEST_KEY: 'test', @@ -11,10 +12,13 @@ const ONYXKEYS = { TEST_KEY: 'test_', TEST_KEY_2: 'test2_', }, + EVICTABLE_TEST_KEY: 'evictable_test', + EVICTABLE_TEST_KEY2: 'evictable_test2', }; Onyx.init({ keys: ONYXKEYS, + safeEvictionKeys: [ONYXKEYS.EVICTABLE_TEST_KEY, ONYXKEYS.EVICTABLE_TEST_KEY2], }); beforeEach(() => Onyx.clear()); @@ -525,4 +529,78 @@ describe('useOnyx', () => { expect(result2.current[1].status).toEqual('loaded'); }); }); + + // This suite test must be the last one to avoid problems when running the other tests here. + describe('canEvict', () => { + const error = (key: string) => `canEvict can't be used on key '${key}'. This key must explicitly be flagged as safe for removal by adding it to Onyx.init({safeEvictionKeys: []}).`; + + beforeEach(() => { + jest.spyOn(console, 'error').mockImplementation(jest.fn); + }); + + afterEach(() => { + (console.error as unknown as jest.SpyInstance>).mockRestore(); + }); + + it('should throw an error when trying to set the "canEvict" property for a non-evictable key', async () => { + await StorageMock.setItem(ONYXKEYS.TEST_KEY, 'test'); + + try { + renderHook(() => useOnyx(ONYXKEYS.TEST_KEY, {canEvict: false})); + + await act(async () => waitForPromisesToResolve()); + + fail('Expected to throw an error.'); + } catch (e) { + expect((e as Error).message).toBe(error(ONYXKEYS.TEST_KEY)); + } + }); + + it('should add the connection to the blocklist when setting "canEvict" to false', async () => { + await StorageMock.setItem(ONYXKEYS.EVICTABLE_TEST_KEY, 'test'); + + renderHook(() => useOnyx(ONYXKEYS.EVICTABLE_TEST_KEY, {canEvict: false})); + + await act(async () => waitForPromisesToResolve()); + + const evictionBlocklist = OnyxUtils.getEvictionBlocklist(); + expect(evictionBlocklist[ONYXKEYS.EVICTABLE_TEST_KEY]).toHaveLength(1); + }); + + it('should handle removal/adding the connection to the blocklist properly when changing the evictable key to another', async () => { + await StorageMock.setItem(ONYXKEYS.EVICTABLE_TEST_KEY, 'test'); + + const {rerender} = renderHook((key: string) => useOnyx(key, {canEvict: false}), {initialProps: ONYXKEYS.EVICTABLE_TEST_KEY as string}); + + await act(async () => waitForPromisesToResolve()); + + const evictionBlocklist = OnyxUtils.getEvictionBlocklist(); + expect(evictionBlocklist[ONYXKEYS.EVICTABLE_TEST_KEY]).toHaveLength(1); + expect(evictionBlocklist[ONYXKEYS.EVICTABLE_TEST_KEY2]).toBeUndefined(); + + await act(async () => { + rerender(ONYXKEYS.EVICTABLE_TEST_KEY2); + }); + + expect(evictionBlocklist[ONYXKEYS.EVICTABLE_TEST_KEY]).toBeUndefined(); + expect(evictionBlocklist[ONYXKEYS.EVICTABLE_TEST_KEY2]).toHaveLength(1); + }); + + it('should remove the connection from the blocklist when setting "canEvict" to true', async () => { + await StorageMock.setItem(ONYXKEYS.EVICTABLE_TEST_KEY, 'test'); + + const {rerender} = renderHook((canEvict: boolean) => useOnyx(ONYXKEYS.EVICTABLE_TEST_KEY, {canEvict}), {initialProps: false as boolean}); + + await act(async () => waitForPromisesToResolve()); + + const evictionBlocklist = OnyxUtils.getEvictionBlocklist(); + expect(evictionBlocklist[ONYXKEYS.EVICTABLE_TEST_KEY]).toHaveLength(1); + + await act(async () => { + rerender(true); + }); + + expect(evictionBlocklist[ONYXKEYS.EVICTABLE_TEST_KEY]).toBeUndefined(); + }); + }); }); From 213e8f51272b6d13333b17fa86c0870c0da707ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Mon, 15 Jul 2024 17:18:23 +0100 Subject: [PATCH 13/31] Implement test for reuseConnection use case --- tests/unit/OnyxConnectionManagerTest.ts | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/unit/OnyxConnectionManagerTest.ts b/tests/unit/OnyxConnectionManagerTest.ts index 8eed2367..270baf5a 100644 --- a/tests/unit/OnyxConnectionManagerTest.ts +++ b/tests/unit/OnyxConnectionManagerTest.ts @@ -172,6 +172,21 @@ describe('OnyxConnectionManager', () => { expect(callback2).toHaveBeenCalledWith('test', ONYXKEYS.TEST_KEY); expect(connectionsMap.size).toEqual(1); }); + + it('should create a separate connection to the same key when setting reuseConnection to false', async () => { + await StorageMock.setItem(ONYXKEYS.TEST_KEY, 'test'); + + const callback1 = jest.fn(); + const connection1 = connectionManager.connect({key: ONYXKEYS.TEST_KEY, callback: callback1}); + + const callback2 = jest.fn(); + const connection2 = connectionManager.connect({key: ONYXKEYS.TEST_KEY, reuseConnection: false, callback: callback2}); + + expect(connection1.id).not.toEqual(connection2.id); + expect(connectionsMap.size).toEqual(2); + expect(connectionsMap.has(connection1.id)).toBeTruthy(); + expect(connectionsMap.has(connection2.id)).toBeTruthy(); + }); }); describe('withOnyx', () => { From 0b1ec569582c88a14e0fa2efa38e0124bdee5b05 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Mon, 15 Jul 2024 17:30:40 +0100 Subject: [PATCH 14/31] Remove disconnectKey() and implement test for disconnectAll() --- lib/OnyxConnectionManager.ts | 17 +-- tests/unit/OnyxConnectionManagerTest.ts | 131 ++++++++++++++---------- 2 files changed, 80 insertions(+), 68 deletions(-) diff --git a/lib/OnyxConnectionManager.ts b/lib/OnyxConnectionManager.ts index 3f8489ae..97943850 100644 --- a/lib/OnyxConnectionManager.ts +++ b/lib/OnyxConnectionManager.ts @@ -31,7 +31,7 @@ class OnyxConnectionManager { this.connectionsMap = new Map(); this.lastCallbackID = 0; - bindAll(this, 'fireCallbacks', 'connectionMapKey', 'connect', 'disconnect', 'disconnectKey', 'disconnectAll', 'addToEvictionBlockList', 'removeFromEvictionBlockList'); + bindAll(this, 'fireCallbacks', 'connectionMapKey', 'connect', 'disconnect', 'disconnectAll', 'addToEvictionBlockList', 'removeFromEvictionBlockList'); } private connectionMapKey(connectOptions: ConnectOptions): string { @@ -133,21 +133,6 @@ class OnyxConnectionManager { } } - disconnectKey(key: string): void { - const connection = this.connectionsMap.get(key); - - if (!connection) { - return; - } - - OnyxUtils.disconnectFromKey(connection.subscriptionID); - Array.from(connection.callbacks.keys()).forEach((callbackID) => { - this.removeFromEvictionBlockList({id: key, callbackID, subscriptionID: connection.subscriptionID}); - }); - - this.connectionsMap.delete(key); - } - disconnectAll(): void { Array.from(this.connectionsMap.entries()).forEach(([connectionID, connection]) => { OnyxUtils.disconnectFromKey(connection.subscriptionID); diff --git a/tests/unit/OnyxConnectionManagerTest.ts b/tests/unit/OnyxConnectionManagerTest.ts index 270baf5a..d80bf80f 100644 --- a/tests/unit/OnyxConnectionManagerTest.ts +++ b/tests/unit/OnyxConnectionManagerTest.ts @@ -187,79 +187,106 @@ describe('OnyxConnectionManager', () => { expect(connectionsMap.has(connection1.id)).toBeTruthy(); expect(connectionsMap.has(connection2.id)).toBeTruthy(); }); - }); - describe('withOnyx', () => { - it('should connect to a key two times with withOnyx and create two connections instead of one', async () => { - await StorageMock.setItem(ONYXKEYS.TEST_KEY, 'test'); + describe('withOnyx', () => { + it('should connect to a key two times with withOnyx and create two connections instead of one', async () => { + await StorageMock.setItem(ONYXKEYS.TEST_KEY, 'test'); + + const connection1 = connectionManager.connect({ + key: ONYXKEYS.TEST_KEY, + displayName: 'Component1', + statePropertyName: 'prop1', + withOnyxInstance: new (class { + // eslint-disable-next-line @typescript-eslint/no-empty-function + setStateProxy() {} + + // eslint-disable-next-line @typescript-eslint/no-empty-function + setWithOnyxState() {} + })() as unknown as WithOnyxInstance, + } as WithOnyxConnectOptions); + + const connection2 = connectionManager.connect({ + key: ONYXKEYS.TEST_KEY, + displayName: 'Component2', + statePropertyName: 'prop2', + withOnyxInstance: new (class { + // eslint-disable-next-line @typescript-eslint/no-empty-function + setStateProxy() {} + + // eslint-disable-next-line @typescript-eslint/no-empty-function + setWithOnyxState() {} + })() as unknown as WithOnyxInstance, + } as WithOnyxConnectOptions); + + await act(async () => waitForPromisesToResolve()); + + expect(connection1.id).not.toEqual(connection2.id); + expect(connectionsMap.size).toEqual(2); + expect(connectionsMap.has(connection1.id)).toBeTruthy(); + expect(connectionsMap.has(connection2.id)).toBeTruthy(); + + connectionManager.disconnect(connection1); + connectionManager.disconnect(connection2); + + expect(connectionsMap.size).toEqual(0); + }); - const connection1 = connectionManager.connect({ - key: ONYXKEYS.TEST_KEY, - displayName: 'Component1', - statePropertyName: 'prop1', - withOnyxInstance: new (class { - // eslint-disable-next-line @typescript-eslint/no-empty-function - setStateProxy() {} + it('should connect to a key directly, connect again with withOnyx but create another connection instead of reusing the first one', async () => { + await StorageMock.setItem(ONYXKEYS.TEST_KEY, 'test'); - // eslint-disable-next-line @typescript-eslint/no-empty-function - setWithOnyxState() {} - })() as unknown as WithOnyxInstance, - } as WithOnyxConnectOptions); + const callback1 = jest.fn(); + const connection1 = connectionManager.connect({key: ONYXKEYS.TEST_KEY, callback: callback1}); - const connection2 = connectionManager.connect({ - key: ONYXKEYS.TEST_KEY, - displayName: 'Component2', - statePropertyName: 'prop2', - withOnyxInstance: new (class { - // eslint-disable-next-line @typescript-eslint/no-empty-function - setStateProxy() {} + await act(async () => waitForPromisesToResolve()); - // eslint-disable-next-line @typescript-eslint/no-empty-function - setWithOnyxState() {} - })() as unknown as WithOnyxInstance, - } as WithOnyxConnectOptions); + const connection2 = connectionManager.connect({ + key: ONYXKEYS.TEST_KEY, + displayName: 'Component2', + statePropertyName: 'prop2', + withOnyxInstance: new (class { + // eslint-disable-next-line @typescript-eslint/no-empty-function + setStateProxy() {} - await act(async () => waitForPromisesToResolve()); + // eslint-disable-next-line @typescript-eslint/no-empty-function + setWithOnyxState() {} + })() as unknown as WithOnyxInstance, + } as WithOnyxConnectOptions); - expect(connection1.id).not.toEqual(connection2.id); - expect(connectionsMap.size).toEqual(2); - expect(connectionsMap.has(connection1.id)).toBeTruthy(); - expect(connectionsMap.has(connection2.id)).toBeTruthy(); + expect(connection1.id).not.toEqual(connection2.id); + expect(connectionsMap.size).toEqual(2); + expect(connectionsMap.has(connection1.id)).toBeTruthy(); + expect(connectionsMap.has(connection2.id)).toBeTruthy(); - connectionManager.disconnect(connection1); - connectionManager.disconnect(connection2); + connectionManager.disconnect(connection1); + connectionManager.disconnect(connection2); - expect(connectionsMap.size).toEqual(0); + expect(connectionsMap.size).toEqual(0); + }); }); + }); - it('should connect to a key directly, connect again with withOnyx but create another connection instead of reusing the first one', async () => { + describe('disconnectAll', () => { + it('should disconnect all connections', async () => { await StorageMock.setItem(ONYXKEYS.TEST_KEY, 'test'); + await StorageMock.setItem(ONYXKEYS.TEST_KEY_2, 'test2'); const callback1 = jest.fn(); const connection1 = connectionManager.connect({key: ONYXKEYS.TEST_KEY, callback: callback1}); - await act(async () => waitForPromisesToResolve()); - - const connection2 = connectionManager.connect({ - key: ONYXKEYS.TEST_KEY, - displayName: 'Component2', - statePropertyName: 'prop2', - withOnyxInstance: new (class { - // eslint-disable-next-line @typescript-eslint/no-empty-function - setStateProxy() {} + const callback2 = jest.fn(); + const connection2 = connectionManager.connect({key: ONYXKEYS.TEST_KEY, callback: callback2}); - // eslint-disable-next-line @typescript-eslint/no-empty-function - setWithOnyxState() {} - })() as unknown as WithOnyxInstance, - } as WithOnyxConnectOptions); + const callback3 = jest.fn(); + const connection3 = connectionManager.connect({key: ONYXKEYS.TEST_KEY_2, callback: callback3}); - expect(connection1.id).not.toEqual(connection2.id); + expect(connection1.id).toEqual(connection2.id); expect(connectionsMap.size).toEqual(2); expect(connectionsMap.has(connection1.id)).toBeTruthy(); - expect(connectionsMap.has(connection2.id)).toBeTruthy(); + expect(connectionsMap.has(connection3.id)).toBeTruthy(); - connectionManager.disconnect(connection1); - connectionManager.disconnect(connection2); + await act(async () => waitForPromisesToResolve()); + + connectionManager.disconnectAll(); expect(connectionsMap.size).toEqual(0); }); From 1ff1db1d0b72233dc9cb6fc7920c665d6c2baaae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Mon, 15 Jul 2024 17:33:34 +0100 Subject: [PATCH 15/31] Improve disconnectAll() implementation --- lib/OnyxConnectionManager.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/OnyxConnectionManager.ts b/lib/OnyxConnectionManager.ts index 97943850..20fb3dcd 100644 --- a/lib/OnyxConnectionManager.ts +++ b/lib/OnyxConnectionManager.ts @@ -134,9 +134,9 @@ class OnyxConnectionManager { } disconnectAll(): void { - Array.from(this.connectionsMap.entries()).forEach(([connectionID, connection]) => { + this.connectionsMap.forEach((connection, connectionID) => { OnyxUtils.disconnectFromKey(connection.subscriptionID); - Array.from(connection.callbacks.keys()).forEach((callbackID) => { + connection.callbacks.forEach((_, callbackID) => { this.removeFromEvictionBlockList({id: connectionID, callbackID, subscriptionID: connection.subscriptionID}); }); }); From 3ed9856dcbf76c2a84bdc358fae16814281accb3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Mon, 15 Jul 2024 17:52:50 +0100 Subject: [PATCH 16/31] Make initWithStoredValues option create unique connections --- lib/OnyxConnectionManager.ts | 2 +- tests/unit/OnyxConnectionManagerTest.ts | 37 +++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/lib/OnyxConnectionManager.ts b/lib/OnyxConnectionManager.ts index 20fb3dcd..f88d4d57 100644 --- a/lib/OnyxConnectionManager.ts +++ b/lib/OnyxConnectionManager.ts @@ -37,7 +37,7 @@ class OnyxConnectionManager { private connectionMapKey(connectOptions: ConnectOptions): string { let suffix = ''; - if (connectOptions.reuseConnection === false || 'withOnyxInstance' in connectOptions) { + if (connectOptions.reuseConnection === false || connectOptions.initWithStoredValues === false || 'withOnyxInstance' in connectOptions) { suffix += `,uniqueID=${Str.guid()}`; } diff --git a/tests/unit/OnyxConnectionManagerTest.ts b/tests/unit/OnyxConnectionManagerTest.ts index d80bf80f..41366325 100644 --- a/tests/unit/OnyxConnectionManagerTest.ts +++ b/tests/unit/OnyxConnectionManagerTest.ts @@ -188,6 +188,43 @@ describe('OnyxConnectionManager', () => { expect(connectionsMap.has(connection2.id)).toBeTruthy(); }); + it('should create a separate connection to the same key when setting initWithStoredValues to false', async () => { + await StorageMock.setItem(ONYXKEYS.TEST_KEY, 'test'); + + const callback1 = jest.fn(); + const connection1 = connectionManager.connect({key: ONYXKEYS.TEST_KEY, initWithStoredValues: false, callback: callback1}); + + await act(async () => waitForPromisesToResolve()); + + expect(callback1).not.toHaveBeenCalled(); + expect(connectionsMap.size).toEqual(1); + expect(connectionsMap.has(connection1.id)).toBeTruthy(); + + await Onyx.set(ONYXKEYS.TEST_KEY, 'test2'); + + expect(callback1).toHaveBeenCalledTimes(1); + expect(callback1).toHaveBeenCalledWith('test2', ONYXKEYS.TEST_KEY); + + const callback2 = jest.fn(); + const connection2 = connectionManager.connect({key: ONYXKEYS.TEST_KEY, initWithStoredValues: false, callback: callback2}); + + await act(async () => waitForPromisesToResolve()); + + expect(callback2).not.toHaveBeenCalled(); + expect(connectionsMap.size).toEqual(2); + expect(connectionsMap.has(connection2.id)).toBeTruthy(); + + await Onyx.set(ONYXKEYS.TEST_KEY, 'test3'); + + expect(callback2).toHaveBeenCalledTimes(1); + expect(callback2).toHaveBeenCalledWith('test3', ONYXKEYS.TEST_KEY); + + connectionManager.disconnect(connection1); + connectionManager.disconnect(connection2); + + expect(connectionsMap.size).toEqual(0); + }); + describe('withOnyx', () => { it('should connect to a key two times with withOnyx and create two connections instead of one', async () => { await StorageMock.setItem(ONYXKEYS.TEST_KEY, 'test'); From 0b9614e0f2184b1e05d92f9b2c4e26ee2fb93665 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Mon, 15 Jul 2024 18:06:41 +0100 Subject: [PATCH 17/31] Remove subscriptionID property from connection metadata --- lib/OnyxConnectionManager.ts | 5 ++--- tests/unit/OnyxConnectionManagerTest.ts | 16 ++++++++-------- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/lib/OnyxConnectionManager.ts b/lib/OnyxConnectionManager.ts index f88d4d57..47e59204 100644 --- a/lib/OnyxConnectionManager.ts +++ b/lib/OnyxConnectionManager.ts @@ -19,7 +19,6 @@ type Connection = { type ConnectionMetadata = { id: string; callbackID: string; - subscriptionID: number; }; class OnyxConnectionManager { @@ -102,7 +101,7 @@ class OnyxConnectionManager { }); } - return {id: mapKey, callbackID, subscriptionID: connection.subscriptionID}; + return {id: mapKey, callbackID}; } /** @@ -137,7 +136,7 @@ class OnyxConnectionManager { this.connectionsMap.forEach((connection, connectionID) => { OnyxUtils.disconnectFromKey(connection.subscriptionID); connection.callbacks.forEach((_, callbackID) => { - this.removeFromEvictionBlockList({id: connectionID, callbackID, subscriptionID: connection.subscriptionID}); + this.removeFromEvictionBlockList({id: connectionID, callbackID}); }); }); diff --git a/tests/unit/OnyxConnectionManagerTest.ts b/tests/unit/OnyxConnectionManagerTest.ts index 41366325..aa8ee3be 100644 --- a/tests/unit/OnyxConnectionManagerTest.ts +++ b/tests/unit/OnyxConnectionManagerTest.ts @@ -335,33 +335,33 @@ describe('OnyxConnectionManager', () => { connectionsMap.set('connectionID1', {subscriptionID: 0, onyxKey: ONYXKEYS.TEST_KEY, callbacks: new Map(), isConnectionMade: true}); connectionsMap.get('connectionID1')?.callbacks.set('callbackID1', () => undefined); - connectionManager.addToEvictionBlockList({id: 'connectionID1', callbackID: 'callbackID1', subscriptionID: 0}); + connectionManager.addToEvictionBlockList({id: 'connectionID1', callbackID: 'callbackID1'}); expect(evictionBlocklist[ONYXKEYS.TEST_KEY]).toEqual(['connectionID1_callbackID1']); connectionsMap.get('connectionID1')?.callbacks.set('callbackID2', () => undefined); - connectionManager.addToEvictionBlockList({id: 'connectionID1', callbackID: 'callbackID2', subscriptionID: 0}); + connectionManager.addToEvictionBlockList({id: 'connectionID1', callbackID: 'callbackID2'}); expect(evictionBlocklist[ONYXKEYS.TEST_KEY]).toEqual(['connectionID1_callbackID1', 'connectionID1_callbackID2']); connectionsMap.set('connectionID2', {subscriptionID: 1, onyxKey: `${ONYXKEYS.COLLECTION.TEST_KEY}entry1`, callbacks: new Map(), isConnectionMade: true}); connectionsMap.get('connectionID2')?.callbacks.set('callbackID3', () => undefined); - connectionManager.addToEvictionBlockList({id: 'connectionID2', callbackID: 'callbackID3', subscriptionID: 1}); + connectionManager.addToEvictionBlockList({id: 'connectionID2', callbackID: 'callbackID3'}); expect(evictionBlocklist[`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`]).toEqual(['connectionID2_callbackID3']); - connectionManager.removeFromEvictionBlockList({id: 'connectionID2', callbackID: 'callbackID3', subscriptionID: 1}); + connectionManager.removeFromEvictionBlockList({id: 'connectionID2', callbackID: 'callbackID3'}); expect(evictionBlocklist[`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`]).toBeUndefined(); // inexistent callback ID, shouldn't do anything - connectionManager.removeFromEvictionBlockList({id: 'connectionID1', callbackID: 'callbackID1000', subscriptionID: 0}); + connectionManager.removeFromEvictionBlockList({id: 'connectionID1', callbackID: 'callbackID1000'}); expect(evictionBlocklist[ONYXKEYS.TEST_KEY]).toEqual(['connectionID1_callbackID1', 'connectionID1_callbackID2']); - connectionManager.removeFromEvictionBlockList({id: 'connectionID1', callbackID: 'callbackID2', subscriptionID: 0}); + connectionManager.removeFromEvictionBlockList({id: 'connectionID1', callbackID: 'callbackID2'}); expect(evictionBlocklist[ONYXKEYS.TEST_KEY]).toEqual(['connectionID1_callbackID1']); - connectionManager.removeFromEvictionBlockList({id: 'connectionID1', callbackID: 'callbackID1', subscriptionID: 0}); + connectionManager.removeFromEvictionBlockList({id: 'connectionID1', callbackID: 'callbackID1'}); expect(evictionBlocklist[ONYXKEYS.TEST_KEY]).toBeUndefined(); // inexistent connection ID, shouldn't do anything - expect(() => connectionManager.removeFromEvictionBlockList({id: 'connectionID0', callbackID: 'callbackID0', subscriptionID: 0})).not.toThrow(); + expect(() => connectionManager.removeFromEvictionBlockList({id: 'connectionID0', callbackID: 'callbackID0'})).not.toThrow(); }); }); }); From 365953d7e4a460bd42e384b4f860fb3b00c9be17 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Fri, 2 Aug 2024 15:37:57 +0100 Subject: [PATCH 18/31] Rename ConnectionMetadata to Connection --- lib/Onyx.ts | 6 +++--- lib/OnyxConnectionManager.ts | 16 ++++++++-------- lib/index.ts | 4 ++-- lib/useOnyx.ts | 4 ++-- lib/withOnyx/index.tsx | 4 ++-- tests/unit/onyxCacheTest.tsx | 4 ++-- tests/unit/onyxClearNativeStorageTest.ts | 4 ++-- tests/unit/onyxClearWebStorageTest.ts | 4 ++-- tests/unit/onyxTest.ts | 12 ++++++------ 9 files changed, 29 insertions(+), 29 deletions(-) diff --git a/lib/Onyx.ts b/lib/Onyx.ts index 3eaab5f3..2809f7d5 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -29,7 +29,7 @@ import type { } from './types'; import OnyxUtils from './OnyxUtils'; import logMessages from './logMessages'; -import type {ConnectionMetadata} from './OnyxConnectionManager'; +import type {Connection} from './OnyxConnectionManager'; import connectionManager from './OnyxConnectionManager'; /** Initialize the store with actions and listening for storage events */ @@ -93,7 +93,7 @@ function init({ * Note that it will not cause the component to have the loading prop set to true. * @returns a connection metadata object to use when calling `Onyx.disconnect()` */ -function connect(connectOptions: ConnectOptions): ConnectionMetadata { +function connect(connectOptions: ConnectOptions): Connection { return connectionManager.connect(connectOptions); } @@ -104,7 +104,7 @@ function connect(connectOptions: ConnectOptions): Co * * @param connection connection metadata object returned by call to `Onyx.connect()` */ -function disconnect(connectionMetadada: ConnectionMetadata): void { +function disconnect(connectionMetadada: Connection): void { connectionManager.disconnect(connectionMetadada); } diff --git a/lib/OnyxConnectionManager.ts b/lib/OnyxConnectionManager.ts index 47e59204..e82bcce9 100644 --- a/lib/OnyxConnectionManager.ts +++ b/lib/OnyxConnectionManager.ts @@ -7,7 +7,7 @@ import type {DefaultConnectCallback, DefaultConnectOptions, OnyxKey, OnyxValue} type ConnectCallback = DefaultConnectCallback; -type Connection = { +type ConnectionMetadata = { subscriptionID: number; onyxKey: OnyxKey; isConnectionMade: boolean; @@ -16,13 +16,13 @@ type Connection = { cachedCallbackKey?: OnyxKey; }; -type ConnectionMetadata = { +type Connection = { id: string; callbackID: string; }; class OnyxConnectionManager { - private connectionsMap: Map; + private connectionsMap: Map; private lastCallbackID: number; @@ -53,7 +53,7 @@ class OnyxConnectionManager { }); } - connect(connectOptions: ConnectOptions): ConnectionMetadata { + connect(connectOptions: ConnectOptions): Connection { const mapKey = this.connectionMapKey(connectOptions); let connection = this.connectionsMap.get(mapKey); let subscriptionID: number | undefined; @@ -111,7 +111,7 @@ class OnyxConnectionManager { * * @param connection connection metadata object returned by call to `Onyx.connect()` */ - disconnect(connectionMetadada: ConnectionMetadata): void { + disconnect(connectionMetadada: Connection): void { if (!connectionMetadada) { Logger.logAlert(`[ConnectionManager] Attempted to disconnect passing an undefined metadata object.`); return; @@ -144,7 +144,7 @@ class OnyxConnectionManager { } /** Keys added to this list can never be deleted. */ - addToEvictionBlockList(connectionMetadada: ConnectionMetadata): void { + addToEvictionBlockList(connectionMetadada: Connection): void { const connection = this.connectionsMap.get(connectionMetadada.id); if (!connection) { return; @@ -164,7 +164,7 @@ class OnyxConnectionManager { * Removes a key previously added to this list * which will enable it to be deleted again. */ - removeFromEvictionBlockList(connectionMetadada: ConnectionMetadata): void { + removeFromEvictionBlockList(connectionMetadada: Connection): void { const connection = this.connectionsMap.get(connectionMetadada.id); if (!connection) { return; @@ -185,4 +185,4 @@ const connectionManager = new OnyxConnectionManager(); export default connectionManager; -export type {ConnectionMetadata}; +export type {Connection}; diff --git a/lib/index.ts b/lib/index.ts index 65e37d1a..cde7a907 100644 --- a/lib/index.ts +++ b/lib/index.ts @@ -18,7 +18,7 @@ import type { OnyxMergeCollectionInput, } from './types'; import type {FetchStatus, ResultMetadata, UseOnyxResult} from './useOnyx'; -import type {ConnectionMetadata} from './OnyxConnectionManager'; +import type {Connection} from './OnyxConnectionManager'; import useOnyx from './useOnyx'; import withOnyx from './withOnyx'; import type {WithOnyxState} from './withOnyx/types'; @@ -47,5 +47,5 @@ export type { Selector, UseOnyxResult, WithOnyxState, - ConnectionMetadata, + Connection, }; diff --git a/lib/useOnyx.ts b/lib/useOnyx.ts index 8f81b15b..aafaa2c6 100644 --- a/lib/useOnyx.ts +++ b/lib/useOnyx.ts @@ -2,7 +2,7 @@ import {deepEqual, shallowEqual} from 'fast-equals'; import {useCallback, useEffect, useRef, useSyncExternalStore} from 'react'; import type {IsEqual} from 'type-fest'; import OnyxCache from './OnyxCache'; -import type {ConnectionMetadata} from './OnyxConnectionManager'; +import type {Connection} from './OnyxConnectionManager'; import connectionManager from './OnyxConnectionManager'; import OnyxUtils from './OnyxUtils'; import type {CollectionKeyBase, OnyxCollection, OnyxEntry, OnyxKey, OnyxValue, Selector} from './types'; @@ -70,7 +70,7 @@ function useOnyx>( options?: BaseUseOnyxOptions & UseOnyxInitialValueOption>, ): UseOnyxResult; function useOnyx>(key: TKey, options?: UseOnyxOptions): UseOnyxResult { - const connectionRef = useRef(null); + const connectionRef = useRef(null); const previousKey = usePrevious(key); // Used to stabilize the selector reference and avoid unnecessary calls to `getSnapshot()`. diff --git a/lib/withOnyx/index.tsx b/lib/withOnyx/index.tsx index 0564ff89..bd821a45 100644 --- a/lib/withOnyx/index.tsx +++ b/lib/withOnyx/index.tsx @@ -10,7 +10,7 @@ import type {GenericFunction, Mapping, OnyxKey, WithOnyxConnectOptions} from '.. import utils from '../utils'; import type {MapOnyxToState, WithOnyxInstance, WithOnyxMapping, WithOnyxProps, WithOnyxState} from './types'; import cache from '../OnyxCache'; -import type {ConnectionMetadata} from '../OnyxConnectionManager'; +import type {Connection} from '../OnyxConnectionManager'; import connectionManager from '../OnyxConnectionManager'; // This is a list of keys that can exist on a `mapping`, but are not directly related to loading data from Onyx. When the keys of a mapping are looped over to check @@ -68,7 +68,7 @@ export default function ( shouldDelayUpdates: boolean; - activeConnections: Record; + activeConnections: Record; tempState: WithOnyxState | undefined; diff --git a/tests/unit/onyxCacheTest.tsx b/tests/unit/onyxCacheTest.tsx index a7624cfe..0759044c 100644 --- a/tests/unit/onyxCacheTest.tsx +++ b/tests/unit/onyxCacheTest.tsx @@ -13,7 +13,7 @@ import type OnyxInstance from '../../lib/Onyx'; import type withOnyxType from '../../lib/withOnyx'; import type {InitOptions} from '../../lib/types'; import generateRange from '../utils/generateRange'; -import type {ConnectionMetadata} from '../../lib/OnyxConnectionManager'; +import type {Connection} from '../../lib/OnyxConnectionManager'; describe('Onyx', () => { describe('Cache Service', () => { @@ -529,7 +529,7 @@ describe('Onyx', () => { StorageMock.getItem.mockResolvedValue('"mockValue"'); const range = generateRange(0, 10); StorageMock.getAllKeys.mockResolvedValue(range.map((number) => `${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}${number}`)); - let connections: Array<{key: string; connection: ConnectionMetadata}> = []; + let connections: Array<{key: string; connection: Connection}> = []; // Given Onyx is configured with max 5 keys in cache return initOnyx({ diff --git a/tests/unit/onyxClearNativeStorageTest.ts b/tests/unit/onyxClearNativeStorageTest.ts index 5c6aea9d..bac2fd06 100644 --- a/tests/unit/onyxClearNativeStorageTest.ts +++ b/tests/unit/onyxClearNativeStorageTest.ts @@ -2,7 +2,7 @@ import waitForPromisesToResolve from '../utils/waitForPromisesToResolve'; import StorageMock from '../../lib/storage'; import Onyx from '../../lib/Onyx'; import type OnyxCache from '../../lib/OnyxCache'; -import type {ConnectionMetadata} from '../../lib/OnyxConnectionManager'; +import type {Connection} from '../../lib/OnyxConnectionManager'; const ONYX_KEYS = { DEFAULT_KEY: 'defaultKey', @@ -15,7 +15,7 @@ const MERGED_VALUE = 1; const DEFAULT_VALUE = 0; describe('Set data while storage is clearing', () => { - let connection: ConnectionMetadata | undefined; + let connection: Connection | undefined; let onyxValue: unknown; /** @type OnyxCache */ diff --git a/tests/unit/onyxClearWebStorageTest.ts b/tests/unit/onyxClearWebStorageTest.ts index b97338ce..6f736fe6 100644 --- a/tests/unit/onyxClearWebStorageTest.ts +++ b/tests/unit/onyxClearWebStorageTest.ts @@ -3,7 +3,7 @@ import StorageMock from '../../lib/storage'; import Onyx from '../../lib/Onyx'; import type OnyxCache from '../../lib/OnyxCache'; import type GenericCollection from '../utils/GenericCollection'; -import type {ConnectionMetadata} from '../../lib/OnyxConnectionManager'; +import type {Connection} from '../../lib/OnyxConnectionManager'; const ONYX_KEYS = { DEFAULT_KEY: 'defaultKey', @@ -17,7 +17,7 @@ const MERGED_VALUE = 'merged'; const DEFAULT_VALUE = 'default'; describe('Set data while storage is clearing', () => { - let connection: ConnectionMetadata | undefined; + let connection: Connection | undefined; let onyxValue: unknown; /** @type OnyxCache */ diff --git a/tests/unit/onyxTest.ts b/tests/unit/onyxTest.ts index 776ee26c..50f7fcfe 100644 --- a/tests/unit/onyxTest.ts +++ b/tests/unit/onyxTest.ts @@ -5,7 +5,7 @@ import OnyxUtils from '../../lib/OnyxUtils'; import type OnyxCache from '../../lib/OnyxCache'; import type {OnyxCollection, OnyxUpdate} from '../../lib/types'; import type GenericCollection from '../utils/GenericCollection'; -import type {ConnectionMetadata} from '../../lib/OnyxConnectionManager'; +import type {Connection} from '../../lib/OnyxConnectionManager'; const ONYX_KEYS = { TEST_KEY: 'test', @@ -30,7 +30,7 @@ Onyx.init({ }); describe('Onyx', () => { - let connection: ConnectionMetadata | undefined; + let connection: Connection | undefined; /** @type OnyxCache */ let cache: typeof OnyxCache; @@ -1111,7 +1111,7 @@ describe('Onyx', () => { }); it('should return a promise that completes when all update() operations are done', () => { - const connections: ConnectionMetadata[] = []; + const connections: Connection[] = []; const testCallback = jest.fn(); const otherTestCallback = jest.fn(); @@ -1335,7 +1335,7 @@ describe('Onyx', () => { }); it('should not call a collection item subscriber if the value did not change', () => { - const connections: ConnectionMetadata[] = []; + const connections: Connection[] = []; const cat = `${ONYX_KEYS.COLLECTION.ANIMALS}cat`; const dog = `${ONYX_KEYS.COLLECTION.ANIMALS}dog`; @@ -1403,7 +1403,7 @@ describe('Onyx', () => { describe('update', () => { it('should squash all updates of collection-related keys into a single mergeCollection call', () => { - const connections: ConnectionMetadata[] = []; + const connections: Connection[] = []; const routineRoute = `${ONYX_KEYS.COLLECTION.ROUTES}routine`; const holidayRoute = `${ONYX_KEYS.COLLECTION.ROUTES}holiday`; @@ -1504,7 +1504,7 @@ describe('Onyx', () => { }); it('should return a promise that completes when all update() operations are done', () => { - const connections: ConnectionMetadata[] = []; + const connections: Connection[] = []; const bob = `${ONYX_KEYS.COLLECTION.PEOPLE}bob`; const lisa = `${ONYX_KEYS.COLLECTION.PEOPLE}lisa`; From 0a629179cd08e3668c5f75a40fb9ab5125623f9f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Mon, 5 Aug 2024 16:41:22 +0100 Subject: [PATCH 19/31] Add comments and refactor some variables --- lib/Onyx.ts | 40 +++--- lib/OnyxConnectionManager.ts | 177 +++++++++++++++++------- lib/OnyxUtils.ts | 96 ++++++------- lib/types.ts | 2 +- lib/withOnyx/types.ts | 2 +- tests/unit/OnyxConnectionManagerTest.ts | 2 + 6 files changed, 193 insertions(+), 126 deletions(-) diff --git a/lib/Onyx.ts b/lib/Onyx.ts index 2809f7d5..d9135a3f 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -66,46 +66,40 @@ function init({ } /** - * Subscribes a react component's state directly to a store key + * Connects to an Onyx key given the options passed and listens to its changes. * * @example + * ```ts * const connection = Onyx.connect({ * key: ONYXKEYS.SESSION, * callback: onSessionChange, * }); + * ``` * - * @param mapping the mapping information to connect Onyx to the components state - * @param mapping.key ONYXKEY to subscribe to - * @param [mapping.statePropertyName] the name of the property in the state to connect the data to - * @param [mapping.withOnyxInstance] whose setState() method will be called with any changed data - * This is used by React components to connect to Onyx - * @param [mapping.callback] a method that will be called with changed data - * This is used by any non-React code to connect to Onyx - * @param [mapping.initWithStoredValues] If set to false, then no data will be prefilled into the - * component - * @param [mapping.waitForCollectionCallback] If set to true, it will return the entire collection to the callback as a single object - * @param [mapping.selector] THIS PARAM IS ONLY USED WITH withOnyx(). If included, this will be used to subscribe to a subset of an Onyx key's data. - * The sourceData and withOnyx state are passed to the selector and should return the simplified data. Using this setting on `withOnyx` can have very positive - * performance benefits because the component will only re-render when the subset of data changes. Otherwise, any change of data on any property would normally - * cause the component to re-render (and that can be expensive from a performance standpoint). - * @param [mapping.initialValue] THIS PARAM IS ONLY USED WITH withOnyx(). - * If included, this will be passed to the component so that something can be rendered while data is being fetched from the DB. - * Note that it will not cause the component to have the loading prop set to true. - * @returns a connection metadata object to use when calling `Onyx.disconnect()` + * @param connectOptions The options object that will define the behavior of the connection. + * @returns The connection object to use when calling `Onyx.disconnect()`. */ function connect(connectOptions: ConnectOptions): Connection { return connectionManager.connect(connectOptions); } /** - * Remove the listener for a react component + * Disconnects and removes the listener from the Onyx key. + * * @example + * ```ts + * const connection = Onyx.connect({ + * key: ONYXKEYS.SESSION, + * callback: onSessionChange, + * }); + * * Onyx.disconnect(connection); + * ``` * - * @param connection connection metadata object returned by call to `Onyx.connect()` + * @param connection Connection object returned by calling `Onyx.connect()`. */ -function disconnect(connectionMetadada: Connection): void { - connectionManager.disconnect(connectionMetadada); +function disconnect(connection: Connection): void { + connectionManager.disconnect(connection); } /** diff --git a/lib/OnyxConnectionManager.ts b/lib/OnyxConnectionManager.ts index e82bcce9..84bf29a1 100644 --- a/lib/OnyxConnectionManager.ts +++ b/lib/OnyxConnectionManager.ts @@ -7,44 +7,102 @@ import type {DefaultConnectCallback, DefaultConnectOptions, OnyxKey, OnyxValue} type ConnectCallback = DefaultConnectCallback; +/** + * Represents the connection's metadata that contains the necessary properties + * to handle that connection. + */ type ConnectionMetadata = { + /** + * The subscription ID returned by `OnyxUtils.subscribeToKey()` that is associated to this connection. + */ subscriptionID: number; + + /** + * The Onyx key associated to this connection. + */ onyxKey: OnyxKey; + + /** + * Whether the first connection's callback was fired or not. + */ isConnectionMade: boolean; + + /** + * A map of the subcriber's callbacks associated to this connection. + */ callbacks: Map; + + /** + * The last callback value returned by `OnyxUtils.subscribeToKey()`'s callback. + */ cachedCallbackValue?: OnyxValue; + + /** + * The last callback key returned by `OnyxUtils.subscribeToKey()`'s callback. + */ cachedCallbackKey?: OnyxKey; }; +/** + * Represents the connection object returned by `Onyx.connect()`. + */ type Connection = { + /** + * The ID used to identify this particular connection. + */ id: string; + + /** + * The ID of the subscriber's callback that is associated to this connection. + */ callbackID: string; }; +/** + * Manages Onyx connections of `Onyx.connect()`, `useOnyx()` and `withOnyx()` subscribers. + */ class OnyxConnectionManager { + /** + * A map where the key is the connection ID generated inside `connect()` and the value is the metadata of that connection. + */ private connectionsMap: Map; + /** + * Stores the last generated callback ID which will be incremented when making a new connection. + */ private lastCallbackID: number; constructor() { this.connectionsMap = new Map(); this.lastCallbackID = 0; - bindAll(this, 'fireCallbacks', 'connectionMapKey', 'connect', 'disconnect', 'disconnectAll', 'addToEvictionBlockList', 'removeFromEvictionBlockList'); + bindAll(this, 'fireCallbacks', 'generateConnectionID', 'connect', 'disconnect', 'disconnectAll', 'addToEvictionBlockList', 'removeFromEvictionBlockList'); } - private connectionMapKey(connectOptions: ConnectOptions): string { + /** + * Generates a connection ID based on the `connectOptions` object passed to the function. + * The properties used to generate the key are handpicked for performance reasons and + * according to their purpose and effect they produce in the Onyx connection. + */ + private generateConnectionID(connectOptions: ConnectOptions): string { let suffix = ''; + // We will generate a unique key in any of the following situations: + // - `connectOptions.reuseConnection` is `false`. That means the subscriber explicitly wants the connection to not be reused. + // - `connectOptions.initWithStoredValues` is `false`. This flag changes the subscription flow when set to `false`, so the connection can't be reused. + // - `withOnyxInstance` is defined inside `connectOptions`. That means the subscriber is a `withOnyx` HOC and therefore doesn't support connection reuse. if (connectOptions.reuseConnection === false || connectOptions.initWithStoredValues === false || 'withOnyxInstance' in connectOptions) { suffix += `,uniqueID=${Str.guid()}`; } - return `key=${connectOptions.key},initWithStoredValues=${connectOptions.initWithStoredValues ?? true},waitForCollectionCallback=${ + return `onyxKey=${connectOptions.key},initWithStoredValues=${connectOptions.initWithStoredValues ?? true},waitForCollectionCallback=${ connectOptions.waitForCollectionCallback ?? false }${suffix}`; } + /** + * Fires all the subscribers callbacks associated with that connection key. + */ private fireCallbacks(mapKey: string): void { const connection = this.connectionsMap.get(mapKey); @@ -53,89 +111,108 @@ class OnyxConnectionManager { }); } + /** + * Connects to an Onyx key given the options passed and listens to its changes. + * + * @param connectOptions The options object that will define the behavior of the connection. + * @returns The connection object to use when calling `disconnect()`. + */ connect(connectOptions: ConnectOptions): Connection { - const mapKey = this.connectionMapKey(connectOptions); - let connection = this.connectionsMap.get(mapKey); + const connectionID = this.generateConnectionID(connectOptions); + let connectionMetadata = this.connectionsMap.get(connectionID); let subscriptionID: number | undefined; const callbackID = String(this.lastCallbackID++); - if (!connection) { + // If there is no connection yet for that connection ID, we create a new one. + if (!connectionMetadata) { let callback: ConnectCallback | undefined; + // If the subscriber is a `withOnyx` HOC we don't define `callback` as the HOC will use + // its own logic to handle the data. if (!('withOnyxInstance' in connectOptions)) { callback = (value, key) => { - const createdConnection = this.connectionsMap.get(mapKey); + const createdConnection = this.connectionsMap.get(connectionID); if (createdConnection) { + // We signal that the first connection was made and now any new subscribers + // can fire their callbacks immediately with the cached value when connecting. createdConnection.isConnectionMade = true; createdConnection.cachedCallbackValue = value; createdConnection.cachedCallbackKey = key; - this.fireCallbacks(mapKey); + this.fireCallbacks(connectionID); } }; } - subscriptionID = OnyxUtils.connectToKey({ + subscriptionID = OnyxUtils.subscribeToKey({ ...(connectOptions as DefaultConnectOptions), callback, }); - connection = { + connectionMetadata = { subscriptionID, onyxKey: connectOptions.key, isConnectionMade: false, callbacks: new Map(), }; - this.connectionsMap.set(mapKey, connection); + this.connectionsMap.set(connectionID, connectionMetadata); } + // We add the subscriber's callback to the list of callbacks associated with this connection. if (connectOptions.callback) { - connection.callbacks.set(callbackID, connectOptions.callback as ConnectCallback); + connectionMetadata.callbacks.set(callbackID, connectOptions.callback as ConnectCallback); } - if (connection.isConnectionMade) { + // If the first connection is already made we want any new subscribers to receive the cached callback value immediately. + if (connectionMetadata.isConnectionMade) { + // Defer the callback execution to the next tick of the event loop. + // This ensures that the current execution flow completes and the result connection object is available when the callback fires. Promise.resolve().then(() => { - (connectOptions as DefaultConnectOptions).callback?.(connection.cachedCallbackValue, connection.cachedCallbackKey as OnyxKey); + (connectOptions as DefaultConnectOptions).callback?.(connectionMetadata.cachedCallbackValue, connectionMetadata.cachedCallbackKey as OnyxKey); }); } - return {id: mapKey, callbackID}; + return {id: connectionID, callbackID}; } /** - * Remove the listener for a react component - * @example - * Onyx.disconnect(connection); + * Disconnects and removes the listener from the Onyx key. * - * @param connection connection metadata object returned by call to `Onyx.connect()` + * @param connection Connection object returned by calling `connect()`. */ - disconnect(connectionMetadada: Connection): void { - if (!connectionMetadada) { - Logger.logAlert(`[ConnectionManager] Attempted to disconnect passing an undefined metadata object.`); + disconnect(connection: Connection): void { + if (!connection) { + Logger.logAlert(`[ConnectionManager] Attempted to disconnect passing an undefined connection object.`); return; } - const connection = this.connectionsMap.get(connectionMetadada.id); - if (!connection) { + const connectionMetadata = this.connectionsMap.get(connection.id); + if (!connectionMetadata) { + Logger.logAlert(`[ConnectionManager] Attempted to disconnect but no connection was found.`); return; } - connection.callbacks.delete(connectionMetadada.callbackID); + // Removes the callback from the connection's callbacks map. + connectionMetadata.callbacks.delete(connection.callbackID); - if (connection.callbacks.size === 0) { - OnyxUtils.disconnectFromKey(connection.subscriptionID); - this.removeFromEvictionBlockList(connectionMetadada); + // If the connection's callbacks map is empty we can safely unsubscribe from the Onyx key. + if (connectionMetadata.callbacks.size === 0) { + OnyxUtils.unsubscribeFromKey(connectionMetadata.subscriptionID); + this.removeFromEvictionBlockList(connection); - this.connectionsMap.delete(connectionMetadada.id); + this.connectionsMap.delete(connection.id); } } + /** + * Disconnect all subscribers from Onyx. + */ disconnectAll(): void { - this.connectionsMap.forEach((connection, connectionID) => { - OnyxUtils.disconnectFromKey(connection.subscriptionID); - connection.callbacks.forEach((_, callbackID) => { + this.connectionsMap.forEach((connectionMetadata, connectionID) => { + OnyxUtils.unsubscribeFromKey(connectionMetadata.subscriptionID); + connectionMetadata.callbacks.forEach((_, callbackID) => { this.removeFromEvictionBlockList({id: connectionID, callbackID}); }); }); @@ -143,40 +220,42 @@ class OnyxConnectionManager { this.connectionsMap.clear(); } - /** Keys added to this list can never be deleted. */ - addToEvictionBlockList(connectionMetadada: Connection): void { - const connection = this.connectionsMap.get(connectionMetadada.id); - if (!connection) { + /** + * Adds the connection to the eviction block list. Connections added to this list can never be evicted. + * */ + addToEvictionBlockList(connection: Connection): void { + const connectionMetadata = this.connectionsMap.get(connection.id); + if (!connectionMetadata) { return; } - this.removeFromEvictionBlockList(connectionMetadada); + this.removeFromEvictionBlockList(connection); const evictionBlocklist = OnyxUtils.getEvictionBlocklist(); - if (!evictionBlocklist[connection.onyxKey]) { - evictionBlocklist[connection.onyxKey] = []; + if (!evictionBlocklist[connectionMetadata.onyxKey]) { + evictionBlocklist[connectionMetadata.onyxKey] = []; } - evictionBlocklist[connection.onyxKey]?.push(`${connectionMetadada.id}_${connectionMetadada.callbackID}`); + evictionBlocklist[connectionMetadata.onyxKey]?.push(`${connection.id}_${connection.callbackID}`); } /** - * Removes a key previously added to this list - * which will enable it to be deleted again. + * Removes a connection previously added to this list + * which will enable it to be evicted again. */ - removeFromEvictionBlockList(connectionMetadada: Connection): void { - const connection = this.connectionsMap.get(connectionMetadada.id); - if (!connection) { + removeFromEvictionBlockList(connection: Connection): void { + const connectionMetadata = this.connectionsMap.get(connection.id); + if (!connectionMetadata) { return; } const evictionBlocklist = OnyxUtils.getEvictionBlocklist(); - evictionBlocklist[connection.onyxKey] = - evictionBlocklist[connection.onyxKey]?.filter((evictionKey) => evictionKey !== `${connectionMetadada.id}_${connectionMetadada.callbackID}`) ?? []; + evictionBlocklist[connectionMetadata.onyxKey] = + evictionBlocklist[connectionMetadata.onyxKey]?.filter((evictionKey) => evictionKey !== `${connection.id}_${connection.callbackID}`) ?? []; // Remove the key if there are no more subscribers. - if (evictionBlocklist[connection.onyxKey]?.length === 0) { - delete evictionBlocklist[connection.onyxKey]; + if (evictionBlocklist[connectionMetadata.onyxKey]?.length === 0) { + delete evictionBlocklist[connectionMetadata.onyxKey]; } } } diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index 9ebb8ad7..96feb6b5 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -54,8 +54,8 @@ const callbackToStateMapping: Record> = {}; // Keeps a copy of the values of the onyx collection keys as a map for faster lookups let onyxCollectionKeySet = new Set(); -// Holds a mapping of the connected key to the connectionID for faster lookups -const onyxKeyToConnectionIDs = new Map(); +// Holds a mapping of the connected key to the subscriptionID for faster lookups +const onyxKeyToSubscriptionIDs = new Map(); // Holds a list of keys that have been directly subscribed to or recently modified from least to most recent let recentlyAccessedKeys: OnyxKey[] = []; @@ -76,8 +76,8 @@ let batchUpdatesQueue: Array<() => void> = []; let snapshotKey: OnyxKey | null = null; -// Keeps track of the last connectionID that was used so we can keep incrementing it -let lastConnectionID = 0; +// Keeps track of the last subscriptionID that was used so we can keep incrementing it +let lastSubscriptionID = 0; // Connections can be made before `Onyx.init`. They would wait for this task before resolving const deferredInitTask = createDeferredTask(); @@ -346,29 +346,29 @@ function multiGet(keys: CollectionKeyBase[]): Promise id !== connectionID); - onyxKeyToConnectionIDs.set(subscriber.key, updatedConnectionIDs); + if (subscriber && onyxKeyToSubscriptionIDs.has(subscriber.key)) { + const updatedSubscriptionsIDs = onyxKeyToSubscriptionIDs.get(subscriber.key).filter((id: number) => id !== subscriptionID); + onyxKeyToSubscriptionIDs.set(subscriber.key, updatedSubscriptionsIDs); } } @@ -788,13 +788,13 @@ function keyChanged( // Given the amount of times this function is called we need to make sure we are not iterating over all subscribers every time. On the other hand, we don't need to // do the same in keysChanged, because we only call that function when a collection key changes, and it doesn't happen that often. // For performance reason, we look for the given key and later if don't find it we look for the collection key, instead of checking if it is a collection key first. - let stateMappingKeys = onyxKeyToConnectionIDs.get(key) ?? []; + let stateMappingKeys = onyxKeyToSubscriptionIDs.get(key) ?? []; const collectionKey = getCollectionKey(key); const plainCollectionKey = collectionKey.lastIndexOf('_') !== -1 ? collectionKey : undefined; if (plainCollectionKey) { // Getting the collection key from the specific key because only collection keys were stored in the mapping. - stateMappingKeys = [...stateMappingKeys, ...(onyxKeyToConnectionIDs.get(plainCollectionKey) ?? [])]; + stateMappingKeys = [...stateMappingKeys, ...(onyxKeyToSubscriptionIDs.get(plainCollectionKey) ?? [])]; if (stateMappingKeys.length === 0) { return; } @@ -926,7 +926,7 @@ function keyChanged( function sendDataToConnection(mapping: Mapping, value: OnyxValue | null, matchedKey: TKey | undefined, isBatched: boolean): void { // If the mapping no longer exists then we should not send any data. // This means our subscriber disconnected or withOnyx wrapped component unmounted. - if (!callbackToStateMapping[mapping.connectionID]) { + if (!callbackToStateMapping[mapping.subscriptionID]) { return; } @@ -1217,13 +1217,7 @@ function doAllCollectionItemsBelongToSameParent( } /** - * Subscribes a react component's state directly to a store key - * - * @example - * const connectionID = OnyxUtils.connectToKey({ - * key: ONYXKEYS.SESSION, - * callback: onSessionChange, - * }); + * Subscribes to an Onyx key and listens to its changes. * * @param mapping the mapping information to connect Onyx to the components state * @param mapping.key ONYXKEY to subscribe to @@ -1242,21 +1236,21 @@ function doAllCollectionItemsBelongToSameParent( * @param [mapping.initialValue] THIS PARAM IS ONLY USED WITH withOnyx(). * If included, this will be passed to the component so that something can be rendered while data is being fetched from the DB. * Note that it will not cause the component to have the loading prop set to true. - * @returns an ID to use when calling `OnyxUtils.disconnectFromKey()` + * @returns The subscription ID to use when calling `OnyxUtils.unsubscribeFromKey()`. */ -function connectToKey(connectOptions: ConnectOptions): number { +function subscribeToKey(connectOptions: ConnectOptions): number { const mapping = connectOptions as Mapping; - const connectionID = lastConnectionID++; - callbackToStateMapping[connectionID] = mapping as Mapping; - callbackToStateMapping[connectionID].connectionID = connectionID; + const subscriptionID = lastSubscriptionID++; + callbackToStateMapping[subscriptionID] = mapping as Mapping; + callbackToStateMapping[subscriptionID].subscriptionID = subscriptionID; - // When keyChanged is called, a key is passed and the method looks through all the Subscribers in callbackToStateMapping for the matching key to get the connectionID + // When keyChanged is called, a key is passed and the method looks through all the Subscribers in callbackToStateMapping for the matching key to get the subscriptionID // to avoid having to loop through all the Subscribers all the time (even when just one connection belongs to one key), - // We create a mapping from key to lists of connectionIDs to access the specific list of connectionIDs. - OnyxUtils.storeKeyByConnections(mapping.key, callbackToStateMapping[connectionID].connectionID); + // We create a mapping from key to lists of subscriptionIDs to access the specific list of subscriptionIDs. + OnyxUtils.storeKeyByConnections(mapping.key, callbackToStateMapping[subscriptionID].subscriptionID); if (mapping.initWithStoredValues === false) { - return connectionID; + return subscriptionID; } // Commit connection only after init passes @@ -1337,25 +1331,23 @@ function connectToKey(connectOptions: ConnectOptions console.error('Warning: Onyx.connect() was found without a callback or withOnyxInstance'); }); - // The connectionID is returned back to the caller so that it can be used to clean up the connection when it's no longer needed - // by calling Onyx.disconnect(connectionID). - return connectionID; + // The subscriptionID is returned back to the caller so that it can be used to clean up the connection when it's no longer needed + // by calling OnyxUtils.disconnect(subscriptionID). + return subscriptionID; } /** - * Remove the listener for a react component - * @example - * Onyx.disconnectFromKey(connectionID); + * Disconnects and removes the listener from the Onyx key. * - * @param connectionID unique id returned by call to `OnyxUtils.connectToKey()` + * @param subscriptionID Subscription ID returned by calling `OnyxUtils.subscribeToKey()`. */ -function disconnectFromKey(connectionID: number): void { - if (!callbackToStateMapping[connectionID]) { +function unsubscribeFromKey(subscriptionID: number): void { + if (!callbackToStateMapping[subscriptionID]) { return; } - OnyxUtils.deleteKeyByConnections(lastConnectionID); - delete callbackToStateMapping[connectionID]; + OnyxUtils.deleteKeyByConnections(lastSubscriptionID); + delete callbackToStateMapping[subscriptionID]; } const OnyxUtils = { @@ -1399,14 +1391,14 @@ const OnyxUtils = { prepareKeyValuePairsForStorage, applyMerge, initializeWithDefaultKeyStates, - storeKeyByConnections, - deleteKeyByConnections, + storeKeyByConnections: storeKeyBySubscriptions, + deleteKeyByConnections: deleteKeyBySubscriptions, getSnapshotKey, multiGet, isValidNonEmptyCollectionForMerge, doAllCollectionItemsBelongToSameParent, - connectToKey, - disconnectFromKey, + subscribeToKey, + unsubscribeFromKey, getEvictionBlocklist, }; diff --git a/lib/types.ts b/lib/types.ts index e201e723..c1ddecde 100644 --- a/lib/types.ts +++ b/lib/types.ts @@ -310,7 +310,7 @@ type WithOnyxConnectOptions = ConnectOptions & { }; type Mapping = WithOnyxConnectOptions & { - connectionID: number; + subscriptionID: number; }; /** diff --git a/lib/withOnyx/types.ts b/lib/withOnyx/types.ts index 4b8b85ec..f536fec3 100644 --- a/lib/withOnyx/types.ts +++ b/lib/withOnyx/types.ts @@ -109,7 +109,7 @@ type Mapping = Mapping & { - connectionID: number; + subscriptionID: number; previousKey?: OnyxKey; }; diff --git a/tests/unit/OnyxConnectionManagerTest.ts b/tests/unit/OnyxConnectionManagerTest.ts index aa8ee3be..2ccab325 100644 --- a/tests/unit/OnyxConnectionManagerTest.ts +++ b/tests/unit/OnyxConnectionManagerTest.ts @@ -9,6 +9,8 @@ import type GenericCollection from '../utils/GenericCollection'; import waitForPromisesToResolve from '../utils/waitForPromisesToResolve'; import OnyxUtils from '../../lib/OnyxUtils'; +// We need access to `connectionsMap` during the tests but the property is private, +// so this workaround allows us to have access to it. // eslint-disable-next-line dot-notation const connectionsMap = connectionManager['connectionsMap']; From 80e56a536bd217c15dae0da1795265b64a12a2cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Mon, 5 Aug 2024 17:08:33 +0100 Subject: [PATCH 20/31] Add tests for generateConnectionID --- lib/OnyxConnectionManager.ts | 13 ++--- tests/unit/OnyxConnectionManagerTest.ts | 71 +++++++++++++++++-------- 2 files changed, 55 insertions(+), 29 deletions(-) diff --git a/lib/OnyxConnectionManager.ts b/lib/OnyxConnectionManager.ts index 84bf29a1..a572f6e2 100644 --- a/lib/OnyxConnectionManager.ts +++ b/lib/OnyxConnectionManager.ts @@ -76,18 +76,19 @@ class OnyxConnectionManager { this.connectionsMap = new Map(); this.lastCallbackID = 0; - bindAll(this, 'fireCallbacks', 'generateConnectionID', 'connect', 'disconnect', 'disconnectAll', 'addToEvictionBlockList', 'removeFromEvictionBlockList'); + bindAll(this, 'generateConnectionID', 'fireCallbacks', 'connect', 'disconnect', 'disconnectAll', 'addToEvictionBlockList', 'removeFromEvictionBlockList'); } /** * Generates a connection ID based on the `connectOptions` object passed to the function. - * The properties used to generate the key are handpicked for performance reasons and + * + * The properties used to generate the ID are handpicked for performance reasons and * according to their purpose and effect they produce in the Onyx connection. */ private generateConnectionID(connectOptions: ConnectOptions): string { let suffix = ''; - // We will generate a unique key in any of the following situations: + // We will generate a unique ID in any of the following situations: // - `connectOptions.reuseConnection` is `false`. That means the subscriber explicitly wants the connection to not be reused. // - `connectOptions.initWithStoredValues` is `false`. This flag changes the subscription flow when set to `false`, so the connection can't be reused. // - `withOnyxInstance` is defined inside `connectOptions`. That means the subscriber is a `withOnyx` HOC and therefore doesn't support connection reuse. @@ -101,10 +102,10 @@ class OnyxConnectionManager { } /** - * Fires all the subscribers callbacks associated with that connection key. + * Fires all the subscribers callbacks associated with that connection ID. */ - private fireCallbacks(mapKey: string): void { - const connection = this.connectionsMap.get(mapKey); + private fireCallbacks(connectionID: string): void { + const connection = this.connectionsMap.get(connectionID); connection?.callbacks.forEach((callback) => { callback(connection.cachedCallbackValue, connection.cachedCallbackKey as OnyxKey); diff --git a/tests/unit/OnyxConnectionManagerTest.ts b/tests/unit/OnyxConnectionManagerTest.ts index 2ccab325..a9f77a3c 100644 --- a/tests/unit/OnyxConnectionManagerTest.ts +++ b/tests/unit/OnyxConnectionManagerTest.ts @@ -9,10 +9,22 @@ import type GenericCollection from '../utils/GenericCollection'; import waitForPromisesToResolve from '../utils/waitForPromisesToResolve'; import OnyxUtils from '../../lib/OnyxUtils'; -// We need access to `connectionsMap` during the tests but the property is private, -// so this workaround allows us to have access to it. +// We need access to `connectionsMap` and `generateConnectionID` during the tests but the properties are private, +// so this workaround allows us to have access to them. // eslint-disable-next-line dot-notation const connectionsMap = connectionManager['connectionsMap']; +// eslint-disable-next-line dot-notation +const generateConnectionID = connectionManager['generateConnectionID']; + +function generateEmptyWithOnyxInstance() { + return new (class { + // eslint-disable-next-line @typescript-eslint/no-empty-function + setStateProxy() {} + + // eslint-disable-next-line @typescript-eslint/no-empty-function + setWithOnyxState() {} + })() as unknown as WithOnyxInstance; +} const ONYXKEYS = { TEST_KEY: 'test', @@ -35,6 +47,37 @@ describe('OnyxConnectionManager', () => { connectionManager.disconnectAll(); }); + describe('generateConnectionID', () => { + it('should generate a stable connection ID', async () => { + const connectionID = generateConnectionID({key: ONYXKEYS.TEST_KEY}); + expect(connectionID).toEqual(`onyxKey=${ONYXKEYS.TEST_KEY},initWithStoredValues=true,waitForCollectionCallback=false`); + }); + + it("should generate a stable connection ID regardless of the order which the option's properties were passed", async () => { + const connectionID = generateConnectionID({key: ONYXKEYS.TEST_KEY, waitForCollectionCallback: true, initWithStoredValues: true}); + expect(connectionID).toEqual(`onyxKey=${ONYXKEYS.TEST_KEY},initWithStoredValues=true,waitForCollectionCallback=true`); + }); + + it('should generate unique connection IDs if certain options are passed', async () => { + const connectionID1 = generateConnectionID({key: ONYXKEYS.TEST_KEY, reuseConnection: false}); + const connectionID2 = generateConnectionID({key: ONYXKEYS.TEST_KEY, reuseConnection: false}); + expect(connectionID1.startsWith(`onyxKey=${ONYXKEYS.TEST_KEY},initWithStoredValues=true,waitForCollectionCallback=false,uniqueID=`)).toBeTruthy(); + expect(connectionID2.startsWith(`onyxKey=${ONYXKEYS.TEST_KEY},initWithStoredValues=true,waitForCollectionCallback=false,uniqueID=`)).toBeTruthy(); + expect(connectionID1).not.toEqual(connectionID2); + + const connectionID3 = generateConnectionID({key: ONYXKEYS.TEST_KEY, initWithStoredValues: false}); + expect(connectionID3.startsWith(`onyxKey=${ONYXKEYS.TEST_KEY},initWithStoredValues=false,waitForCollectionCallback=false,uniqueID=`)).toBeTruthy(); + + const connectionID4 = generateConnectionID({ + key: ONYXKEYS.TEST_KEY, + displayName: 'Component1', + statePropertyName: 'prop1', + withOnyxInstance: generateEmptyWithOnyxInstance(), + } as WithOnyxConnectOptions); + expect(connectionID4.startsWith(`onyxKey=${ONYXKEYS.TEST_KEY},initWithStoredValues=true,waitForCollectionCallback=false,uniqueID=`)).toBeTruthy(); + }); + }); + describe('connect / disconnect', () => { it('should connect to a key and fire the callback with its value', async () => { await StorageMock.setItem(ONYXKEYS.TEST_KEY, 'test'); @@ -235,26 +278,14 @@ describe('OnyxConnectionManager', () => { key: ONYXKEYS.TEST_KEY, displayName: 'Component1', statePropertyName: 'prop1', - withOnyxInstance: new (class { - // eslint-disable-next-line @typescript-eslint/no-empty-function - setStateProxy() {} - - // eslint-disable-next-line @typescript-eslint/no-empty-function - setWithOnyxState() {} - })() as unknown as WithOnyxInstance, + withOnyxInstance: generateEmptyWithOnyxInstance(), } as WithOnyxConnectOptions); const connection2 = connectionManager.connect({ key: ONYXKEYS.TEST_KEY, displayName: 'Component2', statePropertyName: 'prop2', - withOnyxInstance: new (class { - // eslint-disable-next-line @typescript-eslint/no-empty-function - setStateProxy() {} - - // eslint-disable-next-line @typescript-eslint/no-empty-function - setWithOnyxState() {} - })() as unknown as WithOnyxInstance, + withOnyxInstance: generateEmptyWithOnyxInstance(), } as WithOnyxConnectOptions); await act(async () => waitForPromisesToResolve()); @@ -282,13 +313,7 @@ describe('OnyxConnectionManager', () => { key: ONYXKEYS.TEST_KEY, displayName: 'Component2', statePropertyName: 'prop2', - withOnyxInstance: new (class { - // eslint-disable-next-line @typescript-eslint/no-empty-function - setStateProxy() {} - - // eslint-disable-next-line @typescript-eslint/no-empty-function - setWithOnyxState() {} - })() as unknown as WithOnyxInstance, + withOnyxInstance: generateEmptyWithOnyxInstance(), } as WithOnyxConnectOptions); expect(connection1.id).not.toEqual(connection2.id); From 717f0253283036d518321fde47683df8bdcf4039 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Mon, 5 Aug 2024 18:31:15 +0100 Subject: [PATCH 21/31] Update descriptions --- lib/OnyxUtils.ts | 18 +----------------- lib/types.ts | 40 +++++++++++++++++++++++++++++++++++++--- lib/useOnyx.ts | 4 ++-- 3 files changed, 40 insertions(+), 22 deletions(-) diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index 96feb6b5..a12f45b0 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -1219,23 +1219,7 @@ function doAllCollectionItemsBelongToSameParent( /** * Subscribes to an Onyx key and listens to its changes. * - * @param mapping the mapping information to connect Onyx to the components state - * @param mapping.key ONYXKEY to subscribe to - * @param [mapping.statePropertyName] the name of the property in the state to connect the data to - * @param [mapping.withOnyxInstance] whose setState() method will be called with any changed data - * This is used by React components to connect to Onyx - * @param [mapping.callback] a method that will be called with changed data - * This is used by any non-React code to connect to Onyx - * @param [mapping.initWithStoredValues] If set to false, then no data will be prefilled into the - * component - * @param [mapping.waitForCollectionCallback] If set to true, it will return the entire collection to the callback as a single object - * @param [mapping.selector] THIS PARAM IS ONLY USED WITH withOnyx(). If included, this will be used to subscribe to a subset of an Onyx key's data. - * The sourceData and withOnyx state are passed to the selector and should return the simplified data. Using this setting on `withOnyx` can have very positive - * performance benefits because the component will only re-render when the subset of data changes. Otherwise, any change of data on any property would normally - * cause the component to re-render (and that can be expensive from a performance standpoint). - * @param [mapping.initialValue] THIS PARAM IS ONLY USED WITH withOnyx(). - * If included, this will be passed to the component so that something can be rendered while data is being fetched from the DB. - * Note that it will not cause the component to have the loading prop set to true. + * @param connectOptions The options object that will define the behavior of the connection. * @returns The subscription ID to use when calling `OnyxUtils.unsubscribeFromKey()`. */ function subscribeToKey(connectOptions: ConnectOptions): number { diff --git a/lib/types.ts b/lib/types.ts index c1ddecde..0b913833 100644 --- a/lib/types.ts +++ b/lib/types.ts @@ -261,8 +261,16 @@ type Collection = { }; /** Represents the base options used in `Onyx.connect()` method. */ +// NOTE: Any changes to this type like adding or removing options must be accounted in OnyxConnectionManager's `generateConnectionID()` method! type BaseConnectOptions = { + /** If set to `false`, then the initial data will be only sent to the callback function if it changes. */ initWithStoredValues?: boolean; + + /** + * If set to `false`, the connection won't be reused between other subscribers that are listening to the same Onyx key + * with the same connect configurations. + */ + reuseConnection?: boolean; }; /** Represents the callback function used in `Onyx.connect()` method with a regular key. */ @@ -272,19 +280,29 @@ type DefaultConnectCallback = (value: OnyxEntry = (value: NonUndefined>, key: undefined) => void; /** Represents the options used in `Onyx.connect()` method with a regular key. */ +// NOTE: Any changes to this type like adding or removing options must be accounted in OnyxConnectionManager's `generateConnectionID()` method! type DefaultConnectOptions = BaseConnectOptions & { + /** The Onyx key to subscribe to. */ key: TKey; + + /** A function that will be called when the Onyx data we are subscribed changes. */ callback?: DefaultConnectCallback; + + /** If set to `true`, it will return the entire collection to the callback as a single object. */ waitForCollectionCallback?: false; - reuseConnection?: boolean; }; /** Represents the options used in `Onyx.connect()` method with a collection key. */ +// NOTE: Any changes to this type like adding or removing options must be accounted in OnyxConnectionManager's `generateConnectionID()` method! type CollectionConnectOptions = BaseConnectOptions & { + /** The Onyx key to subscribe to. */ key: TKey extends CollectionKeyBase ? TKey : never; + + /** A function that will be called when the Onyx data we are subscribed changes. */ callback?: CollectionConnectCallback; + + /** If set to `true`, it will return the entire collection to the callback as a single object. */ waitForCollectionCallback: true; - reuseConnection?: boolean; }; /** @@ -298,14 +316,30 @@ type CollectionConnectOptions = BaseConnectOptions & { * If `waitForCollectionCallback` is `false` or not specified, the `key` can be any Onyx key and `callback` will be triggered with updates of each collection item * and will pass `value` as an `OnyxEntry`. */ +// NOTE: Any changes to this type like adding or removing options must be accounted in OnyxConnectionManager's `generateConnectionID()` method! type ConnectOptions = DefaultConnectOptions | CollectionConnectOptions; -/** Represents additional `Onyx.connect()` options used inside withOnyx HOC. */ +/** Represents additional `Onyx.connect()` options used inside `withOnyx()` HOC. */ +// NOTE: Any changes to this type like adding or removing options must be accounted in OnyxConnectionManager's `generateConnectionID()` method! type WithOnyxConnectOptions = ConnectOptions & { + /** The `withOnyx` class instance to be internally passed. */ withOnyxInstance: WithOnyxInstance; + + /** The name of the component's prop that is connected to the Onyx key. */ statePropertyName: string; + + /** The component's display name. */ displayName: string; + + /** + * This will be used to subscribe to a subset of an Onyx key's data. + * Using this setting on `withOnyx` can have very positive performance benefits because the component will only re-render + * when the subset of data changes. Otherwise, any change of data on any property would normally + * cause the component to re-render (and that can be expensive from a performance standpoint). + */ selector?: Selector; + + /** Determines if this key in this subscription is safe to be evicted. */ canEvict?: boolean; }; diff --git a/lib/useOnyx.ts b/lib/useOnyx.ts index aafaa2c6..34d8d5f9 100644 --- a/lib/useOnyx.ts +++ b/lib/useOnyx.ts @@ -16,12 +16,12 @@ type BaseUseOnyxOptions = { canEvict?: boolean; /** - * If set to false, then no data will be prefilled into the component. + * If set to `false`, then no data will be prefilled into the component. */ initWithStoredValues?: boolean; /** - * If set to true, data will be retrieved from cache during the first render even if there is a pending merge for the key. + * If set to `true`, data will be retrieved from cache during the first render even if there is a pending merge for the key. */ allowStaleData?: boolean; }; From 62c407500df187fc5a3d12a6a2334d59788d0693 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Mon, 5 Aug 2024 18:36:49 +0100 Subject: [PATCH 22/31] Remove self access to OnyxUtils --- lib/OnyxUtils.ts | 34 ++++++++++++++++------------------ 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index a12f45b0..56eec899 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -1201,7 +1201,7 @@ function isValidNonEmptyCollectionForMerge function doAllCollectionItemsBelongToSameParent(collectionKey: TKey, collectionKeys: string[]): boolean { let hasCollectionKeyCheckFailed = false; collectionKeys.forEach((dataKey) => { - if (OnyxUtils.isKeyMatch(collectionKey, dataKey)) { + if (isKeyMatch(collectionKey, dataKey)) { return; } @@ -1231,7 +1231,7 @@ function subscribeToKey(connectOptions: ConnectOptions(connectOptions: ConnectOptions OnyxUtils.addKeyToRecentlyAccessedIfNeeded(mapping)) + .then(() => addKeyToRecentlyAccessedIfNeeded(mapping)) .then(() => { // Performance improvement // If the mapping is connected to an onyx key that is not a collection @@ -1247,7 +1247,7 @@ function subscribeToKey(connectOptions: ConnectOptions { // We search all the keys in storage to see if any are a "match" for the subscriber we are connecting so that we @@ -1255,7 +1255,7 @@ function subscribeToKey(connectOptions: ConnectOptions { - if (!OnyxUtils.isKeyMatch(mapping.key, key)) { + if (!isKeyMatch(mapping.key, key)) { return; } matchingKeys.push(key); @@ -1265,13 +1265,13 @@ function subscribeToKey(connectOptions: ConnectOptions(connectOptions: ConnectOptions { + multiGet(matchingKeys).then((values) => { values.forEach((val, key) => { - OnyxUtils.sendDataToConnection(mapping, val as OnyxValue, key as TKey, true); + sendDataToConnection(mapping, val as OnyxValue, key as TKey, true); }); }); return; } // If we are not subscribed to a collection key then there's only a single key to send an update for. - OnyxUtils.get(mapping.key).then((val) => OnyxUtils.sendDataToConnection(mapping, val as OnyxValue, mapping.key, true)); + get(mapping.key).then((val) => sendDataToConnection(mapping, val as OnyxValue, mapping.key, true)); return; } // If we have a withOnyxInstance that means a React component has subscribed via the withOnyx() HOC and we need to // group collection key member data into an object. if ('withOnyxInstance' in mapping && mapping.withOnyxInstance) { - if (OnyxUtils.isCollectionKey(mapping.key)) { - OnyxUtils.getCollectionDataAndSendAsObject(matchingKeys, mapping); + if (isCollectionKey(mapping.key)) { + getCollectionDataAndSendAsObject(matchingKeys, mapping); return; } // If the subscriber is not using a collection key then we just send a single value back to the subscriber - OnyxUtils.get(mapping.key).then((val) => OnyxUtils.sendDataToConnection(mapping, val as OnyxValue, mapping.key, true)); + get(mapping.key).then((val) => sendDataToConnection(mapping, val as OnyxValue, mapping.key, true)); return; } @@ -1330,7 +1330,7 @@ function unsubscribeFromKey(subscriptionID: number): void { return; } - OnyxUtils.deleteKeyByConnections(lastSubscriptionID); + deleteKeyBySubscriptions(lastSubscriptionID); delete callbackToStateMapping[subscriptionID]; } @@ -1375,8 +1375,6 @@ const OnyxUtils = { prepareKeyValuePairsForStorage, applyMerge, initializeWithDefaultKeyStates, - storeKeyByConnections: storeKeyBySubscriptions, - deleteKeyByConnections: deleteKeyBySubscriptions, getSnapshotKey, multiGet, isValidNonEmptyCollectionForMerge, From c3b7c087cf5003d959cd48394f567e96fa588ad4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Wed, 7 Aug 2024 16:20:50 +0100 Subject: [PATCH 23/31] Fix tests --- tests/unit/onyxTest.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/onyxTest.ts b/tests/unit/onyxTest.ts index c090c986..6b6450c7 100644 --- a/tests/unit/onyxTest.ts +++ b/tests/unit/onyxTest.ts @@ -210,7 +210,7 @@ describe('Onyx', () => { it('should notify key subscribers that use a underscore in their name', () => { const mockCallback = jest.fn(); - connectionID = Onyx.connect({ + connection = Onyx.connect({ key: ONYX_KEYS.KEY_WITH_UNDERSCORE, callback: mockCallback, }); From c11c987bfed550acf02cbb82d4a47e19c3ad1f5a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Wed, 7 Aug 2024 18:53:56 +0100 Subject: [PATCH 24/31] Change logs level --- lib/OnyxConnectionManager.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/OnyxConnectionManager.ts b/lib/OnyxConnectionManager.ts index a572f6e2..14305be5 100644 --- a/lib/OnyxConnectionManager.ts +++ b/lib/OnyxConnectionManager.ts @@ -185,13 +185,13 @@ class OnyxConnectionManager { */ disconnect(connection: Connection): void { if (!connection) { - Logger.logAlert(`[ConnectionManager] Attempted to disconnect passing an undefined connection object.`); + Logger.logInfo(`[ConnectionManager] Attempted to disconnect passing an undefined connection object.`); return; } const connectionMetadata = this.connectionsMap.get(connection.id); if (!connectionMetadata) { - Logger.logAlert(`[ConnectionManager] Attempted to disconnect but no connection was found.`); + Logger.logInfo(`[ConnectionManager] Attempted to disconnect but no connection was found.`); return; } From da88b9bfe0b156d26c47c4e8ffc15a4e52ab0893 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Wed, 7 Aug 2024 19:03:06 +0100 Subject: [PATCH 25/31] Minor fixes --- lib/OnyxConnectionManager.ts | 2 -- tests/unit/useOnyxTest.ts | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/OnyxConnectionManager.ts b/lib/OnyxConnectionManager.ts index 14305be5..716b8ae2 100644 --- a/lib/OnyxConnectionManager.ts +++ b/lib/OnyxConnectionManager.ts @@ -230,8 +230,6 @@ class OnyxConnectionManager { return; } - this.removeFromEvictionBlockList(connection); - const evictionBlocklist = OnyxUtils.getEvictionBlocklist(); if (!evictionBlocklist[connectionMetadata.onyxKey]) { evictionBlocklist[connectionMetadata.onyxKey] = []; diff --git a/tests/unit/useOnyxTest.ts b/tests/unit/useOnyxTest.ts index 87305189..19388f36 100644 --- a/tests/unit/useOnyxTest.ts +++ b/tests/unit/useOnyxTest.ts @@ -550,7 +550,7 @@ describe('useOnyx', () => { }); }); - // This suite test must be the last one to avoid problems when running the other tests here. + // This test suite must be the last one to avoid problems when running the other tests here. describe('canEvict', () => { const error = (key: string) => `canEvict can't be used on key '${key}'. This key must explicitly be flagged as safe for removal by adding it to Onyx.init({safeEvictionKeys: []}).`; From cc7403e940b7a4d29ea2d7e27e4868d08c80e40b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Wed, 14 Aug 2024 16:37:50 +0100 Subject: [PATCH 26/31] Update docs --- API-INTERNAL.md | 153 +++++++++++++++++++++++++++++++++++++++++------- API.md | 43 +++++++------- 2 files changed, 151 insertions(+), 45 deletions(-) diff --git a/API-INTERNAL.md b/API-INTERNAL.md index db3e45f5..4381fede 100644 --- a/API-INTERNAL.md +++ b/API-INTERNAL.md @@ -17,6 +17,12 @@
getDefaultKeyStates()

Getter - returns the default key states.

+
getDeferredInitTask()
+

Getter - returns the deffered init task.

+
+
getEvictionBlocklist()
+

Getter - returns the eviction block list.

+
initStoreValues(keys, initialKeyStates, safeEvictionKeys)

Sets the initial values for the Onyx store

@@ -34,11 +40,20 @@ The resulting collection will only contain items that are returned by the select
get()

Get some data from the store

+
storeKeyBySubscriptions(subscriptionID, key)
+

Stores a subscription ID associated with a given key.

+
+
deleteKeyBySubscriptions(subscriptionID)
+

Deletes a subscription ID associated with its corresponding key.

+
getAllKeys()

Returns current key names stored in persisted storage

+
getCollectionKeys()
+

Returns set of all registered collection keys

+
isCollectionKey()
-

Checks to see if the a subscriber's supplied key +

Checks to see if the subscriber's supplied key is associated with a collection of keys.

splitCollectionMemberKey(key) ⇒
@@ -51,6 +66,15 @@ or if the provided key is a collection member key (in case our configured key is
isSafeEvictionKey()

Checks to see if this key has been flagged as safe for removal.

+
getCollectionKey(key) ⇒ string
+

It extracts the non-numeric collection identifier of a given key.

+

For example:

+
    +
  • getCollectionKey("report_123") would return "report_"
    • +
  • getCollectionKey("report") would return "report"
    • +
  • getCollectionKey("report_") would return "report_"
    • +
+
tryGetCachedValue()

Tries to get a value from the cache. If the value is not present in cache it will return the default value or undefined. If the requested key is a collection, it will return an object with all the collection members.

@@ -63,13 +87,6 @@ If the requested key is a collection, it will return an object with all the coll recently accessed key should be at the head and the most recently accessed key at the tail.

-
removeFromEvictionBlockList()
-

Removes a key previously added to this list -which will enable it to be deleted again.

-
-
addToEvictionBlockList()
-

Keys added to this list can never be deleted.

-
addAllSafeEvictionKeysToRecentlyAccessedList()

Take all the keys that are safe to evict and add them to the recently accessed list when initializing the app. This @@ -129,6 +146,18 @@ to an array of key-value pairs in the above format and removes key-value pairs t

initializeWithDefaultKeyStates()

Merge user provided default key value pairs.

+
isValidNonEmptyCollectionForMerge()
+

Validate the collection is not empty and has a correct type before applying mergeCollection()

+
+
doAllCollectionItemsBelongToSameParent()
+

Verify if all the collection keys belong to the same parent

+
+
subscribeToKey(connectOptions) ⇒
+

Subscribes to an Onyx key and listens to its changes.

+
+
unsubscribeFromKey(subscriptionID)
+

Disconnects and removes the listener from the Onyx key.

+
@@ -154,6 +183,18 @@ Getter - returns the callback to state mapping. ## getDefaultKeyStates() Getter - returns the default key states. +**Kind**: global function + + +## getDeferredInitTask() +Getter - returns the deffered init task. + +**Kind**: global function + + +## getEvictionBlocklist() +Getter - returns the eviction block list. + **Kind**: global function @@ -191,16 +232,45 @@ The resulting collection will only contain items that are returned by the select Get some data from the store **Kind**: global function + + +## storeKeyBySubscriptions(subscriptionID, key) +Stores a subscription ID associated with a given key. + +**Kind**: global function + +| Param | Description | +| --- | --- | +| subscriptionID | A subscription ID of the subscriber. | +| key | A key that the subscriber is subscribed to. | + + + +## deleteKeyBySubscriptions(subscriptionID) +Deletes a subscription ID associated with its corresponding key. + +**Kind**: global function + +| Param | Description | +| --- | --- | +| subscriptionID | The subscription ID to be deleted. | + ## getAllKeys() Returns current key names stored in persisted storage +**Kind**: global function + + +## getCollectionKeys() +Returns set of all registered collection keys + **Kind**: global function ## isCollectionKey() -Checks to see if the a subscriber's supplied key +Checks to see if the subscriber's supplied key is associated with a collection of keys. **Kind**: global function @@ -229,6 +299,23 @@ or if the provided key is a collection member key (in case our configured key is Checks to see if this key has been flagged as safe for removal. **Kind**: global function + + +## getCollectionKey(key) ⇒ string +It extracts the non-numeric collection identifier of a given key. + +For example: +- `getCollectionKey("report_123")` would return "report_" +- `getCollectionKey("report")` would return "report" +- `getCollectionKey("report_")` would return "report_" + +**Kind**: global function +**Returns**: string - The pure key without any numeric + +| Param | Type | Description | +| --- | --- | --- | +| key | OnyxKey | The key to process. | + ## tryGetCachedValue() @@ -249,19 +336,6 @@ Add a key to the list of recently accessed keys. The least recently accessed key should be at the head and the most recently accessed key at the tail. -**Kind**: global function - - -## removeFromEvictionBlockList() -Removes a key previously added to this list -which will enable it to be deleted again. - -**Kind**: global function - - -## addToEvictionBlockList() -Keys added to this list can never be deleted. - **Kind**: global function @@ -399,3 +473,38 @@ Merges an array of changes with an existing value Merge user provided default key value pairs. **Kind**: global function + + +## isValidNonEmptyCollectionForMerge() +Validate the collection is not empty and has a correct type before applying mergeCollection() + +**Kind**: global function + + +## doAllCollectionItemsBelongToSameParent() +Verify if all the collection keys belong to the same parent + +**Kind**: global function + + +## subscribeToKey(connectOptions) ⇒ +Subscribes to an Onyx key and listens to its changes. + +**Kind**: global function +**Returns**: The subscription ID to use when calling `OnyxUtils.unsubscribeFromKey()`. + +| Param | Description | +| --- | --- | +| connectOptions | The options object that will define the behavior of the connection. | + + + +## unsubscribeFromKey(subscriptionID) +Disconnects and removes the listener from the Onyx key. + +**Kind**: global function + +| Param | Description | +| --- | --- | +| subscriptionID | Subscription ID returned by calling `OnyxUtils.subscribeToKey()`. | + diff --git a/API.md b/API.md index 59e76ce7..e18a0027 100644 --- a/API.md +++ b/API.md @@ -8,11 +8,11 @@
init()

Initialize the store with actions and listening for storage events

-
connect(mapping) ⇒
-

Subscribes a react component's state directly to a store key

+
connect(connectOptions) ⇒
+

Connects to an Onyx key given the options passed and listens to its changes.

-
disconnect(connectionID)
-

Remove the listener for a react component

+
disconnect(connection)
+

Disconnects and removes the listener from the Onyx key.

set(key, value)

Write a value to our store with the given key

@@ -60,45 +60,42 @@ Initialize the store with actions and listening for storage events **Kind**: global function -## connect(mapping) ⇒ -Subscribes a react component's state directly to a store key +## connect(connectOptions) ⇒ +Connects to an Onyx key given the options passed and listens to its changes. **Kind**: global function -**Returns**: an ID to use when calling disconnect +**Returns**: The connection object to use when calling `Onyx.disconnect()`. | Param | Description | | --- | --- | -| mapping | the mapping information to connect Onyx to the components state | -| mapping.key | ONYXKEY to subscribe to | -| [mapping.statePropertyName] | the name of the property in the state to connect the data to | -| [mapping.withOnyxInstance] | whose setState() method will be called with any changed data This is used by React components to connect to Onyx | -| [mapping.callback] | a method that will be called with changed data This is used by any non-React code to connect to Onyx | -| [mapping.initWithStoredValues] | If set to false, then no data will be prefilled into the component | -| [mapping.waitForCollectionCallback] | If set to true, it will return the entire collection to the callback as a single object | -| [mapping.selector] | THIS PARAM IS ONLY USED WITH withOnyx(). If included, this will be used to subscribe to a subset of an Onyx key's data. The sourceData and withOnyx state are passed to the selector and should return the simplified data. Using this setting on `withOnyx` can have very positive performance benefits because the component will only re-render when the subset of data changes. Otherwise, any change of data on any property would normally cause the component to re-render (and that can be expensive from a performance standpoint). | -| [mapping.initialValue] | THIS PARAM IS ONLY USED WITH withOnyx(). If included, this will be passed to the component so that something can be rendered while data is being fetched from the DB. Note that it will not cause the component to have the loading prop set to true. | +| connectOptions | The options object that will define the behavior of the connection. | **Example** -```js -const connectionID = Onyx.connect({ +```ts +const connection = Onyx.connect({ key: ONYXKEYS.SESSION, callback: onSessionChange, }); ``` -## disconnect(connectionID) -Remove the listener for a react component +## disconnect(connection) +Disconnects and removes the listener from the Onyx key. **Kind**: global function | Param | Description | | --- | --- | -| connectionID | unique id returned by call to Onyx.connect() | +| connection | Connection object returned by calling `Onyx.connect()`. | **Example** -```js -Onyx.disconnect(connectionID); +```ts +const connection = Onyx.connect({ + key: ONYXKEYS.SESSION, + callback: onSessionChange, +}); + +Onyx.disconnect(connection); ``` From 52bbf424d021223cf9068957c4cf3e08a0b047ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Wed, 14 Aug 2024 16:40:54 +0100 Subject: [PATCH 27/31] Remove param from guid() function --- lib/Str.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/lib/Str.ts b/lib/Str.ts index 8666f2e7..7940b384 100644 --- a/lib/Str.ts +++ b/lib/Str.ts @@ -23,16 +23,14 @@ function result unknown, TArgs extends unknow /** * A simple GUID generator taken from https://stackoverflow.com/a/32760401/9114791 - * - * @param [prefix] an optional prefix to put in front of the guid */ -function guid(prefix = ''): string { +function guid(): string { function s4() { return Math.floor((1 + Math.random()) * 0x10000) .toString(16) .substring(1); } - return `${prefix}${s4()}${s4()}-${s4()}-${s4()}-${s4()}-${s4()}${s4()}${s4()}`; + return `${s4()}${s4()}-${s4()}-${s4()}-${s4()}-${s4()}${s4()}${s4()}`; } export {guid, result, startsWith}; From 8fef1148bf05c0d4066621c7f9368f1a86158be0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Tue, 20 Aug 2024 13:25:42 +0100 Subject: [PATCH 28/31] Implement hasWithOnyxInstance() method --- lib/OnyxConnectionManager.ts | 5 +++-- lib/OnyxUtils.ts | 10 +++++----- lib/utils.ts | 11 +++++++++-- 3 files changed, 17 insertions(+), 9 deletions(-) diff --git a/lib/OnyxConnectionManager.ts b/lib/OnyxConnectionManager.ts index 716b8ae2..a99ad250 100644 --- a/lib/OnyxConnectionManager.ts +++ b/lib/OnyxConnectionManager.ts @@ -4,6 +4,7 @@ import type {ConnectOptions} from './Onyx'; import OnyxUtils from './OnyxUtils'; import * as Str from './Str'; import type {DefaultConnectCallback, DefaultConnectOptions, OnyxKey, OnyxValue} from './types'; +import utils from './utils'; type ConnectCallback = DefaultConnectCallback; @@ -92,7 +93,7 @@ class OnyxConnectionManager { // - `connectOptions.reuseConnection` is `false`. That means the subscriber explicitly wants the connection to not be reused. // - `connectOptions.initWithStoredValues` is `false`. This flag changes the subscription flow when set to `false`, so the connection can't be reused. // - `withOnyxInstance` is defined inside `connectOptions`. That means the subscriber is a `withOnyx` HOC and therefore doesn't support connection reuse. - if (connectOptions.reuseConnection === false || connectOptions.initWithStoredValues === false || 'withOnyxInstance' in connectOptions) { + if (connectOptions.reuseConnection === false || connectOptions.initWithStoredValues === false || utils.hasWithOnyxInstance(connectOptions)) { suffix += `,uniqueID=${Str.guid()}`; } @@ -131,7 +132,7 @@ class OnyxConnectionManager { // If the subscriber is a `withOnyx` HOC we don't define `callback` as the HOC will use // its own logic to handle the data. - if (!('withOnyxInstance' in connectOptions)) { + if (!utils.hasWithOnyxInstance(connectOptions)) { callback = (value, key) => { const createdConnection = this.connectionsMap.get(connectionID); if (createdConnection) { diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index a792b737..36182b83 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -666,7 +666,7 @@ function keysChanged( } // React component subscriber found. - if ('withOnyxInstance' in subscriber && subscriber.withOnyxInstance) { + if (utils.hasWithOnyxInstance(subscriber)) { if (!notifyWithOnyxSubscibers) { continue; } @@ -848,7 +848,7 @@ function keyChanged( } // Subscriber connected via withOnyx() HOC - if ('withOnyxInstance' in subscriber && subscriber.withOnyxInstance) { + if (utils.hasWithOnyxInstance(subscriber)) { if (!notifyWithOnyxSubscribers) { continue; } @@ -954,7 +954,7 @@ function sendDataToConnection(mapping: Mapping, valu return; } - if ('withOnyxInstance' in mapping && mapping.withOnyxInstance) { + if (utils.hasWithOnyxInstance(mapping)) { let newData: OnyxValue = value; // If the mapping has a selector, then the component's state must only be updated with the data @@ -1006,7 +1006,7 @@ function addKeyToRecentlyAccessedIfNeeded(mapping: Mapping // Try to free some cache whenever we connect to a safe eviction key cache.removeLeastRecentlyUsedKeys(); - if ('withOnyxInstance' in mapping && mapping.withOnyxInstance && !isCollectionKey(mapping.key)) { + if (utils.hasWithOnyxInstance(mapping) && !isCollectionKey(mapping.key)) { // All React components subscribing to a key flagged as a safe eviction key must implement the canEvict property. if (mapping.canEvict === undefined) { throw new Error(`Cannot subscribe to safe eviction key '${mapping.key}' without providing a canEvict value.`); @@ -1334,7 +1334,7 @@ function subscribeToKey(connectOptions: ConnectOptions; type EmptyValue = EmptyObject | null | undefined; @@ -196,4 +196,11 @@ function omit(obj: Record, condition: string | string[] return filterObject(obj, condition, false); } -export default {isEmptyObject, fastMerge, formatActionName, removeNestedNullValues, checkCompatibilityWithExistingValue, pick, omit}; +/** + * Whether the connect options has the `withOnyxInstance` property defined, that is, it's used by the `withOnyx()` HOC. + */ +function hasWithOnyxInstance(mapping: ConnectOptions) { + return 'withOnyxInstance' in mapping && mapping.withOnyxInstance; +} + +export default {isEmptyObject, fastMerge, formatActionName, removeNestedNullValues, checkCompatibilityWithExistingValue, pick, omit, hasWithOnyxInstance}; From 5ac0fd1515fcb572a03e89892c74b345ce2a87e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Tue, 20 Aug 2024 13:26:57 +0100 Subject: [PATCH 29/31] Address review comments --- lib/OnyxConnectionManager.ts | 3 ++- lib/OnyxUtils.ts | 11 +---------- 2 files changed, 3 insertions(+), 11 deletions(-) diff --git a/lib/OnyxConnectionManager.ts b/lib/OnyxConnectionManager.ts index a99ad250..f2564c58 100644 --- a/lib/OnyxConnectionManager.ts +++ b/lib/OnyxConnectionManager.ts @@ -29,7 +29,7 @@ type ConnectionMetadata = { isConnectionMade: boolean; /** - * A map of the subcriber's callbacks associated to this connection. + * A map of the subscriber's callbacks associated to this connection. */ callbacks: Map; @@ -77,6 +77,7 @@ class OnyxConnectionManager { this.connectionsMap = new Map(); this.lastCallbackID = 0; + // Binds all public methods to prevent problems with `this`. bindAll(this, 'generateConnectionID', 'fireCallbacks', 'connect', 'disconnect', 'disconnectAll', 'addToEvictionBlockList', 'removeFromEvictionBlockList'); } diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index 36182b83..ce74124d 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -103,13 +103,6 @@ function getMergeQueuePromise(): Record> { return mergeQueuePromise; } -/** - * Getter - returns the callback to state mapping. - */ -function getCallbackToStateMapping(): Record> { - return callbackToStateMapping; -} - /** * Getter - returns the default key states. */ @@ -1349,7 +1342,7 @@ function subscribeToKey(connectOptions: ConnectOptions Date: Tue, 20 Aug 2024 15:27:13 +0100 Subject: [PATCH 30/31] Send collection key on connect callback --- lib/OnyxUtils.ts | 4 ++-- lib/types.ts | 2 +- tests/unit/onyxClearWebStorageTest.ts | 4 ++-- tests/unit/onyxTest.ts | 20 ++++++++++---------- 4 files changed, 15 insertions(+), 15 deletions(-) diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index ce74124d..57c718b7 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -624,7 +624,7 @@ function keysChanged( // send the whole cached collection. if (isSubscribedToCollectionKey) { if (subscriber.waitForCollectionCallback) { - subscriber.callback(cachedCollection, undefined); + subscriber.callback(cachedCollection, subscriber.key); continue; } @@ -829,7 +829,7 @@ function keyChanged( } cachedCollection[key] = value; - subscriber.callback(cachedCollection, undefined); + subscriber.callback(cachedCollection, subscriber.key); continue; } diff --git a/lib/types.ts b/lib/types.ts index 0b913833..3a6696ce 100644 --- a/lib/types.ts +++ b/lib/types.ts @@ -277,7 +277,7 @@ type BaseConnectOptions = { type DefaultConnectCallback = (value: OnyxEntry, key: TKey) => void; /** Represents the callback function used in `Onyx.connect()` method with a collection key. */ -type CollectionConnectCallback = (value: NonUndefined>, key: undefined) => void; +type CollectionConnectCallback = (value: NonUndefined>, key: TKey) => void; /** Represents the options used in `Onyx.connect()` method with a regular key. */ // NOTE: Any changes to this type like adding or removing options must be accounted in OnyxConnectionManager's `generateConnectionID()` method! diff --git a/tests/unit/onyxClearWebStorageTest.ts b/tests/unit/onyxClearWebStorageTest.ts index 6f736fe6..d2894db6 100644 --- a/tests/unit/onyxClearWebStorageTest.ts +++ b/tests/unit/onyxClearWebStorageTest.ts @@ -170,9 +170,9 @@ describe('Set data while storage is clearing', () => { test_3: 3, test_4: 4, }, - undefined, + ONYX_KEYS.COLLECTION.TEST, ); - expect(collectionCallback).toHaveBeenLastCalledWith({}, undefined); + expect(collectionCallback).toHaveBeenLastCalledWith({}, ONYX_KEYS.COLLECTION.TEST); }) ); }); diff --git a/tests/unit/onyxTest.ts b/tests/unit/onyxTest.ts index 6b6450c7..08e3600b 100644 --- a/tests/unit/onyxTest.ts +++ b/tests/unit/onyxTest.ts @@ -1017,7 +1017,7 @@ describe('Onyx', () => { expect(mockCallback).toHaveBeenNthCalledWith(1, undefined, undefined); // AND the value for the second call should be collectionUpdate since the collection was updated - expect(mockCallback).toHaveBeenNthCalledWith(2, collectionUpdate, undefined); + expect(mockCallback).toHaveBeenNthCalledWith(2, collectionUpdate, ONYX_KEYS.COLLECTION.TEST_POLICY); }) ); }); @@ -1072,7 +1072,7 @@ describe('Onyx', () => { expect(mockCallback).toHaveBeenCalledTimes(2); // AND the value for the second call should be collectionUpdate - expect(mockCallback).toHaveBeenLastCalledWith(collectionUpdate, undefined); + expect(mockCallback).toHaveBeenLastCalledWith(collectionUpdate, ONYX_KEYS.COLLECTION.TEST_POLICY); }) ); }); @@ -1107,7 +1107,7 @@ describe('Onyx', () => { expect(mockCallback).toHaveBeenCalledTimes(2); // And the value for the second call should be collectionUpdate - expect(mockCallback).toHaveBeenNthCalledWith(2, collectionUpdate, undefined); + expect(mockCallback).toHaveBeenNthCalledWith(2, collectionUpdate, ONYX_KEYS.COLLECTION.TEST_POLICY); }) // When merge is called again with the same collection not modified @@ -1148,7 +1148,7 @@ describe('Onyx', () => { expect(mockCallback).toHaveBeenCalledTimes(1); // And the value for the second call should be collectionUpdate - expect(mockCallback).toHaveBeenNthCalledWith(1, collectionUpdate, undefined); + expect(mockCallback).toHaveBeenNthCalledWith(1, collectionUpdate, ONYX_KEYS.COLLECTION.TEST_POLICY); }) // When merge is called again with the same collection not modified @@ -1186,7 +1186,7 @@ describe('Onyx', () => { ]).then(() => { expect(collectionCallback).toHaveBeenCalledTimes(2); expect(collectionCallback).toHaveBeenNthCalledWith(1, undefined, undefined); - expect(collectionCallback).toHaveBeenNthCalledWith(2, {[itemKey]: {a: 'a'}}, undefined); + expect(collectionCallback).toHaveBeenNthCalledWith(2, {[itemKey]: {a: 'a'}}, ONYX_KEYS.COLLECTION.TEST_UPDATE); expect(testCallback).toHaveBeenCalledTimes(2); expect(testCallback).toHaveBeenNthCalledWith(1, undefined, undefined); @@ -1425,7 +1425,7 @@ describe('Onyx', () => { }) .then(() => { expect(collectionCallback).toHaveBeenCalledTimes(3); - expect(collectionCallback).toHaveBeenCalledWith(collectionDiff, undefined); + expect(collectionCallback).toHaveBeenCalledWith(collectionDiff, ONYX_KEYS.COLLECTION.ANIMALS); // Cat hasn't changed from its original value, expect only the initial connect callback expect(catCallback).toHaveBeenCalledTimes(1); @@ -1556,7 +1556,7 @@ describe('Onyx', () => { }, }, }, - undefined, + ONYX_KEYS.COLLECTION.ROUTES, ); connections.map((id) => Onyx.disconnect(id)); @@ -1626,7 +1626,7 @@ describe('Onyx', () => { { [cat]: {age: 3, sound: 'meow'}, }, - undefined, + ONYX_KEYS.COLLECTION.ANIMALS, ); expect(animalsCollectionCallback).toHaveBeenNthCalledWith( 2, @@ -1634,7 +1634,7 @@ describe('Onyx', () => { [cat]: {age: 3, sound: 'meow'}, [dog]: {size: 'M', sound: 'woof'}, }, - undefined, + ONYX_KEYS.COLLECTION.ANIMALS, ); expect(catCallback).toHaveBeenNthCalledWith(1, {age: 3, sound: 'meow'}, cat); @@ -1645,7 +1645,7 @@ describe('Onyx', () => { [bob]: {age: 25, car: 'sedan'}, [lisa]: {age: 21, car: 'SUV'}, }, - undefined, + ONYX_KEYS.COLLECTION.PEOPLE, ); connections.map((id) => Onyx.disconnect(id)); From 971819c30f34ac11ca32f907751d0028c7563f84 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Tue, 20 Aug 2024 15:27:21 +0100 Subject: [PATCH 31/31] Update docs --- API-INTERNAL.md | 9 --------- API.md | 7 +++++++ lib/Onyx.ts | 10 ++++++++++ 3 files changed, 17 insertions(+), 9 deletions(-) diff --git a/API-INTERNAL.md b/API-INTERNAL.md index 4381fede..bb1c647b 100644 --- a/API-INTERNAL.md +++ b/API-INTERNAL.md @@ -11,9 +11,6 @@
getMergeQueuePromise()

Getter - returns the merge queue promise.

-
getCallbackToStateMapping()
-

Getter - returns the callback to state mapping.

-
getDefaultKeyStates()

Getter - returns the default key states.

@@ -171,12 +168,6 @@ Getter - returns the merge queue. ## getMergeQueuePromise() Getter - returns the merge queue promise. -**Kind**: global function - - -## getCallbackToStateMapping() -Getter - returns the callback to state mapping. - **Kind**: global function diff --git a/API.md b/API.md index e18a0027..a6287849 100644 --- a/API.md +++ b/API.md @@ -69,6 +69,13 @@ Connects to an Onyx key given the options passed and listens to its changes. | Param | Description | | --- | --- | | connectOptions | The options object that will define the behavior of the connection. | +| connectOptions.key | The Onyx key to subscribe to. | +| connectOptions.callback | A function that will be called when the Onyx data we are subscribed changes. | +| connectOptions.waitForCollectionCallback | If set to `true`, it will return the entire collection to the callback as a single object. | +| connectOptions.withOnyxInstance | The `withOnyx` class instance to be internally passed. **Only used inside `withOnyx()` HOC.** | +| connectOptions.statePropertyName | The name of the component's prop that is connected to the Onyx key. **Only used inside `withOnyx()` HOC.** | +| connectOptions.displayName | The component's display name. **Only used inside `withOnyx()` HOC.** | +| connectOptions.selector | This will be used to subscribe to a subset of an Onyx key's data. **Only used inside `useOnyx()` hook or `withOnyx()` HOC.** Using this setting on `useOnyx()` or `withOnyx()` can have very positive performance benefits because the component will only re-render when the subset of data changes. Otherwise, any change of data on any property would normally cause the component to re-render (and that can be expensive from a performance standpoint). | **Example** ```ts diff --git a/lib/Onyx.ts b/lib/Onyx.ts index a08a5084..88be4a2f 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -77,6 +77,16 @@ function init({ * ``` * * @param connectOptions The options object that will define the behavior of the connection. + * @param connectOptions.key The Onyx key to subscribe to. + * @param connectOptions.callback A function that will be called when the Onyx data we are subscribed changes. + * @param connectOptions.waitForCollectionCallback If set to `true`, it will return the entire collection to the callback as a single object. + * @param connectOptions.withOnyxInstance The `withOnyx` class instance to be internally passed. **Only used inside `withOnyx()` HOC.** + * @param connectOptions.statePropertyName The name of the component's prop that is connected to the Onyx key. **Only used inside `withOnyx()` HOC.** + * @param connectOptions.displayName The component's display name. **Only used inside `withOnyx()` HOC.** + * @param connectOptions.selector This will be used to subscribe to a subset of an Onyx key's data. **Only used inside `useOnyx()` hook or `withOnyx()` HOC.** + * Using this setting on `useOnyx()` or `withOnyx()` can have very positive performance benefits because the component will only re-render + * when the subset of data changes. Otherwise, any change of data on any property would normally + * cause the component to re-render (and that can be expensive from a performance standpoint). * @returns The connection object to use when calling `Onyx.disconnect()`. */ function connect(connectOptions: ConnectOptions): Connection {