From 92817c4bac4873ee17a5e1b907afed48518f098c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hanno=20J=2E=20G=C3=B6decke?= Date: Mon, 18 Nov 2024 13:27:26 +0100 Subject: [PATCH 01/17] wip: re-enable performance metrics --- .gitignore | 2 ++ lib/Onyx.ts | 19 +++++++++++++++++++ lib/metrics.ts | 49 +++++++++++++++++++++++++++++++++++++++++++++++++ lib/types.ts | 6 ++++++ package.json | 3 +-- 5 files changed, 77 insertions(+), 2 deletions(-) create mode 100644 lib/metrics.ts diff --git a/.gitignore b/.gitignore index a46d06d8..5592c6d4 100644 --- a/.gitignore +++ b/.gitignore @@ -21,3 +21,5 @@ yalc.lock # Perf tests .reassure + +example/ \ No newline at end of file diff --git a/lib/Onyx.ts b/lib/Onyx.ts index 91c44e91..14de5fb7 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -40,6 +40,7 @@ function init({ maxCachedKeysCount = 1000, shouldSyncMultipleInstances = Boolean(global.localStorage), debugSetState = false, + enablePerformanceMetrics = false, }: InitOptions): void { Storage.init(); @@ -65,6 +66,24 @@ function init({ Promise.all([OnyxUtils.addAllSafeEvictionKeysToRecentlyAccessedList(), OnyxUtils.initializeWithDefaultKeyStates()]).then(OnyxUtils.getDeferredInitTask().resolve); } +function applyDecorators() { + // We're requiring the script dynamically here so that it's only evaluated when decorators are used + // eslint-disable-next-line @typescript-eslint/no-var-requires + const decorate = require('./metrics'); + + // Re-assign with decorated functions + /* eslint-disable no-func-assign */ + connect = decorate(connect, 'Onyx:connect'); + set = decorate(set, 'Onyx:set'); + multiSet = decorate(multiSet, 'Onyx:multiSet'); + clear = decorate(clear, 'Onyx:clear'); + merge = decorate(merge, 'Onyx:merge'); + mergeCollection = decorate(mergeCollection, 'Onyx:mergeCollection'); + update = decorate(update, 'Onyx:update'); + clear = decorate(clear, 'Onyx:clear'); + /* eslint-enable */ +} + /** * Connects to an Onyx key given the options passed and listens to its changes. * diff --git a/lib/metrics.ts b/lib/metrics.ts new file mode 100644 index 00000000..341f0cd1 --- /dev/null +++ b/lib/metrics.ts @@ -0,0 +1,49 @@ +import performance from 'react-native-performance'; +import type {PerformanceMark} from 'react-native-performance'; + +const decoratedAliases = new Set(); + +/** + * Capture a measurement between the start mark and now + */ +function measureMarkToNow(startMark: PerformanceMark, detail: Record) { + performance.measure(`${startMark.name} [${startMark.detail.args.toString()}]`, { + start: startMark.startTime, + end: performance.now(), + detail: {...startMark.detail, ...detail}, + }); +} + +/** + * Wraps a function with metrics capturing logic + */ +function decorateWithMetrics>(func: (...args: Args) => ReturnType, alias = func.name) { + if (decoratedAliases.has(alias)) { + throw new Error(`"${alias}" is already decorated`); + } + + decoratedAliases.add(alias); + function decorated(...args: Args) { + const mark = performance.mark(alias, {detail: {args, alias}}); + + const originalPromise = func(...args); + + /* + * The handlers added here are not affecting the original promise + * They create a separate chain that's not exposed (returned) to the original caller + */ + originalPromise + .then((result) => { + measureMarkToNow(mark, {result}); + }) + .catch((error) => { + measureMarkToNow(mark, {error}); + }); + + return originalPromise; + } + + return decorated; +} + +export default decorateWithMetrics; diff --git a/lib/types.ts b/lib/types.ts index 3a6696ce..685a0fc3 100644 --- a/lib/types.ts +++ b/lib/types.ts @@ -462,6 +462,12 @@ type InitOptions = { /** Enables debugging setState() calls to connected components */ debugSetState?: boolean; + + /** + * If enabled it will use the performance API to measure the time taken by Onyx operations. + * @default false + */ + enablePerformanceMetrics?: boolean; }; // eslint-disable-next-line @typescript-eslint/no-explicit-any diff --git a/package.json b/package.json index 75a5503e..58d1ae51 100644 --- a/package.json +++ b/package.json @@ -25,8 +25,7 @@ "README.md", "LICENSE.md" ], - "main": "dist/index.js", - "types": "dist/index.d.ts", + "main": "lib/index.ts", "scripts": { "lint": "eslint .", "typecheck": "tsc --noEmit", From 3f2182778c5cfbacbf220902d7f92b2c3ce0aecb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hanno=20J=2E=20G=C3=B6decke?= Date: Wed, 20 Nov 2024 16:28:28 +0100 Subject: [PATCH 02/17] add decorator pattern also to onyx utils --- lib/GlobalSettings.ts | 34 +++++++++++++++++ lib/Onyx.ts | 46 +++++++++++++---------- lib/OnyxUtils.ts | 47 +++++++++++++++++++++++ lib/dependencies/ModuleProxy.ts | 39 +++++++++++++++++++ lib/dependencies/PerformanceProxy.ts | 13 +++++++ lib/metrics.ts | 56 ++++++++++++++++------------ 6 files changed, 193 insertions(+), 42 deletions(-) create mode 100644 lib/GlobalSettings.ts create mode 100644 lib/dependencies/ModuleProxy.ts create mode 100644 lib/dependencies/PerformanceProxy.ts diff --git a/lib/GlobalSettings.ts b/lib/GlobalSettings.ts new file mode 100644 index 00000000..f72ba14b --- /dev/null +++ b/lib/GlobalSettings.ts @@ -0,0 +1,34 @@ +/** + * Stores settings from Onyx.init globally so they can be made accessible by other parts of the library. + */ + +type GlobalSettings = { + enablePerformanceMetrics: boolean; +}; + +const globalSettings: GlobalSettings = { + enablePerformanceMetrics: false, +}; + +const listeners = new Set<(settings: GlobalSettings) => unknown>(); +function addGlobalSettingsChangeListener(listener: (settings: GlobalSettings) => unknown) { + listeners.add(listener); + return () => { + listeners.delete(listener); + }; +} + +function notifyListeners() { + listeners.forEach((listener) => listener(globalSettings)); +} + +function setPerformanceMetricsEnabled(enabled: boolean) { + globalSettings.enablePerformanceMetrics = enabled; + notifyListeners(); +} + +function isPerformanceMetricsEnabled() { + return globalSettings.enablePerformanceMetrics; +} + +export {setPerformanceMetricsEnabled, isPerformanceMetricsEnabled, addGlobalSettingsChangeListener}; diff --git a/lib/Onyx.ts b/lib/Onyx.ts index 14de5fb7..ff6a821b 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -31,6 +31,8 @@ import OnyxUtils from './OnyxUtils'; import logMessages from './logMessages'; import type {Connection} from './OnyxConnectionManager'; import connectionManager from './OnyxConnectionManager'; +import * as GlobalSettings from './GlobalSettings'; +import decorateWithMetrics from './metrics'; /** Initialize the store with actions and listening for storage events */ function init({ @@ -42,6 +44,11 @@ function init({ debugSetState = false, enablePerformanceMetrics = false, }: InitOptions): void { + if (enablePerformanceMetrics) { + GlobalSettings.setPerformanceMetricsEnabled(true); + applyDecorators(); + } + Storage.init(); if (shouldSyncMultipleInstances) { @@ -66,24 +73,6 @@ function init({ Promise.all([OnyxUtils.addAllSafeEvictionKeysToRecentlyAccessedList(), OnyxUtils.initializeWithDefaultKeyStates()]).then(OnyxUtils.getDeferredInitTask().resolve); } -function applyDecorators() { - // We're requiring the script dynamically here so that it's only evaluated when decorators are used - // eslint-disable-next-line @typescript-eslint/no-var-requires - const decorate = require('./metrics'); - - // Re-assign with decorated functions - /* eslint-disable no-func-assign */ - connect = decorate(connect, 'Onyx:connect'); - set = decorate(set, 'Onyx:set'); - multiSet = decorate(multiSet, 'Onyx:multiSet'); - clear = decorate(clear, 'Onyx:clear'); - merge = decorate(merge, 'Onyx:merge'); - mergeCollection = decorate(mergeCollection, 'Onyx:mergeCollection'); - update = decorate(update, 'Onyx:update'); - clear = decorate(clear, 'Onyx:clear'); - /* eslint-enable */ -} - /** * Connects to an Onyx key given the options passed and listens to its changes. * @@ -759,7 +748,26 @@ const Onyx = { clear, init, registerLogger: Logger.registerLogger, -} as const; +}; + +function applyDecorators() { + /* eslint-disable rulesdir/prefer-actions-set-data */ + // @ts-expect-error Reassign + connect = decorateWithMetrics(connect, 'Onyx:connect'); + // @ts-expect-error Reassign + set = decorateWithMetrics(set, 'Onyx:set'); + // @ts-expect-error Reassign + multiSet = decorateWithMetrics(multiSet, 'Onyx:multiSet'); + // @ts-expect-error Reassign + merge = decorateWithMetrics(merge, 'Onyx:merge'); + // @ts-expect-error Reassign + mergeCollection = decorateWithMetrics(mergeCollection, 'Onyx:mergeCollection'); + // @ts-expect-error Reassign + update = decorateWithMetrics(update, 'Onyx:update'); + // @ts-expect-error Reassign + clear = decorateWithMetrics(clear, 'Onyx:clear'); + /* eslint-enable rulesdir/prefer-actions-set-data */ +} export default Onyx; export type {OnyxUpdate, Mapping, ConnectOptions}; diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index de89a230..f94a25b0 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -32,6 +32,8 @@ import utils from './utils'; import type {WithOnyxState} from './withOnyx/types'; import type {DeferredTask} from './createDeferredTask'; import createDeferredTask from './createDeferredTask'; +import * as GlobalSettings from './GlobalSettings'; +import decorateWithMetrics from './metrics'; // Method constants const METHOD = { @@ -1417,4 +1419,49 @@ const OnyxUtils = { getEvictionBlocklist, }; +GlobalSettings.addGlobalSettingsChangeListener(({enablePerformanceMetrics}) => { + if (!enablePerformanceMetrics) { + return; + } + + // @ts-expect-error Reassign + initStoreValues = decorateWithMetrics(initStoreValues, 'OnyxUtils.initStoreValues'); + // @ts-expect-error Reassign + maybeFlushBatchUpdates = decorateWithMetrics(maybeFlushBatchUpdates, 'OnyxUtils.maybeFlushBatchUpdates'); + // @ts-expect-error Reassign + batchUpdates = decorateWithMetrics(batchUpdates, 'OnyxUtils.batchUpdates'); + // @ts-expect-error Complex type signature + get = decorateWithMetrics(get, 'OnyxUtils.get'); + // @ts-expect-error Reassign + getAllKeys = decorateWithMetrics(getAllKeys, 'OnyxUtils.getAllKeys'); + // @ts-expect-error Reassign + getCollectionKeys = decorateWithMetrics(getCollectionKeys, 'OnyxUtils.getCollectionKeys'); + // @ts-expect-error Reassign + addAllSafeEvictionKeysToRecentlyAccessedList = decorateWithMetrics(addAllSafeEvictionKeysToRecentlyAccessedList, 'OnyxUtils.addAllSafeEvictionKeysToRecentlyAccessedList'); + // @ts-expect-error Reassign + keysChanged = decorateWithMetrics(keysChanged, 'OnyxUtils.keysChanged'); + // @ts-expect-error Reassign + keyChanged = decorateWithMetrics(keyChanged, 'OnyxUtils.keyChanged'); + // @ts-expect-error Reassign + sendDataToConnection = decorateWithMetrics(sendDataToConnection, 'OnyxUtils.sendDataToConnection'); + // @ts-expect-error Reassign + scheduleSubscriberUpdate = decorateWithMetrics(scheduleSubscriberUpdate, 'OnyxUtils.scheduleSubscriberUpdate'); + // @ts-expect-error Reassign + scheduleNotifyCollectionSubscribers = decorateWithMetrics(scheduleNotifyCollectionSubscribers, 'OnyxUtils.scheduleNotifyCollectionSubscribers'); + // @ts-expect-error Reassign + remove = decorateWithMetrics(remove, 'OnyxUtils.remove'); + // @ts-expect-error Reassign + reportStorageQuota = decorateWithMetrics(reportStorageQuota, 'OnyxUtils.reportStorageQuota'); + // @ts-expect-error Complex type signature + evictStorageAndRetry = decorateWithMetrics(evictStorageAndRetry, 'OnyxUtils.evictStorageAndRetry'); + // @ts-expect-error Reassign + broadcastUpdate = decorateWithMetrics(broadcastUpdate, 'OnyxUtils.broadcastUpdate'); + // @ts-expect-error Reassign + initializeWithDefaultKeyStates = decorateWithMetrics(initializeWithDefaultKeyStates, 'OnyxUtils.initializeWithDefaultKeyStates'); + // @ts-expect-error Complex type signature + multiGet = decorateWithMetrics(multiGet, 'OnyxUtils.multiGet'); + // @ts-expect-error Reassign + subscribeToKey = decorateWithMetrics(subscribeToKey, 'OnyxUtils.subscribeToKey'); +}); + export default OnyxUtils; diff --git a/lib/dependencies/ModuleProxy.ts b/lib/dependencies/ModuleProxy.ts new file mode 100644 index 00000000..597f38a8 --- /dev/null +++ b/lib/dependencies/ModuleProxy.ts @@ -0,0 +1,39 @@ +type ImportType = ReturnType; + +/** + * Create a lazily-imported module proxy. + * This is useful for lazily requiring optional dependencies. + */ +const createModuleProxy = (getModule: () => ImportType): TModule => { + const holder: {module: TModule | undefined} = {module: undefined}; + + const proxy = new Proxy(holder, { + get: (target, property) => { + if (property === '$$typeof') { + // If inlineRequires is enabled, Metro will look up all imports + // with the $$typeof operator. In this case, this will throw the + // `OptionalDependencyNotInstalledError` error because we try to access the module + // even though we are not using it (Metro does it), so instead we return undefined + // to bail out of inlineRequires here. + return undefined; + } + + if (target.module == null) { + // lazy initialize module via require() + // caller needs to make sure the require() call is wrapped in a try/catch + // eslint-disable-next-line no-param-reassign + target.module = getModule() as TModule; + } + return target.module[property as keyof typeof holder.module]; + }, + }); + return proxy as unknown as TModule; +}; + +class OptionalDependencyNotInstalledError extends Error { + constructor(name: string) { + super(`${name} is not installed!`); + } +} + +export {createModuleProxy, OptionalDependencyNotInstalledError}; diff --git a/lib/dependencies/PerformanceProxy.ts b/lib/dependencies/PerformanceProxy.ts new file mode 100644 index 00000000..1f15458f --- /dev/null +++ b/lib/dependencies/PerformanceProxy.ts @@ -0,0 +1,13 @@ +import type performance from 'react-native-performance'; +import {createModuleProxy, OptionalDependencyNotInstalledError} from './ModuleProxy'; + +const PerformanceProxy = createModuleProxy(() => { + try { + // eslint-disable-next-line @typescript-eslint/no-var-requires + return require('react-native-performance').default; + } catch { + throw new OptionalDependencyNotInstalledError('react-native-performance'); + } +}); + +export default PerformanceProxy; diff --git a/lib/metrics.ts b/lib/metrics.ts index 341f0cd1..f6cb3349 100644 --- a/lib/metrics.ts +++ b/lib/metrics.ts @@ -1,47 +1,57 @@ -import performance from 'react-native-performance'; -import type {PerformanceMark} from 'react-native-performance'; +import type * as performance from 'react-native-performance'; +import PerformanceProxy from './dependencies/PerformanceProxy'; const decoratedAliases = new Set(); /** * Capture a measurement between the start mark and now */ -function measureMarkToNow(startMark: PerformanceMark, detail: Record) { - performance.measure(`${startMark.name} [${startMark.detail.args.toString()}]`, { +function measureMarkToNow(startMark: performance.PerformanceMark, detail: Record) { + PerformanceProxy.measure(`${startMark.name} [${startMark.detail.args.toString()}]`, { start: startMark.startTime, - end: performance.now(), + end: PerformanceProxy.now(), detail: {...startMark.detail, ...detail}, }); } +function isPromiseLike(value: unknown): value is Promise { + return value != null && typeof value === 'object' && 'then' in value; +} + /** * Wraps a function with metrics capturing logic */ -function decorateWithMetrics>(func: (...args: Args) => ReturnType, alias = func.name) { +function decorateWithMetrics(func: (...args: Args) => ReturnType, alias = func.name) { if (decoratedAliases.has(alias)) { throw new Error(`"${alias}" is already decorated`); } decoratedAliases.add(alias); function decorated(...args: Args) { - const mark = performance.mark(alias, {detail: {args, alias}}); - - const originalPromise = func(...args); - - /* - * The handlers added here are not affecting the original promise - * They create a separate chain that's not exposed (returned) to the original caller - */ - originalPromise - .then((result) => { - measureMarkToNow(mark, {result}); - }) - .catch((error) => { - measureMarkToNow(mark, {error}); - }); - - return originalPromise; + const mark = PerformanceProxy.mark(alias, {detail: {args, alias}}); + + const originalReturnValue = func(...args); + + if (isPromiseLike(originalReturnValue)) { + /* + * The handlers added here are not affecting the original promise + * They create a separate chain that's not exposed (returned) to the original caller + */ + originalReturnValue + .then((result) => { + measureMarkToNow(mark, {result}); + }) + .catch((error) => { + measureMarkToNow(mark, {error}); + }); + + return originalReturnValue; + } + + measureMarkToNow(mark, {result: originalReturnValue}); + return originalReturnValue; } + decorated.name = `${alias}_DECORATED`; return decorated; } From 65c157170091efac41f6d8b7327680c6e503c668 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hanno=20J=2E=20G=C3=B6decke?= Date: Wed, 20 Nov 2024 16:36:22 +0100 Subject: [PATCH 03/17] decorate storage as well --- lib/Onyx.ts | 15 ++++++++------- lib/OnyxUtils.ts | 1 + lib/storage/index.ts | 24 ++++++++++++++++++++++-- 3 files changed, 31 insertions(+), 9 deletions(-) diff --git a/lib/Onyx.ts b/lib/Onyx.ts index ff6a821b..5b3dbd0e 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -751,21 +751,22 @@ const Onyx = { }; function applyDecorators() { + // We are reassigning the functions directly so that internal function calls are also decorated /* eslint-disable rulesdir/prefer-actions-set-data */ // @ts-expect-error Reassign - connect = decorateWithMetrics(connect, 'Onyx:connect'); + connect = decorateWithMetrics(connect, 'Onyx.connect'); // @ts-expect-error Reassign - set = decorateWithMetrics(set, 'Onyx:set'); + set = decorateWithMetrics(set, 'Onyx.set'); // @ts-expect-error Reassign - multiSet = decorateWithMetrics(multiSet, 'Onyx:multiSet'); + multiSet = decorateWithMetrics(multiSet, 'Onyx.multiSet'); // @ts-expect-error Reassign - merge = decorateWithMetrics(merge, 'Onyx:merge'); + merge = decorateWithMetrics(merge, 'Onyx.merge'); // @ts-expect-error Reassign - mergeCollection = decorateWithMetrics(mergeCollection, 'Onyx:mergeCollection'); + mergeCollection = decorateWithMetrics(mergeCollection, 'Onyx.mergeCollection'); // @ts-expect-error Reassign - update = decorateWithMetrics(update, 'Onyx:update'); + update = decorateWithMetrics(update, 'Onyx.update'); // @ts-expect-error Reassign - clear = decorateWithMetrics(clear, 'Onyx:clear'); + clear = decorateWithMetrics(clear, 'Onyx.clear'); /* eslint-enable rulesdir/prefer-actions-set-data */ } diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index f94a25b0..891c9065 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -1423,6 +1423,7 @@ GlobalSettings.addGlobalSettingsChangeListener(({enablePerformanceMetrics}) => { if (!enablePerformanceMetrics) { return; } + // We are reassigning the functions directly so that internal function calls are also decorated // @ts-expect-error Reassign initStoreValues = decorateWithMetrics(initStoreValues, 'OnyxUtils.initStoreValues'); diff --git a/lib/storage/index.ts b/lib/storage/index.ts index c9b797b1..938b615a 100644 --- a/lib/storage/index.ts +++ b/lib/storage/index.ts @@ -4,6 +4,8 @@ import PlatformStorage from './platforms'; import InstanceSync from './InstanceSync'; import MemoryOnlyProvider from './providers/MemoryOnlyProvider'; import type StorageProvider from './providers/types'; +import * as GlobalSettings from '../GlobalSettings'; +import decorateWithMetrics from '../metrics'; let provider = PlatformStorage; let shouldKeepInstancesSync = false; @@ -55,7 +57,7 @@ function tryOrDegradePerformance(fn: () => Promise | T, waitForInitializat }); } -const Storage: Storage = { +const storage: Storage = { /** * Returns the storage provider currently in use */ @@ -202,4 +204,22 @@ const Storage: Storage = { }, }; -export default Storage; +GlobalSettings.addGlobalSettingsChangeListener(({enablePerformanceMetrics}) => { + if (!enablePerformanceMetrics) { + return; + } + + // Apply decorators + storage.getItem = decorateWithMetrics(storage.getItem, 'Storage.getItem'); + storage.multiGet = decorateWithMetrics(storage.multiGet, 'Storage.multiGet'); + storage.setItem = decorateWithMetrics(storage.setItem, 'Storage.setItem'); + storage.multiSet = decorateWithMetrics(storage.multiSet, 'Storage.multiSet'); + storage.mergeItem = decorateWithMetrics(storage.mergeItem, 'Storage.mergeItem'); + storage.multiMerge = decorateWithMetrics(storage.multiMerge, 'Storage.multiMerge'); + storage.removeItem = decorateWithMetrics(storage.removeItem, 'Storage.removeItem'); + storage.removeItems = decorateWithMetrics(storage.removeItems, 'Storage.removeItems'); + storage.clear = decorateWithMetrics(storage.clear, 'Storage.clear'); + storage.getAllKeys = decorateWithMetrics(storage.getAllKeys, 'Storage.getAllKeys'); +}); + +export default storage; From 2578654d9844a065a15f95e11733e0643191a5a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hanno=20J=2E=20G=C3=B6decke?= Date: Wed, 20 Nov 2024 16:39:13 +0100 Subject: [PATCH 04/17] remove example --- .gitignore | 2 -- 1 file changed, 2 deletions(-) diff --git a/.gitignore b/.gitignore index 5592c6d4..a46d06d8 100644 --- a/.gitignore +++ b/.gitignore @@ -21,5 +21,3 @@ yalc.lock # Perf tests .reassure - -example/ \ No newline at end of file From 0494c31ba653c8dbb582657d53103bc70a49e7a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hanno=20J=2E=20G=C3=B6decke?= Date: Wed, 20 Nov 2024 16:50:13 +0100 Subject: [PATCH 05/17] provide web impl --- .../{PerformanceProxy.ts => PerformanceProxy/index.native.ts} | 2 +- lib/dependencies/PerformanceProxy/index.ts | 2 ++ lib/metrics.ts | 3 +-- 3 files changed, 4 insertions(+), 3 deletions(-) rename lib/dependencies/{PerformanceProxy.ts => PerformanceProxy/index.native.ts} (95%) create mode 100644 lib/dependencies/PerformanceProxy/index.ts diff --git a/lib/dependencies/PerformanceProxy.ts b/lib/dependencies/PerformanceProxy/index.native.ts similarity index 95% rename from lib/dependencies/PerformanceProxy.ts rename to lib/dependencies/PerformanceProxy/index.native.ts index 1f15458f..da35d419 100644 --- a/lib/dependencies/PerformanceProxy.ts +++ b/lib/dependencies/PerformanceProxy/index.native.ts @@ -1,5 +1,5 @@ import type performance from 'react-native-performance'; -import {createModuleProxy, OptionalDependencyNotInstalledError} from './ModuleProxy'; +import {createModuleProxy, OptionalDependencyNotInstalledError} from '../ModuleProxy'; const PerformanceProxy = createModuleProxy(() => { try { diff --git a/lib/dependencies/PerformanceProxy/index.ts b/lib/dependencies/PerformanceProxy/index.ts new file mode 100644 index 00000000..3aff0ad4 --- /dev/null +++ b/lib/dependencies/PerformanceProxy/index.ts @@ -0,0 +1,2 @@ +// Use the existing performance API on web +export default performance; diff --git a/lib/metrics.ts b/lib/metrics.ts index f6cb3349..301a5467 100644 --- a/lib/metrics.ts +++ b/lib/metrics.ts @@ -1,4 +1,3 @@ -import type * as performance from 'react-native-performance'; import PerformanceProxy from './dependencies/PerformanceProxy'; const decoratedAliases = new Set(); @@ -6,7 +5,7 @@ const decoratedAliases = new Set(); /** * Capture a measurement between the start mark and now */ -function measureMarkToNow(startMark: performance.PerformanceMark, detail: Record) { +function measureMarkToNow(startMark: PerformanceMark, detail: Record) { PerformanceProxy.measure(`${startMark.name} [${startMark.detail.args.toString()}]`, { start: startMark.startTime, end: PerformanceProxy.now(), From 5d16bb5d2dfe7653f46cff7ca3eb3dc70d7d9594 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Mon, 25 Nov 2024 16:02:55 +0000 Subject: [PATCH 06/17] useEffect approach for dependency list in useOnyx --- lib/useOnyx.ts | 28 +++++++++++++++++++++- tests/unit/useOnyxTest.ts | 49 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+), 1 deletion(-) diff --git a/lib/useOnyx.ts b/lib/useOnyx.ts index fa460dd7..dbd72633 100644 --- a/lib/useOnyx.ts +++ b/lib/useOnyx.ts @@ -1,5 +1,6 @@ import {deepEqual, shallowEqual} from 'fast-equals'; import {useCallback, useEffect, useRef, useSyncExternalStore} from 'react'; +import type {DependencyList} from 'react'; import OnyxCache, {TASK} from './OnyxCache'; import type {Connection} from './OnyxConnectionManager'; import connectionManager from './OnyxConnectionManager'; @@ -104,12 +105,18 @@ function getCachedValue(key: TKey, selector?: UseO function useOnyx>( key: TKey, options?: BaseUseOnyxOptions & UseOnyxInitialValueOption & Required>, + dependencies?: DependencyList, ): UseOnyxResult; function useOnyx>( key: TKey, options?: BaseUseOnyxOptions & UseOnyxInitialValueOption>, + dependencies?: DependencyList, ): UseOnyxResult; -function useOnyx>(key: TKey, options?: UseOnyxOptions): UseOnyxResult { +function useOnyx>( + key: TKey, + options?: UseOnyxOptions, + dependencies: DependencyList = [], +): UseOnyxResult { const connectionRef = useRef(null); const previousKey = usePrevious(key); @@ -137,6 +144,10 @@ function useOnyx>(key: TKey // in `getSnapshot()` to be satisfied several times. const isFirstConnectionRef = useRef(true); + const isConnectingRef = useRef(false); + + const onStoreChangeFnRef = useRef<(() => void) | null>(null); + // Indicates if we should get the newest cached value from Onyx during `getSnapshot()` execution. const shouldGetCachedValueRef = useRef(true); @@ -168,6 +179,14 @@ function useOnyx>(key: TKey ); }, [previousKey, key]); + // eslint-disable-next-line rulesdir/prefer-early-return + useEffect(() => { + if (connectionRef.current !== null && !isConnectingRef.current && onStoreChangeFnRef.current) { + shouldGetCachedValueRef.current = true; + onStoreChangeFnRef.current(); + } + }, [...dependencies]); + // Mimics withOnyx's checkEvictableKeys() behavior. const checkEvictableKey = useCallback(() => { if (options?.canEvict === undefined || !connectionRef.current) { @@ -255,9 +274,14 @@ function useOnyx>(key: TKey const subscribe = useCallback( (onStoreChange: () => void) => { + isConnectingRef.current = true; + onStoreChangeFnRef.current = onStoreChange; + connectionRef.current = connectionManager.connect({ key, callback: () => { + isConnectingRef.current = false; + // Signals that the first connection was made, so some logics in `getSnapshot()` // won't be executed anymore. isFirstConnectionRef.current = false; @@ -281,6 +305,8 @@ function useOnyx>(key: TKey } connectionManager.disconnect(connectionRef.current); + onStoreChangeFnRef.current = null; + isConnectingRef.current = false; isFirstConnectionRef.current = false; }; }, diff --git a/tests/unit/useOnyxTest.ts b/tests/unit/useOnyxTest.ts index 6e74ee4a..ebc975b8 100644 --- a/tests/unit/useOnyxTest.ts +++ b/tests/unit/useOnyxTest.ts @@ -610,6 +610,55 @@ describe('useOnyx', () => { }); }); + describe('dependencies', () => { + it('should 1', async () => { + Onyx.mergeCollection(ONYXKEYS.COLLECTION.TEST_KEY, { + [`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`]: {id: 'entry1_id', name: 'entry1_name'}, + [`${ONYXKEYS.COLLECTION.TEST_KEY}entry2`]: {id: 'entry2_id', name: 'entry2_name'}, + [`${ONYXKEYS.COLLECTION.TEST_KEY}entry3`]: {id: 'entry3_id', name: 'entry3_name'}, + } as GenericCollection); + + let externalValue = 'ex1'; + + const {result, rerender} = renderHook(() => + useOnyx( + ONYXKEYS.COLLECTION.TEST_KEY, + { + // @ts-expect-error bypass + selector: (entries: OnyxCollection<{id: string; name: string}>) => + Object.entries(entries ?? {}).reduce>>((acc, [key, value]) => { + acc[key] = `${value?.id}_${externalValue}`; + return acc; + }, {}), + }, + [externalValue], + ), + ); + + await act(async () => waitForPromisesToResolve()); + + expect(result.current[0]).toEqual({ + [`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`]: 'entry1_id_ex1', + [`${ONYXKEYS.COLLECTION.TEST_KEY}entry2`]: 'entry2_id_ex1', + [`${ONYXKEYS.COLLECTION.TEST_KEY}entry3`]: 'entry3_id_ex1', + }); + expect(result.current[1].status).toEqual('loaded'); + + externalValue = 'ex2'; + + await act(async () => { + rerender(undefined); + }); + + expect(result.current[0]).toEqual({ + [`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`]: 'entry1_id_ex2', + [`${ONYXKEYS.COLLECTION.TEST_KEY}entry2`]: 'entry2_id_ex2', + [`${ONYXKEYS.COLLECTION.TEST_KEY}entry3`]: 'entry3_id_ex2', + }); + expect(result.current[1].status).toEqual('loaded'); + }); + }); + // 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 42055a00e5666ac01cb377becfbda33fd270a38e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hanno=20J=2E=20G=C3=B6decke?= Date: Wed, 27 Nov 2024 21:13:17 +0100 Subject: [PATCH 07/17] use typeof --- lib/GlobalSettings.ts | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/lib/GlobalSettings.ts b/lib/GlobalSettings.ts index f72ba14b..68617b7e 100644 --- a/lib/GlobalSettings.ts +++ b/lib/GlobalSettings.ts @@ -2,14 +2,12 @@ * Stores settings from Onyx.init globally so they can be made accessible by other parts of the library. */ -type GlobalSettings = { - enablePerformanceMetrics: boolean; -}; - -const globalSettings: GlobalSettings = { +const globalSettings = { enablePerformanceMetrics: false, }; +type GlobalSettings = typeof globalSettings; + const listeners = new Set<(settings: GlobalSettings) => unknown>(); function addGlobalSettingsChangeListener(listener: (settings: GlobalSettings) => unknown) { listeners.add(listener); From 0361bff9932c7ca1e6d267084ffb2581426fc9be Mon Sep 17 00:00:00 2001 From: "os-botify[bot]" <140437396+os-botify[bot]@users.noreply.github.com> Date: Wed, 4 Dec 2024 02:03:14 +0000 Subject: [PATCH 08/17] Update package-lock.json version to 2.0.83 --- package-lock.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/package-lock.json b/package-lock.json index 8aacd95c..a7eb5e51 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "react-native-onyx", - "version": "2.0.82", + "version": "2.0.83", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "react-native-onyx", - "version": "2.0.82", + "version": "2.0.83", "license": "MIT", "dependencies": { "ascii-table": "0.0.9", From a5e9abb14ea0715e411918666247bd459bd0df8e Mon Sep 17 00:00:00 2001 From: "os-botify[bot]" <140437396+os-botify[bot]@users.noreply.github.com> Date: Wed, 4 Dec 2024 02:03:15 +0000 Subject: [PATCH 09/17] Update package.json version to 2.0.83 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 774d27f3..876d5541 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "react-native-onyx", - "version": "2.0.82", + "version": "2.0.83", "author": "Expensify, Inc.", "homepage": "https://expensify.com", "description": "State management for React Native", From 7c02929f55055a305e06b131ca8cb51a50d13577 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Wed, 4 Dec 2024 22:31:11 +0000 Subject: [PATCH 10/17] Add comments --- lib/useOnyx.ts | 15 +++++++++++---- tests/unit/useOnyxTest.ts | 2 +- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/lib/useOnyx.ts b/lib/useOnyx.ts index dbd72633..cad6f53d 100644 --- a/lib/useOnyx.ts +++ b/lib/useOnyx.ts @@ -144,8 +144,10 @@ function useOnyx>( // in `getSnapshot()` to be satisfied several times. const isFirstConnectionRef = useRef(true); + // Indicates if the hook is connecting to a Onyx key. const isConnectingRef = useRef(false); + // Stores the `onStoreChange` function, which can be used to trigger a `getSnapshot` update when desired. const onStoreChangeFnRef = useRef<(() => void) | null>(null); // Indicates if we should get the newest cached value from Onyx during `getSnapshot()` execution. @@ -179,12 +181,17 @@ function useOnyx>( ); }, [previousKey, key]); - // eslint-disable-next-line rulesdir/prefer-early-return useEffect(() => { - if (connectionRef.current !== null && !isConnectingRef.current && onStoreChangeFnRef.current) { - shouldGetCachedValueRef.current = true; - onStoreChangeFnRef.current(); + // This effect will only run if the `dependencies` array changes. If it changes it will force the hook + // to trigger a `getSnapshot` update by calling the stored `onStoreChange` function reference, thus + // re-running the hook and returning the latest value to the consumer. + if (connectionRef.current === null || isConnectingRef.current || !onStoreChangeFnRef.current) { + return; } + + shouldGetCachedValueRef.current = true; + onStoreChangeFnRef.current(); + // eslint-disable-next-line react-hooks/exhaustive-deps }, [...dependencies]); // Mimics withOnyx's checkEvictableKeys() behavior. diff --git a/tests/unit/useOnyxTest.ts b/tests/unit/useOnyxTest.ts index ebc975b8..2c874469 100644 --- a/tests/unit/useOnyxTest.ts +++ b/tests/unit/useOnyxTest.ts @@ -611,7 +611,7 @@ describe('useOnyx', () => { }); describe('dependencies', () => { - it('should 1', async () => { + it('should return the updated selected value when a external value passed to the dependencies list changes', async () => { Onyx.mergeCollection(ONYXKEYS.COLLECTION.TEST_KEY, { [`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`]: {id: 'entry1_id', name: 'entry1_name'}, [`${ONYXKEYS.COLLECTION.TEST_KEY}entry2`]: {id: 'entry2_id', name: 'entry2_name'}, From 6217ef91081cb8afc1d5e239cda0f4f8fb48b4f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Thu, 5 Dec 2024 17:16:11 +0000 Subject: [PATCH 11/17] Minor adjustments --- lib/useOnyx.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/lib/useOnyx.ts b/lib/useOnyx.ts index cad6f53d..15ccc06c 100644 --- a/lib/useOnyx.ts +++ b/lib/useOnyx.ts @@ -147,7 +147,7 @@ function useOnyx>( // Indicates if the hook is connecting to a Onyx key. const isConnectingRef = useRef(false); - // Stores the `onStoreChange` function, which can be used to trigger a `getSnapshot` update when desired. + // Stores the `onStoreChange()` function, which can be used to trigger a `getSnapshot()` update when desired. const onStoreChangeFnRef = useRef<(() => void) | null>(null); // Indicates if we should get the newest cached value from Onyx during `getSnapshot()` execution. @@ -183,7 +183,7 @@ function useOnyx>( useEffect(() => { // This effect will only run if the `dependencies` array changes. If it changes it will force the hook - // to trigger a `getSnapshot` update by calling the stored `onStoreChange` function reference, thus + // to trigger a `getSnapshot()` update by calling the stored `onStoreChange()` function reference, thus // re-running the hook and returning the latest value to the consumer. if (connectionRef.current === null || isConnectingRef.current || !onStoreChangeFnRef.current) { return; @@ -288,6 +288,7 @@ function useOnyx>( key, callback: () => { isConnectingRef.current = false; + onStoreChangeFnRef.current = onStoreChange; // Signals that the first connection was made, so some logics in `getSnapshot()` // won't be executed anymore. @@ -312,9 +313,9 @@ function useOnyx>( } connectionManager.disconnect(connectionRef.current); - onStoreChangeFnRef.current = null; - isConnectingRef.current = false; isFirstConnectionRef.current = false; + isConnectingRef.current = false; + onStoreChangeFnRef.current = null; }; }, [key, options?.initWithStoredValues, options?.reuseConnection, checkEvictableKey], From 889d36e313bf9700689391e4d6eac273643e6994 Mon Sep 17 00:00:00 2001 From: "os-botify[bot]" <140437396+os-botify[bot]@users.noreply.github.com> Date: Fri, 6 Dec 2024 14:39:57 +0000 Subject: [PATCH 12/17] Update package-lock.json version to 2.0.84 --- package-lock.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/package-lock.json b/package-lock.json index a7eb5e51..1f4e43e3 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "react-native-onyx", - "version": "2.0.83", + "version": "2.0.84", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "react-native-onyx", - "version": "2.0.83", + "version": "2.0.84", "license": "MIT", "dependencies": { "ascii-table": "0.0.9", From 5f4bf772aadb581bbe87d983a56d933e4cd9b55d Mon Sep 17 00:00:00 2001 From: "os-botify[bot]" <140437396+os-botify[bot]@users.noreply.github.com> Date: Fri, 6 Dec 2024 14:39:58 +0000 Subject: [PATCH 13/17] Update package.json version to 2.0.84 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 876d5541..ec00fe85 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "react-native-onyx", - "version": "2.0.83", + "version": "2.0.84", "author": "Expensify, Inc.", "homepage": "https://expensify.com", "description": "State management for React Native", From e0b96b9aa19d20fafe11362cf05c3515f6753dda Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Mon, 9 Dec 2024 10:49:06 +0100 Subject: [PATCH 14/17] Restore main entry point to use dist files --- package.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/package.json b/package.json index ec00fe85..91d83bfe 100644 --- a/package.json +++ b/package.json @@ -25,7 +25,8 @@ "README.md", "LICENSE.md" ], - "main": "lib/index.ts", + "main": "dist/index.js", + "types": "dist/index.d.ts", "scripts": { "lint": "eslint .", "typecheck": "tsc --noEmit", From ca469d96f3f4a19516858b6bd38cd4888e6fd33c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Mon, 9 Dec 2024 10:49:20 +0100 Subject: [PATCH 15/17] Fix typo --- lib/useOnyx.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/useOnyx.ts b/lib/useOnyx.ts index 15ccc06c..701e3ac4 100644 --- a/lib/useOnyx.ts +++ b/lib/useOnyx.ts @@ -144,7 +144,7 @@ function useOnyx>( // in `getSnapshot()` to be satisfied several times. const isFirstConnectionRef = useRef(true); - // Indicates if the hook is connecting to a Onyx key. + // Indicates if the hook is connecting to an Onyx key. const isConnectingRef = useRef(false); // Stores the `onStoreChange()` function, which can be used to trigger a `getSnapshot()` update when desired. From 84ac380994f06bf49cfcdcac27a6bc2ab349b43d Mon Sep 17 00:00:00 2001 From: "os-botify[bot]" <140437396+os-botify[bot]@users.noreply.github.com> Date: Mon, 9 Dec 2024 11:22:26 +0000 Subject: [PATCH 16/17] Update package-lock.json version to 2.0.85 --- package-lock.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/package-lock.json b/package-lock.json index 1f4e43e3..1891513e 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "react-native-onyx", - "version": "2.0.84", + "version": "2.0.85", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "react-native-onyx", - "version": "2.0.84", + "version": "2.0.85", "license": "MIT", "dependencies": { "ascii-table": "0.0.9", From c25f9a07cb62b04c9e6c9ae8f6a564230d3c5a85 Mon Sep 17 00:00:00 2001 From: "os-botify[bot]" <140437396+os-botify[bot]@users.noreply.github.com> Date: Mon, 9 Dec 2024 11:22:28 +0000 Subject: [PATCH 17/17] Update package.json version to 2.0.85 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 91d83bfe..3fe00a8e 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "react-native-onyx", - "version": "2.0.84", + "version": "2.0.85", "author": "Expensify, Inc.", "homepage": "https://expensify.com", "description": "State management for React Native",