From bf88fc68b8db9d343f97eea104e8cd62a207a067 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Fri, 28 Nov 2025 14:57:04 +0000 Subject: [PATCH 01/13] Setup fake-indexeddb library --- jest-test-environment.ts | 11 +++++++++++ jest.config.js | 3 ++- package-lock.json | 11 +++++++++++ package.json | 1 + 4 files changed, 25 insertions(+), 1 deletion(-) create mode 100644 jest-test-environment.ts diff --git a/jest-test-environment.ts b/jest-test-environment.ts new file mode 100644 index 00000000..aa9914a7 --- /dev/null +++ b/jest-test-environment.ts @@ -0,0 +1,11 @@ +import JSDOMEnvironment from 'jest-environment-jsdom'; + +// We need this custom JSDOM environment implementation in order +// to support `structuredClone` in Jest, that is used by `fake-indexeddb` library. +// Reference: https://github.com/jsdom/jsdom/issues/3363#issuecomment-1467894943 +export default class FixJSDOMEnvironment extends JSDOMEnvironment { + constructor(...args: ConstructorParameters) { + super(...args); + this.global.structuredClone = structuredClone; + } +} diff --git a/jest.config.js b/jest.config.js index 947c4cab..97ff64dc 100644 --- a/jest.config.js +++ b/jest.config.js @@ -10,7 +10,8 @@ module.exports = { __DEV__: true, WebSocket: {}, }, - testEnvironment: 'jsdom', + testEnvironment: './jest-test-environment.ts', + setupFiles: ['fake-indexeddb/auto'], setupFilesAfterEnv: ['./jestSetup.js'], testTimeout: 60000, transformIgnorePatterns: ['node_modules/(?!((@)?react-native|@ngneat/falso|uuid)/)'], diff --git a/package-lock.json b/package-lock.json index 2e5f8853..e33c29f5 100644 --- a/package-lock.json +++ b/package-lock.json @@ -44,6 +44,7 @@ "eslint-plugin-import": "^2.29.1", "eslint-plugin-jsx-a11y": "^6.8.0", "eslint-plugin-react": "^7.31.10", + "fake-indexeddb": "^6.2.5", "idb-keyval": "^6.2.1", "jest": "^29.7.0", "jest-cli": "^29.7.0", @@ -7968,6 +7969,16 @@ "dev": true, "license": "Apache-2.0" }, + "node_modules/fake-indexeddb": { + "version": "6.2.5", + "resolved": "https://registry.npmjs.org/fake-indexeddb/-/fake-indexeddb-6.2.5.tgz", + "integrity": "sha512-CGnyrvbhPlWYMngksqrSSUT1BAVP49dZocrHuK0SvtR0D5TMs5wP0o3j7jexDJW01KSadjBp1M/71o/KR3nD1w==", + "dev": true, + "license": "Apache-2.0", + "engines": { + "node": ">=18" + } + }, "node_modules/fast-deep-equal": { "version": "3.1.3", "resolved": "https://registry.npmjs.org/fast-deep-equal/-/fast-deep-equal-3.1.3.tgz", diff --git a/package.json b/package.json index 523d5e2a..940ed0fd 100644 --- a/package.json +++ b/package.json @@ -78,6 +78,7 @@ "eslint-plugin-import": "^2.29.1", "eslint-plugin-jsx-a11y": "^6.8.0", "eslint-plugin-react": "^7.31.10", + "fake-indexeddb": "^6.2.5", "idb-keyval": "^6.2.1", "jest": "^29.7.0", "jest-cli": "^29.7.0", From e2471d9bc8e946ac848de562a936dd0de6ab02d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Fri, 28 Nov 2025 22:22:25 +0000 Subject: [PATCH 02/13] Add store reference to provider and refactor usage inside --- lib/storage/InstanceSync/index.web.ts | 2 +- lib/storage/__mocks__/index.ts | 4 +- lib/storage/index.ts | 6 +- .../providers/IDBKeyValProvider/index.ts | 155 +++++++++++++----- lib/storage/providers/MemoryOnlyProvider.ts | 35 ++-- lib/storage/providers/NoopProvider.ts | 4 +- lib/storage/providers/SQLiteProvider.ts | 88 +++++++--- lib/storage/providers/types.ts | 6 +- 8 files changed, 211 insertions(+), 89 deletions(-) diff --git a/lib/storage/InstanceSync/index.web.ts b/lib/storage/InstanceSync/index.web.ts index 99a7fe32..4a9d15f5 100644 --- a/lib/storage/InstanceSync/index.web.ts +++ b/lib/storage/InstanceSync/index.web.ts @@ -32,7 +32,7 @@ const InstanceSync = { /** * @param {Function} onStorageKeyChanged Storage synchronization mechanism keeping all opened tabs in sync */ - init: (onStorageKeyChanged: OnStorageKeyChanged, store: StorageProvider) => { + init: (onStorageKeyChanged: OnStorageKeyChanged, store: StorageProvider) => { storage = store; // This listener will only be triggered by events coming from other tabs diff --git a/lib/storage/__mocks__/index.ts b/lib/storage/__mocks__/index.ts index 3a15fecf..b4fcc31b 100644 --- a/lib/storage/__mocks__/index.ts +++ b/lib/storage/__mocks__/index.ts @@ -1,4 +1,4 @@ -import MemoryOnlyProvider, {mockStore, mockSet, setMockStore} from '../providers/MemoryOnlyProvider'; +import MemoryOnlyProvider, {mockStore, setMockStore} from '../providers/MemoryOnlyProvider'; const init = jest.fn(MemoryOnlyProvider.init); @@ -18,7 +18,7 @@ const StorageMock = { getAllKeys: jest.fn(MemoryOnlyProvider.getAllKeys), getDatabaseSize: jest.fn(MemoryOnlyProvider.getDatabaseSize), keepInstancesSync: jest.fn(), - mockSet, + getMockStore: jest.fn(() => mockStore), setMockStore: jest.fn((data) => setMockStore(data)), }; diff --git a/lib/storage/index.ts b/lib/storage/index.ts index 97ec7cee..07e7f753 100644 --- a/lib/storage/index.ts +++ b/lib/storage/index.ts @@ -7,7 +7,7 @@ import type StorageProvider from './providers/types'; import * as GlobalSettings from '../GlobalSettings'; import decorateWithMetrics from '../metrics'; -let provider = PlatformStorage; +let provider = PlatformStorage as StorageProvider; let shouldKeepInstancesSync = false; let finishInitalization: (value?: unknown) => void; const initPromise = new Promise((resolve) => { @@ -15,8 +15,8 @@ const initPromise = new Promise((resolve) => { }); type Storage = { - getStorageProvider: () => StorageProvider; -} & Omit; + getStorageProvider: () => StorageProvider; +} & Omit, 'name' | 'store'>; /** * Degrade performance by removing the storage provider and only using cache diff --git a/lib/storage/providers/IDBKeyValProvider/index.ts b/lib/storage/providers/IDBKeyValProvider/index.ts index fc8efa51..f1a4c53c 100644 --- a/lib/storage/providers/IDBKeyValProvider/index.ts +++ b/lib/storage/providers/IDBKeyValProvider/index.ts @@ -1,17 +1,27 @@ import type {UseStore} from 'idb-keyval'; -import {set, keys, getMany, setMany, get, clear, del, delMany, promisifyRequest} from 'idb-keyval'; +import { + set as idbSet, + keys as idbKeys, + getMany as idbGetMany, + setMany as idbSetMany, + get as idbGet, + clear as idbClear, + del as idbDel, + delMany as idbDelMany, + promisifyRequest as idbPromisifyRequest, +} from 'idb-keyval'; import utils from '../../../utils'; import type StorageProvider from '../types'; import type {OnyxKey, OnyxValue} from '../../../types'; import createStore from './createStore'; -// We don't want to initialize the store while the JS bundle loads as idb-keyval will try to use global.indexedDB -// which might not be available in certain environments that load the bundle (e.g. electron main process). -let idbKeyValStore: UseStore; const DB_NAME = 'OnyxDB'; const STORE_NAME = 'keyvaluepairs'; -const provider: StorageProvider = { +const provider: StorageProvider = { + // We don't want to initialize the store while the JS bundle loads as idb-keyval will try to use global.indexedDB + // which might not be available in certain environments that load the bundle (e.g. electron main process). + store: undefined, /** * The name of the provider that can be printed to the logs */ @@ -25,51 +35,71 @@ const provider: StorageProvider = { if (newIdbKeyValStore == null) { throw Error('IDBKeyVal store could not be created'); } - - idbKeyValStore = newIdbKeyValStore; + provider.store = newIdbKeyValStore; }, - setItem: (key, value) => { + setItem(key, value) { + if (!provider.store) { + throw new Error('Store not initialized!'); + } + if (value === null) { provider.removeItem(key); } - return set(key, value, idbKeyValStore); + return idbSet(key, value, provider.store); }, - multiGet: (keysParam) => getMany(keysParam, idbKeyValStore).then((values) => values.map((value, index) => [keysParam[index], value])), - multiMerge: (pairs) => - idbKeyValStore('readwrite', (store) => { - // Note: we are using the manual store transaction here, to fit the read and update - // of the items in one transaction to achieve best performance. - const getValues = Promise.all(pairs.map(([key]) => promisifyRequest>(store.get(key)))); - - return getValues.then((values) => { - const pairsWithoutNull = pairs.filter(([key, value]) => { - if (value === null) { - provider.removeItem(key); - return false; - } - - return true; - }); + multiGet(keysParam) { + if (!provider.store) { + throw new Error('Store not initialized!'); + } + + return idbGetMany(keysParam, provider.store).then((values) => values.map((value, index) => [keysParam[index], value])); + }, + multiMerge(pairs) { + if (!provider.store) { + throw new Error('Store not initialized!'); + } - const upsertMany = pairsWithoutNull.map(([key, value], index) => { - const prev = values[index]; - const newValue = utils.fastMerge(prev as Record, value as Record, { - shouldRemoveNestedNulls: true, - objectRemovalMode: 'replace', - }).result; + return provider + .store('readwrite', (store) => { + // Note: we are using the manual store transaction here, to fit the read and update + // of the items in one transaction to achieve best performance. + const getValues = Promise.all(pairs.map(([key]) => idbPromisifyRequest>(store.get(key)))); - return promisifyRequest(store.put(newValue, key)); + return getValues.then((values) => { + const pairsWithoutNull = pairs.filter(([key, value]) => { + if (value === null) { + provider.removeItem(key); + return false; + } + + return true; + }); + + const upsertMany = pairsWithoutNull.map(([key, value], index) => { + const prev = values[index]; + const newValue = utils.fastMerge(prev as Record, value as Record, { + shouldRemoveNestedNulls: true, + objectRemovalMode: 'replace', + }).result; + + return idbPromisifyRequest(store.put(newValue, key)); + }); + return Promise.all(upsertMany); }); - return Promise.all(upsertMany); - }); - }).then(() => undefined), + }) + .then(() => undefined); + }, mergeItem(key, change) { // Since Onyx already merged the existing value with the changes, we can just set the value directly. return provider.multiMerge([[key, change]]); }, - multiSet: (pairs) => { + multiSet(pairs) { + if (!provider.store) { + throw new Error('Store not initialized!'); + } + const pairsWithoutNull = pairs.filter(([key, value]) => { if (value === null) { provider.removeItem(key); @@ -79,17 +109,52 @@ const provider: StorageProvider = { return true; }) as Array<[IDBValidKey, unknown]>; - return setMany(pairsWithoutNull, idbKeyValStore); + return idbSetMany(pairsWithoutNull, provider.store); + }, + clear() { + if (!provider.store) { + throw new Error('Store not initialized!'); + } + + return idbClear(provider.store); + }, + getAllKeys() { + if (!provider.store) { + throw new Error('Store not initialized!'); + } + + return idbKeys(provider.store); + }, + getItem(key) { + if (!provider.store) { + throw new Error('Store not initialized!'); + } + + return ( + idbGet(key, provider.store) + // idb-keyval returns undefined for missing items, but this needs to return null so that idb-keyval does the same thing as SQLiteStorage. + .then((val) => (val === undefined ? null : val)) + ); + }, + removeItem(key) { + if (!provider.store) { + throw new Error('Store not initialized!'); + } + + return idbDel(key, provider.store); + }, + removeItems(keysParam) { + if (!provider.store) { + throw new Error('Store not initialized!'); + } + + return idbDelMany(keysParam, provider.store); }, - clear: () => clear(idbKeyValStore), - getAllKeys: () => keys(idbKeyValStore), - getItem: (key) => - get(key, idbKeyValStore) - // idb-keyval returns undefined for missing items, but this needs to return null so that idb-keyval does the same thing as SQLiteStorage. - .then((val) => (val === undefined ? null : val)), - removeItem: (key) => del(key, idbKeyValStore), - removeItems: (keysParam) => delMany(keysParam, idbKeyValStore), getDatabaseSize() { + if (!provider.store) { + throw new Error('Store is not initialized!'); + } + if (!window.navigator || !window.navigator.storage) { throw new Error('StorageManager browser API unavailable'); } diff --git a/lib/storage/providers/MemoryOnlyProvider.ts b/lib/storage/providers/MemoryOnlyProvider.ts index 2367e972..b3a44703 100644 --- a/lib/storage/providers/MemoryOnlyProvider.ts +++ b/lib/storage/providers/MemoryOnlyProvider.ts @@ -1,23 +1,25 @@ import _ from 'underscore'; +import type {OnyxKey, OnyxValue} from '../../types'; import utils from '../../utils'; import type StorageProvider from './types'; import type {StorageKeyValuePair} from './types'; -import type {OnyxKey, OnyxValue} from '../../types'; type Store = Record>; // eslint-disable-next-line import/no-mutable-exports -let store: Store = {}; +const storeInternal: Store = {}; const setInternal = (key: OnyxKey, value: OnyxValue) => { - store[key] = value; + storeInternal[key] = value; return Promise.resolve(value); }; const isJestRunning = typeof jest !== 'undefined'; const set = isJestRunning ? jest.fn(setInternal) : setInternal; -const provider: StorageProvider = { +const provider: StorageProvider = { + store: storeInternal, + /** * The name of the provider that can be printed to the logs */ @@ -34,7 +36,7 @@ const provider: StorageProvider = { * Get the value of a given key or return `null` if it's not available in memory */ getItem(key) { - const value = store[key] as OnyxValue; + const value = provider.store[key] as OnyxValue; return Promise.resolve(value === undefined ? (null as OnyxValue) : value); }, @@ -47,7 +49,7 @@ const provider: StorageProvider = { keys, (key) => new Promise((resolve) => { - this.getItem(key).then((value) => resolve([key, value])); + provider.getItem(key).then((value) => resolve([key, value])); }), ) as Array>; return Promise.all(getPromises); @@ -66,7 +68,7 @@ const provider: StorageProvider = { * Stores multiple key-value pairs in a batch */ multiSet(pairs) { - const setPromises = _.map(pairs, ([key, value]) => this.setItem(key, value)); + const setPromises = _.map(pairs, ([key, value]) => provider.setItem(key, value)); return Promise.all(setPromises).then(() => undefined); }, @@ -76,7 +78,7 @@ const provider: StorageProvider = { */ mergeItem(key, change) { // Since Onyx already merged the existing value with the changes, we can just set the value directly. - return this.multiMerge([[key, change]]); + return provider.multiMerge([[key, change]]); }, /** @@ -85,7 +87,7 @@ const provider: StorageProvider = { */ multiMerge(pairs) { _.forEach(pairs, ([key, value]) => { - const existingValue = store[key] as Record; + const existingValue = provider.store[key] as Record; const newValue = utils.fastMerge(existingValue, value as Record, { shouldRemoveNestedNulls: true, @@ -102,7 +104,7 @@ const provider: StorageProvider = { * Remove given key and it's value from memory */ removeItem(key) { - delete store[key]; + delete provider.store[key]; return Promise.resolve(); }, @@ -111,7 +113,7 @@ const provider: StorageProvider = { */ removeItems(keys) { _.each(keys, (key) => { - delete store[key]; + delete provider.store[key]; }); return Promise.resolve(); }, @@ -120,7 +122,8 @@ const provider: StorageProvider = { * Clear everything from memory */ clear() { - store = {}; + // Remove all keys without changing the root object reference. + Object.keys(provider.store).forEach((k) => delete provider.store[k]); return Promise.resolve(); }, @@ -128,7 +131,7 @@ const provider: StorageProvider = { * Returns all keys available in memory */ getAllKeys() { - return Promise.resolve(_.keys(store)); + return Promise.resolve(_.keys(provider.store)); }, /** @@ -141,8 +144,10 @@ const provider: StorageProvider = { }; const setMockStore = (data: Store) => { - store = data; + // Replace keys without changing the root object reference. + Object.keys(storeInternal).forEach((k) => delete storeInternal[k]); + Object.assign(storeInternal, data); }; export default provider; -export {store as mockStore, set as mockSet, setMockStore}; +export {set as mockSet, storeInternal as mockStore, setMockStore}; diff --git a/lib/storage/providers/NoopProvider.ts b/lib/storage/providers/NoopProvider.ts index ccbee65a..f524981d 100644 --- a/lib/storage/providers/NoopProvider.ts +++ b/lib/storage/providers/NoopProvider.ts @@ -1,7 +1,9 @@ import type {OnyxValue} from '../../types'; import type StorageProvider from './types'; -const provider: StorageProvider = { +const provider: StorageProvider = { + store: undefined, + /** * The name of the provider that can be printed to the logs */ diff --git a/lib/storage/providers/SQLiteProvider.ts b/lib/storage/providers/SQLiteProvider.ts index 85917650..1d2927d8 100644 --- a/lib/storage/providers/SQLiteProvider.ts +++ b/lib/storage/providers/SQLiteProvider.ts @@ -42,7 +42,6 @@ type PageCountResult = { }; const DB_NAME = 'OnyxDB'; -let db: NitroSQLiteConnection; /** * Prevents the stringifying of the object markers. @@ -64,7 +63,9 @@ function generateJSONReplaceSQLQueries(key: string, patches: FastMergeReplaceNul return queries; } -const provider: StorageProvider = { +const provider: StorageProvider = { + store: undefined, + /** * The name of the provider that can be printed to the logs */ @@ -73,18 +74,22 @@ const provider: StorageProvider = { * Initializes the storage provider */ init() { - db = open({name: DB_NAME}); + provider.store = open({name: DB_NAME}); - db.execute('CREATE TABLE IF NOT EXISTS keyvaluepairs (record_key TEXT NOT NULL PRIMARY KEY , valueJSON JSON NOT NULL) WITHOUT ROWID;'); + provider.store.execute('CREATE TABLE IF NOT EXISTS keyvaluepairs (record_key TEXT NOT NULL PRIMARY KEY , valueJSON JSON NOT NULL) WITHOUT ROWID;'); // All of the 3 pragmas below were suggested by SQLite team. // You can find more info about them here: https://www.sqlite.org/pragma.html - db.execute('PRAGMA CACHE_SIZE=-20000;'); - db.execute('PRAGMA synchronous=NORMAL;'); - db.execute('PRAGMA journal_mode=WAL;'); + provider.store.execute('PRAGMA CACHE_SIZE=-20000;'); + provider.store.execute('PRAGMA synchronous=NORMAL;'); + provider.store.execute('PRAGMA journal_mode=WAL;'); }, getItem(key) { - return db.executeAsync('SELECT record_key, valueJSON FROM keyvaluepairs WHERE record_key = ?;', [key]).then(({rows}) => { + if (!provider.store) { + throw new Error('Store is not initialized!'); + } + + return provider.store.executeAsync('SELECT record_key, valueJSON FROM keyvaluepairs WHERE record_key = ?;', [key]).then(({rows}) => { if (!rows || rows?.length === 0) { return null; } @@ -98,26 +103,42 @@ const provider: StorageProvider = { }); }, multiGet(keys) { + if (!provider.store) { + throw new Error('Store is not initialized!'); + } + const placeholders = keys.map(() => '?').join(','); const command = `SELECT record_key, valueJSON FROM keyvaluepairs WHERE record_key IN (${placeholders});`; - return db.executeAsync(command, keys).then(({rows}) => { + return provider.store.executeAsync(command, keys).then(({rows}) => { // eslint-disable-next-line no-underscore-dangle const result = rows?._array.map((row) => [row.record_key, JSON.parse(row.valueJSON)]); return (result ?? []) as StorageKeyValuePair[]; }); }, setItem(key, value) { - return db.executeAsync('REPLACE INTO keyvaluepairs (record_key, valueJSON) VALUES (?, ?);', [key, JSON.stringify(value)]).then(() => undefined); + if (!provider.store) { + throw new Error('Store is not initialized!'); + } + + return provider.store.executeAsync('REPLACE INTO keyvaluepairs (record_key, valueJSON) VALUES (?, ?);', [key, JSON.stringify(value)]).then(() => undefined); }, multiSet(pairs) { + if (!provider.store) { + throw new Error('Store is not initialized!'); + } + const query = 'REPLACE INTO keyvaluepairs (record_key, valueJSON) VALUES (?, json(?));'; const params = pairs.map((pair) => [pair[0], JSON.stringify(pair[1] === undefined ? null : pair[1])]); if (utils.isEmptyObject(params)) { return Promise.resolve(); } - return db.executeBatchAsync([{query, params}]).then(() => undefined); + return provider.store.executeBatchAsync([{query, params}]).then(() => undefined); }, multiMerge(pairs) { + if (!provider.store) { + throw new Error('Store is not initialized!'); + } + const commands: BatchQueryCommand[] = []; // Query to merge the change into the DB value. @@ -156,27 +177,52 @@ const provider: StorageProvider = { commands.push({query: replaceQuery, params: replaceQueryArguments}); } - return db.executeBatchAsync(commands).then(() => undefined); + return provider.store.executeBatchAsync(commands).then(() => undefined); }, mergeItem(key, change, replaceNullPatches) { // Since Onyx already merged the existing value with the changes, we can just set the value directly. - return this.multiMerge([[key, change, replaceNullPatches]]); + return provider.multiMerge([[key, change, replaceNullPatches]]); }, - getAllKeys: () => - db.executeAsync('SELECT record_key FROM keyvaluepairs;').then(({rows}) => { + getAllKeys() { + if (!provider.store) { + throw new Error('Store is not initialized!'); + } + + return provider.store.executeAsync('SELECT record_key FROM keyvaluepairs;').then(({rows}) => { // eslint-disable-next-line no-underscore-dangle const result = rows?._array.map((row) => row.record_key); return (result ?? []) as StorageKeyList; - }), - removeItem: (key) => db.executeAsync('DELETE FROM keyvaluepairs WHERE record_key = ?;', [key]).then(() => undefined), - removeItems: (keys) => { + }); + }, + removeItem(key) { + if (!provider.store) { + throw new Error('Store is not initialized!'); + } + + return provider.store.executeAsync('DELETE FROM keyvaluepairs WHERE record_key = ?;', [key]).then(() => undefined); + }, + removeItems(keys) { + if (!provider.store) { + throw new Error('Store is not initialized!'); + } + const placeholders = keys.map(() => '?').join(','); const query = `DELETE FROM keyvaluepairs WHERE record_key IN (${placeholders});`; - return db.executeAsync(query, keys).then(() => undefined); + return provider.store.executeAsync(query, keys).then(() => undefined); + }, + clear() { + if (!provider.store) { + throw new Error('Store is not initialized!'); + } + + return provider.store.executeAsync('DELETE FROM keyvaluepairs;', []).then(() => undefined); }, - clear: () => db.executeAsync('DELETE FROM keyvaluepairs;', []).then(() => undefined), getDatabaseSize() { - return Promise.all([db.executeAsync('PRAGMA page_size;'), db.executeAsync('PRAGMA page_count;'), getFreeDiskStorage()]).then( + if (!provider.store) { + throw new Error('Store is not initialized!'); + } + + return Promise.all([provider.store.executeAsync('PRAGMA page_size;'), provider.store.executeAsync('PRAGMA page_count;'), getFreeDiskStorage()]).then( ([pageSizeResult, pageCountResult, bytesRemaining]) => { const pageSize = pageSizeResult.rows?.item(0)?.page_size ?? 0; const pageCount = pageCountResult.rows?.item(0)?.page_count ?? 0; diff --git a/lib/storage/providers/types.ts b/lib/storage/providers/types.ts index db7525aa..ab275e39 100644 --- a/lib/storage/providers/types.ts +++ b/lib/storage/providers/types.ts @@ -11,15 +11,19 @@ type DatabaseSize = { type OnStorageKeyChanged = (key: TKey, value: OnyxValue) => void; -type StorageProvider = { +type StorageProvider = { + store: TStore; + /** * The name of the provider that can be printed to the logs */ name: string; + /** * Initializes the storage provider */ init: () => void; + /** * Gets the value of a given key or return `null` if it's not available in storage */ From 4d6eb48372915bec20e06cfc572790348c485040 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Fri, 28 Nov 2025 22:24:55 +0000 Subject: [PATCH 03/13] Refactor IDBKeyValProvider tests --- jestSetup.js | 1 - .../providers/IDBKeyvalProviderTest.ts | 377 ++++++++++++------ 2 files changed, 260 insertions(+), 118 deletions(-) diff --git a/jestSetup.js b/jestSetup.js index 82f8f4d5..5c09ddf4 100644 --- a/jestSetup.js +++ b/jestSetup.js @@ -1,7 +1,6 @@ jest.mock('./lib/storage'); jest.mock('./lib/storage/platforms/index.native', () => require('./lib/storage/__mocks__')); jest.mock('./lib/storage/platforms/index', () => require('./lib/storage/__mocks__')); -jest.mock('./lib/storage/providers/IDBKeyValProvider', () => require('./lib/storage/__mocks__')); jest.mock('react-native-device-info', () => ({getFreeDiskStorage: () => {}})); jest.mock('react-native-nitro-sqlite', () => ({ diff --git a/tests/unit/storage/providers/IDBKeyvalProviderTest.ts b/tests/unit/storage/providers/IDBKeyvalProviderTest.ts index e5e84d43..1b8e4b9a 100644 --- a/tests/unit/storage/providers/IDBKeyvalProviderTest.ts +++ b/tests/unit/storage/providers/IDBKeyvalProviderTest.ts @@ -1,139 +1,282 @@ +import {getMany as idbGetMany, clear as idbClear, get as idbGet, keys as idbKeys, set as idbSet, setMany as idbSetMany} from 'idb-keyval'; import IDBKeyValProvider from '../../../../lib/storage/providers/IDBKeyValProvider'; -import createDeferredTask from '../../../../lib/createDeferredTask'; -import waitForPromisesToResolve from '../../../utils/waitForPromisesToResolve'; -import type StorageMock from '../../../../lib/storage/__mocks__'; - -const IDBKeyValProviderMock = IDBKeyValProvider as unknown as typeof StorageMock; - -describe('storage/providers/IDBKeyVal', () => { - const SAMPLE_ITEMS: Array<[string, unknown]> = [ - ['string', 'Plain String'], - ['array', ['Mixed', {array: [{id: 1}, {id: 2}]}]], - ['true', true], - ['false', false], - ['object', {id: 'Object', nested: {content: 'Nested object'}}], - ['number', 100], +import utils from '../../../../lib/utils'; +import type {GenericDeepRecord} from '../../../types'; + +const ONYXKEYS = { + TEST_KEY: 'test', + TEST_KEY_2: 'test2', + TEST_KEY_3: 'test3', + COLLECTION: { + TEST_KEY: 'test_', + TEST_KEY_2: 'test2_', + }, +}; + +describe('IDBKeyValProvider', () => { + const testEntries: Array<[string, unknown]> = [ + [ONYXKEYS.TEST_KEY, 'value'], + [ONYXKEYS.TEST_KEY_2, 1000], + [ + ONYXKEYS.TEST_KEY_3, + { + key: 'value', + property: { + nestedProperty: { + nestedKey1: 'nestedValue1', + nestedKey2: 'nestedValue2', + }, + }, + }, + ], + [`${ONYXKEYS.COLLECTION.TEST_KEY}id1`, true], + [`${ONYXKEYS.COLLECTION.TEST_KEY}id2`, ['a', {key: 'value'}, 1, true]], ]; - // For some reason fake timers cause promises to hang - beforeAll(() => jest.useRealTimers()); - beforeEach(() => { - jest.clearAllMocks(); - IDBKeyValProviderMock.clear(); - IDBKeyValProviderMock.clear.mockClear(); + beforeEach(async () => { + IDBKeyValProvider.init(); + await idbClear(IDBKeyValProvider.store); }); - it('multiSet', () => { - // Given multiple pairs to be saved in storage - const pairs = SAMPLE_ITEMS.slice(); + describe('getItem', () => { + it('should return the stored value for the key', async () => { + await idbSet(ONYXKEYS.TEST_KEY, 'value', IDBKeyValProvider.store); + expect(await IDBKeyValProvider.getItem(ONYXKEYS.TEST_KEY)).toEqual('value'); + }); - // When they are saved - return IDBKeyValProviderMock.multiSet(pairs).then(() => { - // We expect a call to idbKeyval.setItem for each pair - pairs.forEach(([key, value]) => expect(IDBKeyValProviderMock.setItem).toHaveBeenCalledWith(key, value)); + it('should return null if there is no stored value for the key', async () => { + expect(await IDBKeyValProvider.getItem(ONYXKEYS.TEST_KEY)).toBeNull(); }); }); - it('multiGet', () => { - // Given we have some data in storage - IDBKeyValProviderMock.multiSet(SAMPLE_ITEMS); + describe('multiGet', () => { + it('should return the tuples in the order of the keys supplied in a batch', async () => { + await idbSetMany(testEntries, IDBKeyValProvider.store); - return waitForPromisesToResolve().then(() => { - // Then multi get should retrieve them - const keys = SAMPLE_ITEMS.map(([key]) => key); - return IDBKeyValProviderMock.multiGet(keys).then((pairs) => expect(pairs).toEqual(expect.arrayContaining(SAMPLE_ITEMS))); + expect(await IDBKeyValProvider.multiGet([`${ONYXKEYS.COLLECTION.TEST_KEY}id1`, ONYXKEYS.TEST_KEY, ONYXKEYS.TEST_KEY_2])).toEqual([ + testEntries[3], + testEntries[0], + testEntries[1], + ]); }); }); - it('multiMerge', () => { - // Given existing data in storage - const USER_1 = { - name: 'Tom', - age: 30, - traits: {hair: 'brown'}, - }; - - const USER_2 = { - name: 'Sarah', - age: 25, - traits: {hair: 'black'}, - }; - - IDBKeyValProviderMock.multiSet([ - ['@USER_1', USER_1], - ['@USER_2', USER_2], - ]); - - return waitForPromisesToResolve().then(() => { - (IDBKeyValProviderMock.mockSet as jest.Mock).mockClear(); - - // Given deltas matching existing structure - const USER_1_DELTA = { - age: 31, - traits: {eyes: 'blue'}, - }; - - const USER_2_DELTA = { - age: 26, - traits: {hair: 'green'}, - }; - - // When data is merged to storage - return IDBKeyValProviderMock.multiMerge([ - ['@USER_1', USER_1_DELTA], - ['@USER_2', USER_2_DELTA], - ]).then(() => { - // Then each existing item should be set with the merged content - expect(IDBKeyValProviderMock.mockSet).toHaveBeenNthCalledWith(1, '@USER_1', { - name: 'Tom', - age: 31, - traits: { - hair: 'brown', - eyes: 'blue', - }, - }); + describe('setItem', () => { + it('should set the value to the key', async () => { + await IDBKeyValProvider.setItem(ONYXKEYS.TEST_KEY, 'value'); + expect(await idbGet(ONYXKEYS.TEST_KEY, IDBKeyValProvider.store)).toEqual('value'); + }); + + // FIXME: 🐞 IDBKeyValProvider - setItem removes and redundantly sets the value to null + it.skip('should remove the key when passing null', async () => { + await IDBKeyValProvider.setItem(ONYXKEYS.TEST_KEY, 'value'); + expect(await idbGet(ONYXKEYS.TEST_KEY, IDBKeyValProvider.store)).toEqual('value'); + + await IDBKeyValProvider.setItem(ONYXKEYS.TEST_KEY, null); + expect(await idbKeys(IDBKeyValProvider.store)).not.toContainEqual(ONYXKEYS.TEST_KEY); + }); + }); + + describe('multiSet', () => { + it('should set multiple keys in a batch', async () => { + await IDBKeyValProvider.multiSet(testEntries); + expect( + await idbGetMany( + testEntries.map((e) => e[0]), + IDBKeyValProvider.store, + ), + ).toEqual(testEntries.map((e) => (e[1] === null ? undefined : e[1]))); + }); + + // FIXME: 🐞 IDBKeyValProvider - multiSet calls multiple removeItem's instead of delMany + it('should set and remove multiple keys in a batch', async () => { + await idbSetMany(testEntries, IDBKeyValProvider.store); + const changedEntries: Array<[string, unknown]> = [ + [ONYXKEYS.TEST_KEY, 'value_changed'], + [ONYXKEYS.TEST_KEY_2, null], + [ONYXKEYS.TEST_KEY_3, {changed: true}], + [`${ONYXKEYS.COLLECTION.TEST_KEY}id1`, null], + ]; - expect(IDBKeyValProviderMock.mockSet).toHaveBeenNthCalledWith(2, '@USER_2', { - name: 'Sarah', - age: 26, - traits: { - hair: 'green', + await IDBKeyValProvider.multiSet(changedEntries); + // ONYXKEYS.TEST_KEY, ONYXKEYS.TEST_KEY_3 and `${ONYXKEYS.COLLECTION.TEST_KEY}id2`. + expect((await idbKeys(IDBKeyValProvider.store)).length).toEqual(3); + expect( + await idbGetMany( + changedEntries.map((e) => e[0]), + IDBKeyValProvider.store, + ), + ).toEqual(changedEntries.map((e) => (e[1] === null ? undefined : e[1]))); + }); + }); + + describe('multiMerge', () => { + it('should merge multiple keys in a batch', async () => { + await idbSetMany(testEntries, IDBKeyValProvider.store); + const changedEntries: Array<[string, unknown]> = [ + // FIXME: 🐞 IDBKeyValProvider (possibly other places): Primitives are incorrectly stored when using multiMerge + // [ONYXKEYS.TEST_KEY, 'value_changed'], + // [ONYXKEYS.TEST_KEY_2, 1001], + [ + ONYXKEYS.TEST_KEY_3, + { + key: 'value_changed', + property: { + nestedProperty: { + nestedKey2: 'nestedValue2_changed', + [utils.ONYX_INTERNALS__REPLACE_OBJECT_MARK]: true, + }, + newKey: 'newValue', + }, }, - }); + ], + // FIXME: 🐞 IDBKeyValProvider (possibly other places): Primitives are incorrectly stored when using multiMerge + // [`${ONYXKEYS.COLLECTION.TEST_KEY}id1`, false], + [`${ONYXKEYS.COLLECTION.TEST_KEY}id2`, ['a', {newKey: 'newValue'}]], + ]; + + const expectedEntries = structuredClone(changedEntries); + const expectedTestKey3Value = structuredClone(testEntries[2])[1] as GenericDeepRecord; + expectedTestKey3Value.key = 'value_changed'; + expectedTestKey3Value.property.nestedProperty = {nestedKey2: 'nestedValue2_changed'}; + expectedTestKey3Value.property.newKey = 'newValue'; + expectedEntries[0][1] = expectedTestKey3Value; + + await IDBKeyValProvider.multiMerge(changedEntries); + expect( + await idbGetMany( + expectedEntries.map((e) => e[0]), + IDBKeyValProvider.store, + ), + ).toEqual(expectedEntries.map((e) => (e[1] === null ? undefined : e[1]))); + }); + + // FIXME: 🐞 IDBKeyValProvider - multiMerge calls multiple removeItem's instead of using the transaction + // FIXME: 🐞 IDBKeyValProvider: Index misalignment between pairs and values in multiMerge + // FIXME: Check if multiMerge is supposed to handle null values + it.skip('should merge and delete multiple keys in a batch', async () => { + await idbSetMany(testEntries, IDBKeyValProvider.store); + const changedEntries: Array<[string, unknown]> = [ + [ONYXKEYS.TEST_KEY, null], + [ONYXKEYS.TEST_KEY_2, null], + [ONYXKEYS.TEST_KEY_3, {key: 'value_changed'}], + [`${ONYXKEYS.COLLECTION.TEST_KEY}id1`, null], + [`${ONYXKEYS.COLLECTION.TEST_KEY}id2`, ['a', {newKey: 'newValue'}]], + ]; + + const expectedEntries = structuredClone(changedEntries); + const expectedTestKey3Value = structuredClone(testEntries[2])[1] as GenericDeepRecord; + expectedTestKey3Value.key = 'value_changed'; + expectedEntries[2][1] = expectedTestKey3Value; + + await IDBKeyValProvider.multiMerge(changedEntries); + // ONYXKEYS.TEST_KEY_3 and `${ONYXKEYS.COLLECTION.TEST_KEY}id2`. + expect((await idbKeys(IDBKeyValProvider.store)).length).toEqual(2); + expect( + await idbGetMany( + expectedEntries.map((e) => e[0]), + IDBKeyValProvider.store, + ), + ).toEqual(expectedEntries.map((e) => (e[1] === null ? undefined : e[1]))); + }); + }); + + describe('mergeItem', () => { + it('should merge all the supported kinds of data correctly', async () => { + await idbSet(ONYXKEYS.TEST_KEY, 'value', IDBKeyValProvider.store); + await idbSet(ONYXKEYS.TEST_KEY_2, 1000, IDBKeyValProvider.store); + await idbSet(ONYXKEYS.TEST_KEY_3, {key: 'value', property: {propertyKey: 'propertyValue'}}, IDBKeyValProvider.store); + await idbSet(`${ONYXKEYS.COLLECTION.TEST_KEY}id1`, true, IDBKeyValProvider.store); + await idbSet(`${ONYXKEYS.COLLECTION.TEST_KEY}id2`, ['a', {key: 'value'}, 1, true], IDBKeyValProvider.store); + + await IDBKeyValProvider.mergeItem(ONYXKEYS.TEST_KEY, 'value_changed'); + await IDBKeyValProvider.mergeItem(ONYXKEYS.TEST_KEY_2, 1001); + await IDBKeyValProvider.mergeItem(ONYXKEYS.TEST_KEY_3, { + key: 'value_changed', + property: { + [utils.ONYX_INTERNALS__REPLACE_OBJECT_MARK]: true, + newKey: 'newValue', + }, }); + await IDBKeyValProvider.mergeItem(`${ONYXKEYS.COLLECTION.TEST_KEY}id1` as string, false); + await IDBKeyValProvider.mergeItem(`${ONYXKEYS.COLLECTION.TEST_KEY}id2` as string, ['a', {newKey: 'newValue'}]); + + // FIXME: 🐞 IDBKeyValProvider (possibly other places): Primitives are incorrectly stored when using multiMerge + // expect(await idbGet(ONYXKEYS.TEST_KEY, IDBKeyValProvider.store)).toEqual('value_changed'); + // expect(await idbGet(ONYXKEYS.TEST_KEY_2, IDBKeyValProvider.store)).toEqual(1001); + expect(await idbGet(ONYXKEYS.TEST_KEY_3, IDBKeyValProvider.store)).toEqual({key: 'value_changed', property: {newKey: 'newValue'}}); + // expect(await idbGet(`${ONYXKEYS.COLLECTION.TEST_KEY}id1`, IDBKeyValProvider.store)).toEqual(false); + expect(await idbGet(`${ONYXKEYS.COLLECTION.TEST_KEY}id2`, IDBKeyValProvider.store)).toEqual(['a', {newKey: 'newValue'}]); + }); + + // FIXME: Check if multiMerge is supposed to handle null values + it('should remove the key when passing null', async () => { + await idbSet(ONYXKEYS.TEST_KEY, 'value', IDBKeyValProvider.store); + + await IDBKeyValProvider.mergeItem(ONYXKEYS.TEST_KEY, null); + expect(await idbKeys(IDBKeyValProvider.store)).not.toContainEqual(ONYXKEYS.TEST_KEY); + }); + }); + + describe('getAllKeys', () => { + it('should list all the keys stored', async () => { + await idbSetMany(testEntries, IDBKeyValProvider.store); + expect((await IDBKeyValProvider.getAllKeys()).length).toEqual(5); }); }); - it('clear', () => { - // We're creating a Promise which we programatically control when to resolve. - const task = createDeferredTask(); - - // We configure idbKeyval.setItem to return this promise the first time it's called and to otherwise return resolved promises - IDBKeyValProviderMock.setItem = jest - .fn() - .mockReturnValue(Promise.resolve()) // Default behavior - .mockReturnValueOnce(task.promise); // First call behavior - - // Make 5 StorageProvider.setItem calls - this adds 5 items to the queue and starts executing the first idbKeyval.setItem - for (let i = 0; i < 5; i++) { - IDBKeyValProviderMock.setItem(`key${i}`, `value${i}`); - } - - // At this point,`idbKeyval.setItem` should have been called once, but we control when it resolves, and we'll keep it unresolved. - // This simulates the 1st idbKeyval.setItem taking a random time. - // We then call StorageProvider.clear() while the first idbKeyval.setItem isn't completed yet. - IDBKeyValProviderMock.clear(); - - // Any calls that follow this would have been queued - so we don't expect more than 1 `idbKeyval.setItem` call after the - // first one resolves. - task.resolve?.(); - - // waitForPromisesToResolve() makes jest wait for any promises (even promises returned as the result of a promise) to resolve. - // If StorageProvider.clear() does not abort the queue, more idbKeyval.setItem calls would be executed because they would - // be sitting in the setItemQueue - return waitForPromisesToResolve().then(() => { - expect(IDBKeyValProviderMock.mockSet).toHaveBeenCalledTimes(0); - expect(IDBKeyValProviderMock.clear).toHaveBeenCalledTimes(1); + describe('removeItem', () => { + it('should remove the key from the store', async () => { + await idbSetMany(testEntries, IDBKeyValProvider.store); + expect(await idbKeys(IDBKeyValProvider.store)).toContainEqual(ONYXKEYS.TEST_KEY); + + await IDBKeyValProvider.removeItem(ONYXKEYS.TEST_KEY); + expect(await idbKeys(IDBKeyValProvider.store)).not.toContainEqual(ONYXKEYS.TEST_KEY); + }); + }); + + describe('removeItem', () => { + it('should remove all the supplied keys from the store', async () => { + await idbSetMany(testEntries, IDBKeyValProvider.store); + expect(await idbKeys(IDBKeyValProvider.store)).toContainEqual(ONYXKEYS.TEST_KEY); + expect(await idbKeys(IDBKeyValProvider.store)).toContainEqual(ONYXKEYS.TEST_KEY_3); + + await IDBKeyValProvider.removeItems([ONYXKEYS.TEST_KEY, ONYXKEYS.TEST_KEY_3]); + expect(await idbKeys(IDBKeyValProvider.store)).not.toContainEqual(ONYXKEYS.TEST_KEY); + expect(await idbKeys(IDBKeyValProvider.store)).not.toContainEqual(ONYXKEYS.TEST_KEY_3); + }); + }); + + describe('clear', () => { + it('should clear the storage', async () => { + await idbSetMany(testEntries, IDBKeyValProvider.store); + expect((await idbKeys(IDBKeyValProvider.store)).length).toEqual(5); + + await IDBKeyValProvider.clear(); + expect((await idbKeys(IDBKeyValProvider.store)).length).toEqual(0); + }); + }); + + describe('getDatabaseSize', () => { + beforeEach(() => { + Object.defineProperty(window.navigator, 'storage', { + value: { + estimate: jest.fn().mockResolvedValue({quota: 750000, usage: 250000}), + }, + configurable: true, + }); + }); + + afterEach(() => { + // @ts-expect-error tear down of mocked property + delete window.navigator.storage; + }); + + it('should get the current size of the store', async () => { + expect(await IDBKeyValProvider.getDatabaseSize()).toEqual({ + bytesUsed: 250000, + bytesRemaining: 500000, + }); }); }); }); From 5aa029e7dc80b4e252a6302239fa900b1073520d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Fri, 28 Nov 2025 22:42:24 +0000 Subject: [PATCH 04/13] Simplify idb imports --- .../IDBKeyValProvider/createStore.ts | 6 +- .../providers/IDBKeyValProvider/index.ts | 32 +++----- .../providers/IDBKeyvalProviderTest.ts | 80 +++++++++---------- 3 files changed, 54 insertions(+), 64 deletions(-) diff --git a/lib/storage/providers/IDBKeyValProvider/createStore.ts b/lib/storage/providers/IDBKeyValProvider/createStore.ts index c83ce8d1..b0ed89ea 100644 --- a/lib/storage/providers/IDBKeyValProvider/createStore.ts +++ b/lib/storage/providers/IDBKeyValProvider/createStore.ts @@ -1,4 +1,4 @@ -import {promisifyRequest} from 'idb-keyval'; +import IDB from 'idb-keyval'; import type {UseStore} from 'idb-keyval'; import {logInfo} from '../../../Logger'; @@ -12,7 +12,7 @@ function createStore(dbName: string, storeName: string): UseStore { if (dbp) return dbp; const request = indexedDB.open(dbName); request.onupgradeneeded = () => request.result.createObjectStore(storeName); - dbp = promisifyRequest(request); + dbp = IDB.promisifyRequest(request); dbp.then( (db) => { @@ -49,7 +49,7 @@ function createStore(dbName: string, storeName: string): UseStore { updatedDatabase.createObjectStore(storeName); }; - dbp = promisifyRequest(request); + dbp = IDB.promisifyRequest(request); return dbp; }; diff --git a/lib/storage/providers/IDBKeyValProvider/index.ts b/lib/storage/providers/IDBKeyValProvider/index.ts index f1a4c53c..ee6d6236 100644 --- a/lib/storage/providers/IDBKeyValProvider/index.ts +++ b/lib/storage/providers/IDBKeyValProvider/index.ts @@ -1,15 +1,5 @@ import type {UseStore} from 'idb-keyval'; -import { - set as idbSet, - keys as idbKeys, - getMany as idbGetMany, - setMany as idbSetMany, - get as idbGet, - clear as idbClear, - del as idbDel, - delMany as idbDelMany, - promisifyRequest as idbPromisifyRequest, -} from 'idb-keyval'; +import IDB from 'idb-keyval'; import utils from '../../../utils'; import type StorageProvider from '../types'; import type {OnyxKey, OnyxValue} from '../../../types'; @@ -47,14 +37,14 @@ const provider: StorageProvider = { provider.removeItem(key); } - return idbSet(key, value, provider.store); + return IDB.set(key, value, provider.store); }, multiGet(keysParam) { if (!provider.store) { throw new Error('Store not initialized!'); } - return idbGetMany(keysParam, provider.store).then((values) => values.map((value, index) => [keysParam[index], value])); + return IDB.getMany(keysParam, provider.store).then((values) => values.map((value, index) => [keysParam[index], value])); }, multiMerge(pairs) { if (!provider.store) { @@ -65,7 +55,7 @@ const provider: StorageProvider = { .store('readwrite', (store) => { // Note: we are using the manual store transaction here, to fit the read and update // of the items in one transaction to achieve best performance. - const getValues = Promise.all(pairs.map(([key]) => idbPromisifyRequest>(store.get(key)))); + const getValues = Promise.all(pairs.map(([key]) => IDB.promisifyRequest>(store.get(key)))); return getValues.then((values) => { const pairsWithoutNull = pairs.filter(([key, value]) => { @@ -84,7 +74,7 @@ const provider: StorageProvider = { objectRemovalMode: 'replace', }).result; - return idbPromisifyRequest(store.put(newValue, key)); + return IDB.promisifyRequest(store.put(newValue, key)); }); return Promise.all(upsertMany); }); @@ -109,21 +99,21 @@ const provider: StorageProvider = { return true; }) as Array<[IDBValidKey, unknown]>; - return idbSetMany(pairsWithoutNull, provider.store); + return IDB.setMany(pairsWithoutNull, provider.store); }, clear() { if (!provider.store) { throw new Error('Store not initialized!'); } - return idbClear(provider.store); + return IDB.clear(provider.store); }, getAllKeys() { if (!provider.store) { throw new Error('Store not initialized!'); } - return idbKeys(provider.store); + return IDB.keys(provider.store); }, getItem(key) { if (!provider.store) { @@ -131,7 +121,7 @@ const provider: StorageProvider = { } return ( - idbGet(key, provider.store) + IDB.get(key, provider.store) // idb-keyval returns undefined for missing items, but this needs to return null so that idb-keyval does the same thing as SQLiteStorage. .then((val) => (val === undefined ? null : val)) ); @@ -141,14 +131,14 @@ const provider: StorageProvider = { throw new Error('Store not initialized!'); } - return idbDel(key, provider.store); + return IDB.del(key, provider.store); }, removeItems(keysParam) { if (!provider.store) { throw new Error('Store not initialized!'); } - return idbDelMany(keysParam, provider.store); + return IDB.delMany(keysParam, provider.store); }, getDatabaseSize() { if (!provider.store) { diff --git a/tests/unit/storage/providers/IDBKeyvalProviderTest.ts b/tests/unit/storage/providers/IDBKeyvalProviderTest.ts index 1b8e4b9a..ebf0af36 100644 --- a/tests/unit/storage/providers/IDBKeyvalProviderTest.ts +++ b/tests/unit/storage/providers/IDBKeyvalProviderTest.ts @@ -1,4 +1,4 @@ -import {getMany as idbGetMany, clear as idbClear, get as idbGet, keys as idbKeys, set as idbSet, setMany as idbSetMany} from 'idb-keyval'; +import IDB from 'idb-keyval'; import IDBKeyValProvider from '../../../../lib/storage/providers/IDBKeyValProvider'; import utils from '../../../../lib/utils'; import type {GenericDeepRecord} from '../../../types'; @@ -35,12 +35,12 @@ describe('IDBKeyValProvider', () => { beforeEach(async () => { IDBKeyValProvider.init(); - await idbClear(IDBKeyValProvider.store); + await IDB.clear(IDBKeyValProvider.store); }); describe('getItem', () => { it('should return the stored value for the key', async () => { - await idbSet(ONYXKEYS.TEST_KEY, 'value', IDBKeyValProvider.store); + await IDB.set(ONYXKEYS.TEST_KEY, 'value', IDBKeyValProvider.store); expect(await IDBKeyValProvider.getItem(ONYXKEYS.TEST_KEY)).toEqual('value'); }); @@ -51,7 +51,7 @@ describe('IDBKeyValProvider', () => { describe('multiGet', () => { it('should return the tuples in the order of the keys supplied in a batch', async () => { - await idbSetMany(testEntries, IDBKeyValProvider.store); + await IDB.setMany(testEntries, IDBKeyValProvider.store); expect(await IDBKeyValProvider.multiGet([`${ONYXKEYS.COLLECTION.TEST_KEY}id1`, ONYXKEYS.TEST_KEY, ONYXKEYS.TEST_KEY_2])).toEqual([ testEntries[3], @@ -64,16 +64,16 @@ describe('IDBKeyValProvider', () => { describe('setItem', () => { it('should set the value to the key', async () => { await IDBKeyValProvider.setItem(ONYXKEYS.TEST_KEY, 'value'); - expect(await idbGet(ONYXKEYS.TEST_KEY, IDBKeyValProvider.store)).toEqual('value'); + expect(await IDB.get(ONYXKEYS.TEST_KEY, IDBKeyValProvider.store)).toEqual('value'); }); // FIXME: 🐞 IDBKeyValProvider - setItem removes and redundantly sets the value to null it.skip('should remove the key when passing null', async () => { await IDBKeyValProvider.setItem(ONYXKEYS.TEST_KEY, 'value'); - expect(await idbGet(ONYXKEYS.TEST_KEY, IDBKeyValProvider.store)).toEqual('value'); + expect(await IDB.get(ONYXKEYS.TEST_KEY, IDBKeyValProvider.store)).toEqual('value'); await IDBKeyValProvider.setItem(ONYXKEYS.TEST_KEY, null); - expect(await idbKeys(IDBKeyValProvider.store)).not.toContainEqual(ONYXKEYS.TEST_KEY); + expect(await IDB.keys(IDBKeyValProvider.store)).not.toContainEqual(ONYXKEYS.TEST_KEY); }); }); @@ -81,7 +81,7 @@ describe('IDBKeyValProvider', () => { it('should set multiple keys in a batch', async () => { await IDBKeyValProvider.multiSet(testEntries); expect( - await idbGetMany( + await IDB.getMany( testEntries.map((e) => e[0]), IDBKeyValProvider.store, ), @@ -90,7 +90,7 @@ describe('IDBKeyValProvider', () => { // FIXME: 🐞 IDBKeyValProvider - multiSet calls multiple removeItem's instead of delMany it('should set and remove multiple keys in a batch', async () => { - await idbSetMany(testEntries, IDBKeyValProvider.store); + await IDB.setMany(testEntries, IDBKeyValProvider.store); const changedEntries: Array<[string, unknown]> = [ [ONYXKEYS.TEST_KEY, 'value_changed'], [ONYXKEYS.TEST_KEY_2, null], @@ -100,9 +100,9 @@ describe('IDBKeyValProvider', () => { await IDBKeyValProvider.multiSet(changedEntries); // ONYXKEYS.TEST_KEY, ONYXKEYS.TEST_KEY_3 and `${ONYXKEYS.COLLECTION.TEST_KEY}id2`. - expect((await idbKeys(IDBKeyValProvider.store)).length).toEqual(3); + expect((await IDB.keys(IDBKeyValProvider.store)).length).toEqual(3); expect( - await idbGetMany( + await IDB.getMany( changedEntries.map((e) => e[0]), IDBKeyValProvider.store, ), @@ -112,7 +112,7 @@ describe('IDBKeyValProvider', () => { describe('multiMerge', () => { it('should merge multiple keys in a batch', async () => { - await idbSetMany(testEntries, IDBKeyValProvider.store); + await IDB.setMany(testEntries, IDBKeyValProvider.store); const changedEntries: Array<[string, unknown]> = [ // FIXME: 🐞 IDBKeyValProvider (possibly other places): Primitives are incorrectly stored when using multiMerge // [ONYXKEYS.TEST_KEY, 'value_changed'], @@ -144,7 +144,7 @@ describe('IDBKeyValProvider', () => { await IDBKeyValProvider.multiMerge(changedEntries); expect( - await idbGetMany( + await IDB.getMany( expectedEntries.map((e) => e[0]), IDBKeyValProvider.store, ), @@ -155,7 +155,7 @@ describe('IDBKeyValProvider', () => { // FIXME: 🐞 IDBKeyValProvider: Index misalignment between pairs and values in multiMerge // FIXME: Check if multiMerge is supposed to handle null values it.skip('should merge and delete multiple keys in a batch', async () => { - await idbSetMany(testEntries, IDBKeyValProvider.store); + await IDB.setMany(testEntries, IDBKeyValProvider.store); const changedEntries: Array<[string, unknown]> = [ [ONYXKEYS.TEST_KEY, null], [ONYXKEYS.TEST_KEY_2, null], @@ -171,9 +171,9 @@ describe('IDBKeyValProvider', () => { await IDBKeyValProvider.multiMerge(changedEntries); // ONYXKEYS.TEST_KEY_3 and `${ONYXKEYS.COLLECTION.TEST_KEY}id2`. - expect((await idbKeys(IDBKeyValProvider.store)).length).toEqual(2); + expect((await IDB.keys(IDBKeyValProvider.store)).length).toEqual(2); expect( - await idbGetMany( + await IDB.getMany( expectedEntries.map((e) => e[0]), IDBKeyValProvider.store, ), @@ -183,11 +183,11 @@ describe('IDBKeyValProvider', () => { describe('mergeItem', () => { it('should merge all the supported kinds of data correctly', async () => { - await idbSet(ONYXKEYS.TEST_KEY, 'value', IDBKeyValProvider.store); - await idbSet(ONYXKEYS.TEST_KEY_2, 1000, IDBKeyValProvider.store); - await idbSet(ONYXKEYS.TEST_KEY_3, {key: 'value', property: {propertyKey: 'propertyValue'}}, IDBKeyValProvider.store); - await idbSet(`${ONYXKEYS.COLLECTION.TEST_KEY}id1`, true, IDBKeyValProvider.store); - await idbSet(`${ONYXKEYS.COLLECTION.TEST_KEY}id2`, ['a', {key: 'value'}, 1, true], IDBKeyValProvider.store); + await IDB.set(ONYXKEYS.TEST_KEY, 'value', IDBKeyValProvider.store); + await IDB.set(ONYXKEYS.TEST_KEY_2, 1000, IDBKeyValProvider.store); + await IDB.set(ONYXKEYS.TEST_KEY_3, {key: 'value', property: {propertyKey: 'propertyValue'}}, IDBKeyValProvider.store); + await IDB.set(`${ONYXKEYS.COLLECTION.TEST_KEY}id1`, true, IDBKeyValProvider.store); + await IDB.set(`${ONYXKEYS.COLLECTION.TEST_KEY}id2`, ['a', {key: 'value'}, 1, true], IDBKeyValProvider.store); await IDBKeyValProvider.mergeItem(ONYXKEYS.TEST_KEY, 'value_changed'); await IDBKeyValProvider.mergeItem(ONYXKEYS.TEST_KEY_2, 1001); @@ -202,58 +202,58 @@ describe('IDBKeyValProvider', () => { await IDBKeyValProvider.mergeItem(`${ONYXKEYS.COLLECTION.TEST_KEY}id2` as string, ['a', {newKey: 'newValue'}]); // FIXME: 🐞 IDBKeyValProvider (possibly other places): Primitives are incorrectly stored when using multiMerge - // expect(await idbGet(ONYXKEYS.TEST_KEY, IDBKeyValProvider.store)).toEqual('value_changed'); - // expect(await idbGet(ONYXKEYS.TEST_KEY_2, IDBKeyValProvider.store)).toEqual(1001); - expect(await idbGet(ONYXKEYS.TEST_KEY_3, IDBKeyValProvider.store)).toEqual({key: 'value_changed', property: {newKey: 'newValue'}}); - // expect(await idbGet(`${ONYXKEYS.COLLECTION.TEST_KEY}id1`, IDBKeyValProvider.store)).toEqual(false); - expect(await idbGet(`${ONYXKEYS.COLLECTION.TEST_KEY}id2`, IDBKeyValProvider.store)).toEqual(['a', {newKey: 'newValue'}]); + // expect(await IDB.get(ONYXKEYS.TEST_KEY, IDBKeyValProvider.store)).toEqual('value_changed'); + // expect(await IDB.get(ONYXKEYS.TEST_KEY_2, IDBKeyValProvider.store)).toEqual(1001); + expect(await IDB.get(ONYXKEYS.TEST_KEY_3, IDBKeyValProvider.store)).toEqual({key: 'value_changed', property: {newKey: 'newValue'}}); + // expect(await IDB.get(`${ONYXKEYS.COLLECTION.TEST_KEY}id1`, IDBKeyValProvider.store)).toEqual(false); + expect(await IDB.get(`${ONYXKEYS.COLLECTION.TEST_KEY}id2`, IDBKeyValProvider.store)).toEqual(['a', {newKey: 'newValue'}]); }); // FIXME: Check if multiMerge is supposed to handle null values it('should remove the key when passing null', async () => { - await idbSet(ONYXKEYS.TEST_KEY, 'value', IDBKeyValProvider.store); + await IDB.set(ONYXKEYS.TEST_KEY, 'value', IDBKeyValProvider.store); await IDBKeyValProvider.mergeItem(ONYXKEYS.TEST_KEY, null); - expect(await idbKeys(IDBKeyValProvider.store)).not.toContainEqual(ONYXKEYS.TEST_KEY); + expect(await IDB.keys(IDBKeyValProvider.store)).not.toContainEqual(ONYXKEYS.TEST_KEY); }); }); describe('getAllKeys', () => { it('should list all the keys stored', async () => { - await idbSetMany(testEntries, IDBKeyValProvider.store); + await IDB.setMany(testEntries, IDBKeyValProvider.store); expect((await IDBKeyValProvider.getAllKeys()).length).toEqual(5); }); }); describe('removeItem', () => { it('should remove the key from the store', async () => { - await idbSetMany(testEntries, IDBKeyValProvider.store); - expect(await idbKeys(IDBKeyValProvider.store)).toContainEqual(ONYXKEYS.TEST_KEY); + await IDB.setMany(testEntries, IDBKeyValProvider.store); + expect(await IDB.keys(IDBKeyValProvider.store)).toContainEqual(ONYXKEYS.TEST_KEY); await IDBKeyValProvider.removeItem(ONYXKEYS.TEST_KEY); - expect(await idbKeys(IDBKeyValProvider.store)).not.toContainEqual(ONYXKEYS.TEST_KEY); + expect(await IDB.keys(IDBKeyValProvider.store)).not.toContainEqual(ONYXKEYS.TEST_KEY); }); }); describe('removeItem', () => { it('should remove all the supplied keys from the store', async () => { - await idbSetMany(testEntries, IDBKeyValProvider.store); - expect(await idbKeys(IDBKeyValProvider.store)).toContainEqual(ONYXKEYS.TEST_KEY); - expect(await idbKeys(IDBKeyValProvider.store)).toContainEqual(ONYXKEYS.TEST_KEY_3); + await IDB.setMany(testEntries, IDBKeyValProvider.store); + expect(await IDB.keys(IDBKeyValProvider.store)).toContainEqual(ONYXKEYS.TEST_KEY); + expect(await IDB.keys(IDBKeyValProvider.store)).toContainEqual(ONYXKEYS.TEST_KEY_3); await IDBKeyValProvider.removeItems([ONYXKEYS.TEST_KEY, ONYXKEYS.TEST_KEY_3]); - expect(await idbKeys(IDBKeyValProvider.store)).not.toContainEqual(ONYXKEYS.TEST_KEY); - expect(await idbKeys(IDBKeyValProvider.store)).not.toContainEqual(ONYXKEYS.TEST_KEY_3); + expect(await IDB.keys(IDBKeyValProvider.store)).not.toContainEqual(ONYXKEYS.TEST_KEY); + expect(await IDB.keys(IDBKeyValProvider.store)).not.toContainEqual(ONYXKEYS.TEST_KEY_3); }); }); describe('clear', () => { it('should clear the storage', async () => { - await idbSetMany(testEntries, IDBKeyValProvider.store); - expect((await idbKeys(IDBKeyValProvider.store)).length).toEqual(5); + await IDB.setMany(testEntries, IDBKeyValProvider.store); + expect((await IDB.keys(IDBKeyValProvider.store)).length).toEqual(5); await IDBKeyValProvider.clear(); - expect((await idbKeys(IDBKeyValProvider.store)).length).toEqual(0); + expect((await IDB.keys(IDBKeyValProvider.store)).length).toEqual(0); }); }); From 8a913932e4fdad03ff25b34f887647347d4e4dd5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Tue, 2 Dec 2025 08:16:59 +0000 Subject: [PATCH 05/13] Fix "IDBKeyValProvider - setItem removes and redundantly sets the value to null" --- lib/storage/providers/IDBKeyValProvider/index.ts | 2 +- tests/unit/storage/providers/IDBKeyvalProviderTest.ts | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/storage/providers/IDBKeyValProvider/index.ts b/lib/storage/providers/IDBKeyValProvider/index.ts index ee6d6236..01cad0be 100644 --- a/lib/storage/providers/IDBKeyValProvider/index.ts +++ b/lib/storage/providers/IDBKeyValProvider/index.ts @@ -34,7 +34,7 @@ const provider: StorageProvider = { } if (value === null) { - provider.removeItem(key); + return provider.removeItem(key); } return IDB.set(key, value, provider.store); diff --git a/tests/unit/storage/providers/IDBKeyvalProviderTest.ts b/tests/unit/storage/providers/IDBKeyvalProviderTest.ts index ebf0af36..47c5ee3a 100644 --- a/tests/unit/storage/providers/IDBKeyvalProviderTest.ts +++ b/tests/unit/storage/providers/IDBKeyvalProviderTest.ts @@ -67,7 +67,6 @@ describe('IDBKeyValProvider', () => { expect(await IDB.get(ONYXKEYS.TEST_KEY, IDBKeyValProvider.store)).toEqual('value'); }); - // FIXME: 🐞 IDBKeyValProvider - setItem removes and redundantly sets the value to null it.skip('should remove the key when passing null', async () => { await IDBKeyValProvider.setItem(ONYXKEYS.TEST_KEY, 'value'); expect(await IDB.get(ONYXKEYS.TEST_KEY, IDBKeyValProvider.store)).toEqual('value'); From f5185aae2f97d4304d70115314110c1bd353bfd4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Tue, 2 Dec 2025 12:26:22 +0000 Subject: [PATCH 06/13] Fix "IDBKeyValProvider - multiSet calls multiple removeItem's instead of delMany" --- .../providers/IDBKeyValProvider/index.ts | 19 ++++++++++--------- .../providers/IDBKeyvalProviderTest.ts | 1 - 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/lib/storage/providers/IDBKeyValProvider/index.ts b/lib/storage/providers/IDBKeyValProvider/index.ts index 01cad0be..21aaca71 100644 --- a/lib/storage/providers/IDBKeyValProvider/index.ts +++ b/lib/storage/providers/IDBKeyValProvider/index.ts @@ -90,16 +90,17 @@ const provider: StorageProvider = { throw new Error('Store not initialized!'); } - const pairsWithoutNull = pairs.filter(([key, value]) => { - if (value === null) { - provider.removeItem(key); - return false; - } - - return true; - }) as Array<[IDBValidKey, unknown]>; + return provider.store('readwrite', (store) => { + pairs.forEach(([key, value]) => { + if (value === null) { + store.delete(key); + } else { + store.put(value, key); + } + }); - return IDB.setMany(pairsWithoutNull, provider.store); + return IDB.promisifyRequest(store.transaction); + }); }, clear() { if (!provider.store) { diff --git a/tests/unit/storage/providers/IDBKeyvalProviderTest.ts b/tests/unit/storage/providers/IDBKeyvalProviderTest.ts index 47c5ee3a..8a16d064 100644 --- a/tests/unit/storage/providers/IDBKeyvalProviderTest.ts +++ b/tests/unit/storage/providers/IDBKeyvalProviderTest.ts @@ -87,7 +87,6 @@ describe('IDBKeyValProvider', () => { ).toEqual(testEntries.map((e) => (e[1] === null ? undefined : e[1]))); }); - // FIXME: 🐞 IDBKeyValProvider - multiSet calls multiple removeItem's instead of delMany it('should set and remove multiple keys in a batch', async () => { await IDB.setMany(testEntries, IDBKeyValProvider.store); const changedEntries: Array<[string, unknown]> = [ From 0eb066d6d53187adce6700089e5af962c70fe02c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Tue, 2 Dec 2025 15:36:36 +0000 Subject: [PATCH 07/13] Fix "IDBKeyValProvider - multiMerge calls multiple removeItem's instead of using the transaction" and "IDBKeyValProvider: Index misalignment between pairs and values in multiMerge" --- .../providers/IDBKeyValProvider/index.ts | 41 ++++++++----------- .../providers/IDBKeyvalProviderTest.ts | 5 +-- 2 files changed, 18 insertions(+), 28 deletions(-) diff --git a/lib/storage/providers/IDBKeyValProvider/index.ts b/lib/storage/providers/IDBKeyValProvider/index.ts index 21aaca71..f5ec04f0 100644 --- a/lib/storage/providers/IDBKeyValProvider/index.ts +++ b/lib/storage/providers/IDBKeyValProvider/index.ts @@ -51,35 +51,28 @@ const provider: StorageProvider = { throw new Error('Store not initialized!'); } - return provider - .store('readwrite', (store) => { - // Note: we are using the manual store transaction here, to fit the read and update - // of the items in one transaction to achieve best performance. - const getValues = Promise.all(pairs.map(([key]) => IDB.promisifyRequest>(store.get(key)))); - - return getValues.then((values) => { - const pairsWithoutNull = pairs.filter(([key, value]) => { - if (value === null) { - provider.removeItem(key); - return false; - } - - return true; - }); - - const upsertMany = pairsWithoutNull.map(([key, value], index) => { - const prev = values[index]; - const newValue = utils.fastMerge(prev as Record, value as Record, { + return provider.store('readwrite', (store) => { + // Note: we are using the manual store transaction here, to fit the read and update + // of the items in one transaction to achieve best performance. + const getValues = Promise.all(pairs.map(([key]) => IDB.promisifyRequest>(store.get(key)))); + + return getValues.then((values) => { + pairs.forEach(([key, value], index) => { + if (value === null) { + store.delete(key); + } else { + const newValue = utils.fastMerge(values[index] as Record, value as Record, { shouldRemoveNestedNulls: true, objectRemovalMode: 'replace', }).result; - return IDB.promisifyRequest(store.put(newValue, key)); - }); - return Promise.all(upsertMany); + store.put(newValue, key); + } }); - }) - .then(() => undefined); + + return IDB.promisifyRequest(store.transaction); + }); + }); }, mergeItem(key, change) { // Since Onyx already merged the existing value with the changes, we can just set the value directly. diff --git a/tests/unit/storage/providers/IDBKeyvalProviderTest.ts b/tests/unit/storage/providers/IDBKeyvalProviderTest.ts index 8a16d064..f8dc6eb2 100644 --- a/tests/unit/storage/providers/IDBKeyvalProviderTest.ts +++ b/tests/unit/storage/providers/IDBKeyvalProviderTest.ts @@ -149,10 +149,7 @@ describe('IDBKeyValProvider', () => { ).toEqual(expectedEntries.map((e) => (e[1] === null ? undefined : e[1]))); }); - // FIXME: 🐞 IDBKeyValProvider - multiMerge calls multiple removeItem's instead of using the transaction - // FIXME: 🐞 IDBKeyValProvider: Index misalignment between pairs and values in multiMerge - // FIXME: Check if multiMerge is supposed to handle null values - it.skip('should merge and delete multiple keys in a batch', async () => { + it('should merge and delete multiple keys in a batch', async () => { await IDB.setMany(testEntries, IDBKeyValProvider.store); const changedEntries: Array<[string, unknown]> = [ [ONYXKEYS.TEST_KEY, null], From dd7ed160d9ff49627798f1a99669819bbecfcdc8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Tue, 2 Dec 2025 17:01:23 +0000 Subject: [PATCH 08/13] Fix "IDBKeyValProvider (possibly other places): Primitives are incorrectly stored when using multiMerge" --- lib/utils.ts | 4 +- tests/unit/fastMergeTest.ts | 192 ++++++++++-------- .../providers/IDBKeyvalProviderTest.ts | 18 +- 3 files changed, 120 insertions(+), 94 deletions(-) diff --git a/lib/utils.ts b/lib/utils.ts index 28056800..35e0cbca 100644 --- a/lib/utils.ts +++ b/lib/utils.ts @@ -52,10 +52,10 @@ function fastMerge(target: TValue, source: TValue, options?: FastMergeOp }; } - // We have to ignore arrays and nullish values here, + // We have to ignore arrays, primitives and nullish values here, // otherwise "mergeObject" will throw an error, // because it expects an object as "source" - if (Array.isArray(source) || source === null || source === undefined) { + if (!isMergeableObject(source)) { return {result: source, replaceNullPatches: metadata.replaceNullPatches}; } diff --git a/tests/unit/fastMergeTest.ts b/tests/unit/fastMergeTest.ts index 0823db57..5936e4ff 100644 --- a/tests/unit/fastMergeTest.ts +++ b/tests/unit/fastMergeTest.ts @@ -53,93 +53,87 @@ const testMergeChanges: DeepObject[] = [ ]; describe('fastMerge', () => { - it('should merge an object with another object and remove nested null values', () => { - const result = utils.fastMerge(testObject, testObjectWithNullishValues, {shouldRemoveNestedNulls: true}); - - expect(result.result).toEqual({ - a: 'a', - b: { - c: { - h: 'h', - }, - d: { - f: 'f', - }, - g: 'g', - }, + describe('primitives', () => { + it('should replace strings', () => { + const result = utils.fastMerge('old', 'new'); + expect(result.result).toEqual('new'); }); - }); - it('should merge an object with another object and not remove nested null values', () => { - const result = utils.fastMerge(testObject, testObjectWithNullishValues); + it('should replace numbers', () => { + const result = utils.fastMerge(1000, 1001); + expect(result.result).toEqual(1001); + }); - expect(result.result).toEqual({ - a: 'a', - b: { - c: { - h: 'h', - }, - d: { - e: null, - f: 'f', - }, - g: 'g', - }, + it('should replace booleans', () => { + const result = utils.fastMerge(true, false); + expect(result.result).toEqual(false); }); }); - it('should merge an object with an empty object and remove deeply nested null values', () => { - const result = utils.fastMerge({}, testObjectWithNullishValues, { - shouldRemoveNestedNulls: true, + describe('arrays', () => { + it('should replace arrays', () => { + const result = utils.fastMerge(['a', 1, true], ['b', false]); + expect(result.result).toEqual(['b', false]); }); - - expect(result.result).toEqual(testObjectWithNullValuesRemoved); }); - it('should remove null values by merging two identical objects with fastMerge', () => { - const result = utils.removeNestedNullValues(testObjectWithNullishValues); - - expect(result).toEqual(testObjectWithNullValuesRemoved); - }); + describe('objects', () => { + it('should merge an object with another object and remove nested null values', () => { + const result = utils.fastMerge(testObject, testObjectWithNullishValues, {shouldRemoveNestedNulls: true}); - it('should replace an object with an array', () => { - const result = utils.fastMerge(testObject, [1, 2, 3], { - shouldRemoveNestedNulls: true, + expect(result.result).toEqual({ + a: 'a', + b: { + c: { + h: 'h', + }, + d: { + f: 'f', + }, + g: 'g', + }, + }); }); - expect(result.result).toEqual([1, 2, 3]); - }); + it('should merge an object with another object and not remove nested null values', () => { + const result = utils.fastMerge(testObject, testObjectWithNullishValues); - it('should replace an array with an object', () => { - const result = utils.fastMerge([1, 2, 3], testObject, { - shouldRemoveNestedNulls: true, + expect(result.result).toEqual({ + a: 'a', + b: { + c: { + h: 'h', + }, + d: { + e: null, + f: 'f', + }, + g: 'g', + }, + }); }); - expect(result.result).toEqual(testObject); - }); + it('should merge an object with an empty object and remove deeply nested null values', () => { + const result = utils.fastMerge({}, testObjectWithNullishValues, { + shouldRemoveNestedNulls: true, + }); - it('should add the "ONYX_INTERNALS__REPLACE_OBJECT_MARK" flag to the merged object when the change is set to null and "objectRemovalMode" is set to "mark"', () => { - const result = utils.fastMerge(testMergeChanges[1], testMergeChanges[0], { - shouldRemoveNestedNulls: true, - objectRemovalMode: 'mark', + expect(result.result).toEqual(testObjectWithNullValuesRemoved); }); - expect(result.result).toEqual({ - b: { - d: { - h: 'h', - [utils.ONYX_INTERNALS__REPLACE_OBJECT_MARK]: true, - }, - h: 'h', - }, + it('should remove null values by merging two identical objects with fastMerge', () => { + const result = utils.removeNestedNullValues(testObjectWithNullishValues); + + expect(result).toEqual(testObjectWithNullValuesRemoved); }); - expect(result.replaceNullPatches).toEqual([[['b', 'd'], {h: 'h'}]]); - }); - it('should completely replace the target object with its source when the source has the "ONYX_INTERNALS__REPLACE_OBJECT_MARK" flag and "objectRemovalMode" is set to "replace"', () => { - const result = utils.fastMerge( - testObject, - { + it('should add the "ONYX_INTERNALS__REPLACE_OBJECT_MARK" flag to the merged object when the change is set to null and "objectRemovalMode" is set to "mark"', () => { + const result = utils.fastMerge(testMergeChanges[1], testMergeChanges[0], { + shouldRemoveNestedNulls: true, + objectRemovalMode: 'mark', + }); + + expect(result.result).toEqual({ b: { d: { h: 'h', @@ -147,23 +141,59 @@ describe('fastMerge', () => { }, h: 'h', }, - }, - { - shouldRemoveNestedNulls: true, - objectRemovalMode: 'replace', - }, - ); + }); + expect(result.replaceNullPatches).toEqual([[['b', 'd'], {h: 'h'}]]); + }); + + it('should completely replace the target object with its source when the source has the "ONYX_INTERNALS__REPLACE_OBJECT_MARK" flag and "objectRemovalMode" is set to "replace"', () => { + const result = utils.fastMerge( + testObject, + { + b: { + d: { + h: 'h', + [utils.ONYX_INTERNALS__REPLACE_OBJECT_MARK]: true, + }, + h: 'h', + }, + }, + { + shouldRemoveNestedNulls: true, + objectRemovalMode: 'replace', + }, + ); - expect(result.result).toEqual({ - a: 'a', - b: { - c: 'c', - d: { + expect(result.result).toEqual({ + a: 'a', + b: { + c: 'c', + d: { + h: 'h', + }, h: 'h', + g: 'g', }, - h: 'h', - g: 'g', - }, + }); + }); + + test.each([ + ['a string', 'value'], + ['a number', 1000], + ['a boolean', true], + ['an array', []], + ])('should replace an object with %s', (_label, expected) => { + const result = utils.fastMerge(testObject, expected); + expect(result.result).toEqual(expected); + }); + + test.each([ + ['a string', 'value'], + ['a number', 1000], + ['a boolean', true], + ['an array', []], + ])('should %s with an object', (_label, data) => { + const result = utils.fastMerge(data, testObject); + expect(result.result).toEqual(testObject); }); }); }); diff --git a/tests/unit/storage/providers/IDBKeyvalProviderTest.ts b/tests/unit/storage/providers/IDBKeyvalProviderTest.ts index f8dc6eb2..c7d83f31 100644 --- a/tests/unit/storage/providers/IDBKeyvalProviderTest.ts +++ b/tests/unit/storage/providers/IDBKeyvalProviderTest.ts @@ -112,9 +112,8 @@ describe('IDBKeyValProvider', () => { it('should merge multiple keys in a batch', async () => { await IDB.setMany(testEntries, IDBKeyValProvider.store); const changedEntries: Array<[string, unknown]> = [ - // FIXME: 🐞 IDBKeyValProvider (possibly other places): Primitives are incorrectly stored when using multiMerge - // [ONYXKEYS.TEST_KEY, 'value_changed'], - // [ONYXKEYS.TEST_KEY_2, 1001], + [ONYXKEYS.TEST_KEY, 'value_changed'], + [ONYXKEYS.TEST_KEY_2, 1001], [ ONYXKEYS.TEST_KEY_3, { @@ -128,8 +127,7 @@ describe('IDBKeyValProvider', () => { }, }, ], - // FIXME: 🐞 IDBKeyValProvider (possibly other places): Primitives are incorrectly stored when using multiMerge - // [`${ONYXKEYS.COLLECTION.TEST_KEY}id1`, false], + [`${ONYXKEYS.COLLECTION.TEST_KEY}id1`, false], [`${ONYXKEYS.COLLECTION.TEST_KEY}id2`, ['a', {newKey: 'newValue'}]], ]; @@ -138,7 +136,7 @@ describe('IDBKeyValProvider', () => { expectedTestKey3Value.key = 'value_changed'; expectedTestKey3Value.property.nestedProperty = {nestedKey2: 'nestedValue2_changed'}; expectedTestKey3Value.property.newKey = 'newValue'; - expectedEntries[0][1] = expectedTestKey3Value; + expectedEntries[2][1] = expectedTestKey3Value; await IDBKeyValProvider.multiMerge(changedEntries); expect( @@ -196,15 +194,13 @@ describe('IDBKeyValProvider', () => { await IDBKeyValProvider.mergeItem(`${ONYXKEYS.COLLECTION.TEST_KEY}id1` as string, false); await IDBKeyValProvider.mergeItem(`${ONYXKEYS.COLLECTION.TEST_KEY}id2` as string, ['a', {newKey: 'newValue'}]); - // FIXME: 🐞 IDBKeyValProvider (possibly other places): Primitives are incorrectly stored when using multiMerge - // expect(await IDB.get(ONYXKEYS.TEST_KEY, IDBKeyValProvider.store)).toEqual('value_changed'); - // expect(await IDB.get(ONYXKEYS.TEST_KEY_2, IDBKeyValProvider.store)).toEqual(1001); + expect(await IDB.get(ONYXKEYS.TEST_KEY, IDBKeyValProvider.store)).toEqual('value_changed'); + expect(await IDB.get(ONYXKEYS.TEST_KEY_2, IDBKeyValProvider.store)).toEqual(1001); expect(await IDB.get(ONYXKEYS.TEST_KEY_3, IDBKeyValProvider.store)).toEqual({key: 'value_changed', property: {newKey: 'newValue'}}); - // expect(await IDB.get(`${ONYXKEYS.COLLECTION.TEST_KEY}id1`, IDBKeyValProvider.store)).toEqual(false); + expect(await IDB.get(`${ONYXKEYS.COLLECTION.TEST_KEY}id1`, IDBKeyValProvider.store)).toEqual(false); expect(await IDB.get(`${ONYXKEYS.COLLECTION.TEST_KEY}id2`, IDBKeyValProvider.store)).toEqual(['a', {newKey: 'newValue'}]); }); - // FIXME: Check if multiMerge is supposed to handle null values it('should remove the key when passing null', async () => { await IDB.set(ONYXKEYS.TEST_KEY, 'value', IDBKeyValProvider.store); From f088dec173656f65e8bdc2165bcee9da63bcea8a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Thu, 11 Dec 2025 21:32:50 +0000 Subject: [PATCH 09/13] Use star import to fix runtime errors --- lib/storage/providers/IDBKeyValProvider/createStore.ts | 2 +- lib/storage/providers/IDBKeyValProvider/index.ts | 2 +- tests/unit/storage/providers/IDBKeyvalProviderTest.ts | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/storage/providers/IDBKeyValProvider/createStore.ts b/lib/storage/providers/IDBKeyValProvider/createStore.ts index b0ed89ea..b9d3da67 100644 --- a/lib/storage/providers/IDBKeyValProvider/createStore.ts +++ b/lib/storage/providers/IDBKeyValProvider/createStore.ts @@ -1,4 +1,4 @@ -import IDB from 'idb-keyval'; +import * as IDB from 'idb-keyval'; import type {UseStore} from 'idb-keyval'; import {logInfo} from '../../../Logger'; diff --git a/lib/storage/providers/IDBKeyValProvider/index.ts b/lib/storage/providers/IDBKeyValProvider/index.ts index f5ec04f0..51831045 100644 --- a/lib/storage/providers/IDBKeyValProvider/index.ts +++ b/lib/storage/providers/IDBKeyValProvider/index.ts @@ -1,5 +1,5 @@ import type {UseStore} from 'idb-keyval'; -import IDB from 'idb-keyval'; +import * as IDB from 'idb-keyval'; import utils from '../../../utils'; import type StorageProvider from '../types'; import type {OnyxKey, OnyxValue} from '../../../types'; diff --git a/tests/unit/storage/providers/IDBKeyvalProviderTest.ts b/tests/unit/storage/providers/IDBKeyvalProviderTest.ts index c7d83f31..bb29ec9a 100644 --- a/tests/unit/storage/providers/IDBKeyvalProviderTest.ts +++ b/tests/unit/storage/providers/IDBKeyvalProviderTest.ts @@ -1,4 +1,4 @@ -import IDB from 'idb-keyval'; +import * as IDB from 'idb-keyval'; import IDBKeyValProvider from '../../../../lib/storage/providers/IDBKeyValProvider'; import utils from '../../../../lib/utils'; import type {GenericDeepRecord} from '../../../types'; From 8acc9e164dca96c0953e8a4f2bdbf1ce96321748 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Henriques?= Date: Thu, 11 Dec 2025 23:26:42 +0000 Subject: [PATCH 10/13] Typo --- tests/unit/fastMergeTest.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/fastMergeTest.ts b/tests/unit/fastMergeTest.ts index 5936e4ff..06132d43 100644 --- a/tests/unit/fastMergeTest.ts +++ b/tests/unit/fastMergeTest.ts @@ -191,7 +191,7 @@ describe('fastMerge', () => { ['a number', 1000], ['a boolean', true], ['an array', []], - ])('should %s with an object', (_label, data) => { + ])('should replace %s with an object', (_label, data) => { const result = utils.fastMerge(data, testObject); expect(result.result).toEqual(testObject); }); From 5b0dd48afc7e1ba3182e306b21f0484149897b1e Mon Sep 17 00:00:00 2001 From: VickyStash Date: Wed, 17 Dec 2025 18:30:27 +0100 Subject: [PATCH 11/13] Update jest test to include Date/RegExp tests --- tests/unit/fastMergeTest.ts | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/unit/fastMergeTest.ts b/tests/unit/fastMergeTest.ts index 06132d43..edcbdfab 100644 --- a/tests/unit/fastMergeTest.ts +++ b/tests/unit/fastMergeTest.ts @@ -127,6 +127,20 @@ describe('fastMerge', () => { expect(result).toEqual(testObjectWithNullValuesRemoved); }); + it('should replace Date objects', () => { + const oldDate = new Date('2024-01-01'); + const newDate = new Date('2025-01-01'); + const result = utils.fastMerge(oldDate, newDate); + expect(result.result).toEqual(newDate); + }); + + it('should replace RegExp objects', () => { + const oldRegex = /old/gi; + const newRegex = /new/i; + const result = utils.fastMerge(oldRegex, newRegex); + expect(result.result).toEqual(newRegex); + }); + it('should add the "ONYX_INTERNALS__REPLACE_OBJECT_MARK" flag to the merged object when the change is set to null and "objectRemovalMode" is set to "mark"', () => { const result = utils.fastMerge(testMergeChanges[1], testMergeChanges[0], { shouldRemoveNestedNulls: true, From 616d497b527f392fad488a70bdf9631830162cde Mon Sep 17 00:00:00 2001 From: VickyStash Date: Wed, 17 Dec 2025 18:57:46 +0100 Subject: [PATCH 12/13] Replace forEach with for..of --- lib/storage/providers/IDBKeyValProvider/index.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/storage/providers/IDBKeyValProvider/index.ts b/lib/storage/providers/IDBKeyValProvider/index.ts index 51831045..e57bd351 100644 --- a/lib/storage/providers/IDBKeyValProvider/index.ts +++ b/lib/storage/providers/IDBKeyValProvider/index.ts @@ -57,7 +57,7 @@ const provider: StorageProvider = { const getValues = Promise.all(pairs.map(([key]) => IDB.promisifyRequest>(store.get(key)))); return getValues.then((values) => { - pairs.forEach(([key, value], index) => { + for (const [index, [key, value]] of pairs.entries()) { if (value === null) { store.delete(key); } else { @@ -68,7 +68,7 @@ const provider: StorageProvider = { store.put(newValue, key); } - }); + } return IDB.promisifyRequest(store.transaction); }); From 14115ade52b9bd14b61b19fc8c0fc49305c7973d Mon Sep 17 00:00:00 2001 From: VickyStash Date: Fri, 19 Dec 2025 10:39:11 +0100 Subject: [PATCH 13/13] Replace more forEach with for..of --- lib/storage/providers/IDBKeyValProvider/index.ts | 4 ++-- lib/storage/providers/MemoryOnlyProvider.ts | 8 ++++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/lib/storage/providers/IDBKeyValProvider/index.ts b/lib/storage/providers/IDBKeyValProvider/index.ts index e57bd351..c140ed76 100644 --- a/lib/storage/providers/IDBKeyValProvider/index.ts +++ b/lib/storage/providers/IDBKeyValProvider/index.ts @@ -84,13 +84,13 @@ const provider: StorageProvider = { } return provider.store('readwrite', (store) => { - pairs.forEach(([key, value]) => { + for (const [key, value] of pairs) { if (value === null) { store.delete(key); } else { store.put(value, key); } - }); + } return IDB.promisifyRequest(store.transaction); }); diff --git a/lib/storage/providers/MemoryOnlyProvider.ts b/lib/storage/providers/MemoryOnlyProvider.ts index b3a44703..d3fcb7c1 100644 --- a/lib/storage/providers/MemoryOnlyProvider.ts +++ b/lib/storage/providers/MemoryOnlyProvider.ts @@ -123,7 +123,9 @@ const provider: StorageProvider = { */ clear() { // Remove all keys without changing the root object reference. - Object.keys(provider.store).forEach((k) => delete provider.store[k]); + for (const key of Object.keys(provider.store)) { + delete provider.store[key]; + } return Promise.resolve(); }, @@ -145,7 +147,9 @@ const provider: StorageProvider = { const setMockStore = (data: Store) => { // Replace keys without changing the root object reference. - Object.keys(storeInternal).forEach((k) => delete storeInternal[k]); + for (const key of Object.keys(storeInternal)) { + delete storeInternal[key]; + } Object.assign(storeInternal, data); };