diff --git a/lib/Onyx.ts b/lib/Onyx.ts index 1474e33d..45065165 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -45,7 +45,7 @@ function init({ if (shouldSyncMultipleInstances) { Storage.keepInstancesSync?.((key, value) => { - const prevValue = cache.getValue(key, false) as OnyxValue; + const prevValue = cache.get(key, false) as OnyxValue; cache.set(key, value); OnyxUtils.keyChanged(key, value as OnyxValue, prevValue); }); @@ -111,7 +111,7 @@ function connect(connectOptions: ConnectOptions): nu // Performance improvement // If the mapping is connected to an onyx key that is not a collection // we can skip the call to getAllKeys() and return an array with a single item - if (Boolean(mapping.key) && typeof mapping.key === 'string' && !mapping.key.endsWith('_') && cache.storageKeys.has(mapping.key)) { + if (Boolean(mapping.key) && typeof mapping.key === 'string' && !mapping.key.endsWith('_') && cache.getAllKeys().has(mapping.key)) { return new Set([mapping.key]); } return OnyxUtils.getAllKeys(); @@ -128,12 +128,12 @@ function connect(connectOptions: ConnectOptions): nu // component. This null value will be filtered out so that the connected component can utilize defaultProps. if (matchingKeys.length === 0) { if (mapping.key && !OnyxUtils.isCollectionKey(mapping.key)) { - cache.set(mapping.key, null); + cache.addNullishStorageKey(mapping.key); } - // Here we cannot use batching because the null value is expected to be set immediately for default props + // Here we cannot use batching because the nullish value is expected to be set immediately for default props // or they will be undefined. - OnyxUtils.sendDataToConnection(mapping, null as OnyxValue, undefined, false); + OnyxUtils.sendDataToConnection(mapping, null, undefined, false); return; } @@ -210,8 +210,20 @@ function disconnect(connectionID: number, keyToRemoveFromEvictionBlocklist?: Ony * @param value value to store */ function set(key: TKey, value: NonUndefined>): Promise { - // check if the value is compatible with the existing value in the storage - const existingValue = cache.getValue(key, false); + // When we use Onyx.set to set a key we want to clear the current delta changes from Onyx.merge that were queued + // before the value was set. If Onyx.merge is currently reading the old value from storage, it will then not apply the changes. + if (OnyxUtils.hasPendingMergeForKey(key)) { + delete OnyxUtils.getMergeQueue()[key]; + } + + const existingValue = cache.get(key, false); + + // If the existing value as well as the new value are null, we can return early. + if (value === null && existingValue === null) { + return Promise.resolve(); + } + + // Check if the value is compatible with the existing value in the storage const {isCompatible, existingValueType, newValueType} = utils.checkCompatibilityWithExistingValue(value, existingValue); if (!isCompatible) { Logger.logAlert(logMessages.incompatibleUpdateAlert(key, 'set', existingValueType, newValueType)); @@ -220,22 +232,29 @@ function set(key: TKey, value: NonUndefined; - if (OnyxUtils.hasPendingMergeForKey(key)) { - delete OnyxUtils.getMergeQueue()[key]; + const logSetCall = (hasChanged = true) => { + // Logging properties only since values could be sensitive things we don't want to log + Logger.logInfo(`set called for key: ${key}${_.isObject(value) ? ` properties: ${_.keys(value).join(',')}` : ''} hasChanged: ${hasChanged}`); + }; + + // Calling "OnyxUtils.removeNullValues" removes the key from storage and cache and updates the subscriber. + // Therefore, we don't need to further broadcast and update the value so we can return early. + if (wasRemoved) { + logSetCall(); + return Promise.resolve(); } + const valueWithoutNullValues = valueAfterRemoving as OnyxValue; const hasChanged = cache.hasValueChanged(key, valueWithoutNullValues); - // Logging properties only since values could be sensitive things we don't want to log - Logger.logInfo(`set called for key: ${key}${_.isObject(value) ? ` properties: ${_.keys(value).join(',')}` : ''} hasChanged: ${hasChanged}`); + logSetCall(hasChanged); // This approach prioritizes fast UI changes without waiting for data to be stored in device storage. - const updatePromise = OnyxUtils.broadcastUpdate(key, valueWithoutNullValues, hasChanged, wasRemoved); + const updatePromise = OnyxUtils.broadcastUpdate(key, valueWithoutNullValues, hasChanged); // If the value has not changed or the key got removed, calling Storage.setItem() would be redundant and a waste of performance, so return early instead. - if (!hasChanged || wasRemoved) { + if (!hasChanged) { return updatePromise; } @@ -255,21 +274,33 @@ function set(key: TKey, value: NonUndefined): Promise { - const keyValuePairs = OnyxUtils.prepareKeyValuePairsForStorage(data, true); + const allKeyValuePairs = OnyxUtils.prepareKeyValuePairsForStorage(data, true); + + // When a key is set to null, we need to remove the remove the key from storage using "OnyxUtils.remove". + // Therefore, we filter the key value pairs to exclude null values and remove those keys explicitly. + const removePromises: Array> = []; + const keyValuePairsToUpdate = allKeyValuePairs.filter(([key, value]) => { + if (value === null) { + removePromises.push(OnyxUtils.remove(key)); + return false; + } + + return true; + }); - const updatePromises = keyValuePairs.map(([key, value]) => { - const prevValue = cache.getValue(key, false); + const updatePromises = keyValuePairsToUpdate.map(([key, value]) => { + const prevValue = cache.get(key, false); // Update cache and optimistically inform subscribers on the next tick cache.set(key, value); return OnyxUtils.scheduleSubscriberUpdate(key, value, prevValue); }); - return Storage.multiSet(keyValuePairs) + return Storage.multiSet(allKeyValuePairs) .catch((error) => OnyxUtils.evictStorageAndRetry(error, multiSet, data)) .then(() => { OnyxUtils.sendActionToDevTools(OnyxUtils.METHOD.MULTI_SET, undefined, data); - return Promise.all(updatePromises); + return Promise.all([removePromises, updatePromises]); }) .then(() => undefined); } @@ -311,7 +342,7 @@ function merge(key: TKey, changes: NonUndefined { // Calls to Onyx.set after a merge will terminate the current merge process and clear the merge queue if (mergeQueue[key] == null) { - return undefined; + return Promise.resolve(); } try { @@ -326,7 +357,7 @@ function merge(key: TKey, changes: NonUndefined(key: TKey, changes: NonUndefined { + // Logging properties only since values could be sensitive things we don't want to log + Logger.logInfo(`merge called for key: ${key}${_.isObject(batchedDeltaChanges) ? ` properties: ${_.keys(batchedDeltaChanges).join(',')}` : ''} hasChanged: ${hasChanged}`); + }; + // If the batched changes equal null, we want to remove the key from storage, to reduce storage size const {wasRemoved} = OnyxUtils.removeNullValues(key, batchedDeltaChanges); + // Calling "OnyxUtils.removeNullValues" removes the key from storage and cache and updates the subscriber. + // Therefore, we don't need to further broadcast and update the value so we can return early. + if (wasRemoved) { + logMergeCall(); + return Promise.resolve(); + } + // For providers that can't handle delta changes, we need to merge the batched changes with the existing value beforehand. // The "preMergedValue" will be directly "set" in storage instead of being merged // Therefore we merge the batched changes with the existing value to get the final merged value that will be stored. @@ -351,14 +394,13 @@ function merge(key: TKey, changes: NonUndefined, hasChanged, wasRemoved); + const updatePromise = OnyxUtils.broadcastUpdate(key, preMergedValue as OnyxValue, hasChanged); // If the value has not changed, calling Storage.setItem() would be redundant and a waste of performance, so return early instead. - if (!hasChanged || wasRemoved) { + if (!hasChanged) { return updatePromise; } @@ -518,6 +560,8 @@ function mergeCollection(collectionKey: TK function clear(keysToPreserve: OnyxKey[] = []): Promise { return OnyxUtils.getAllKeys() .then((keys) => { + cache.clearNullishStorageKeys(); + const keysToBeClearedFromStorage: OnyxKey[] = []; const keyValuesToResetAsCollection: Record> = {}; const keyValuesToResetIndividually: NullableKeyValueMapping = {}; @@ -537,7 +581,7 @@ function clear(keysToPreserve: OnyxKey[] = []): Promise { // 2. Figure out whether it is a collection key or not, // since collection key subscribers need to be updated differently if (!isKeyToPreserve) { - const oldValue = cache.getValue(key); + const oldValue = cache.get(key); const newValue = defaultKeyStates[key] ?? null; if (newValue !== oldValue) { cache.set(key, newValue); @@ -565,7 +609,7 @@ function clear(keysToPreserve: OnyxKey[] = []): Promise { // Notify the subscribers for each key/value group so they can receive the new values Object.entries(keyValuesToResetIndividually).forEach(([key, value]) => { - updatePromises.push(OnyxUtils.scheduleSubscriberUpdate(key, value, cache.getValue(key, false))); + updatePromises.push(OnyxUtils.scheduleSubscriberUpdate(key, value, cache.get(key, false))); }); Object.entries(keyValuesToResetAsCollection).forEach(([key, value]) => { updatePromises.push(OnyxUtils.scheduleNotifyCollectionSubscribers(key, value)); diff --git a/lib/OnyxCache.ts b/lib/OnyxCache.ts index f139b041..2d323369 100644 --- a/lib/OnyxCache.ts +++ b/lib/OnyxCache.ts @@ -9,7 +9,10 @@ import type {OnyxKey, OnyxValue} from './types'; */ class OnyxCache { /** Cache of all the storage keys available in persistent storage */ - storageKeys: Set; + private storageKeys: Set; + + /** A list of keys where a nullish value has been fetched from storage before, but the key still exists in cache */ + private nullishStorageKeys: Set; /** Unique list of keys maintained in access order (most recent at the end) */ private recentKeys: Set; @@ -28,6 +31,7 @@ class OnyxCache { constructor() { this.storageKeys = new Set(); + this.nullishStorageKeys = new Set(); this.recentKeys = new Set(); this.storageMap = {}; this.pendingPromises = new Map(); @@ -36,9 +40,11 @@ class OnyxCache { bindAll( this, 'getAllKeys', - 'getValue', + 'get', 'hasCacheForKey', 'addKey', + 'addNullishStorageKey', + 'clearNullishStorageKeys', 'set', 'drop', 'merge', @@ -57,19 +63,18 @@ class OnyxCache { } /** - * Get a cached value from storage - * @param [shouldReindexCache] – This is an LRU cache, and by default accessing a value will make it become last in line to be evicted. This flag can be used to skip that and just access the value directly without side-effects. + * Allows to set all the keys at once. + * This is useful when we are getting + * all the keys from the storage provider + * and we want to keep the cache in sync. + * + * Previously, we had to call `addKey` in a loop + * to achieve the same result. + * + * @param keys - an array of keys */ - getValue(key: OnyxKey, shouldReindexCache = true): OnyxValue { - if (shouldReindexCache) { - this.addToAccessedKeys(key); - } - return this.storageMap[key]; - } - - /** Check whether cache has data for the given key */ - hasCacheForKey(key: OnyxKey): boolean { - return this.storageMap[key] !== undefined; + setAllKeys(keys: OnyxKey[]) { + this.storageKeys = new Set(keys); } /** Saves a key in the storage keys list @@ -79,6 +84,32 @@ class OnyxCache { this.storageKeys.add(key); } + /** Used to set keys that are null/undefined in storage without adding null to the storage map */ + addNullishStorageKey(key: OnyxKey): void { + this.nullishStorageKeys.add(key); + } + + /** Used to clear keys that are null/undefined in cache */ + clearNullishStorageKeys(): void { + this.nullishStorageKeys = new Set(); + } + + /** Check whether cache has data for the given key */ + hasCacheForKey(key: OnyxKey): boolean { + return this.storageMap[key] !== undefined || this.nullishStorageKeys.has(key); + } + + /** + * Get a cached value from storage + * @param [shouldReindexCache] – This is an LRU cache, and by default accessing a value will make it become last in line to be evicted. This flag can be used to skip that and just access the value directly without side-effects. + */ + get(key: OnyxKey, shouldReindexCache = true): OnyxValue { + if (shouldReindexCache) { + this.addToAccessedKeys(key); + } + return this.storageMap[key]; + } + /** * Set's a key value in cache * Adds the key to the storage keys list as well @@ -86,6 +117,16 @@ class OnyxCache { set(key: OnyxKey, value: OnyxValue): OnyxValue { this.addKey(key); this.addToAccessedKeys(key); + + // When a key is explicitly set in cache, we can remove it from the list of nullish keys, + // since it will either be set to a non nullish value or removed from the cache completely. + this.nullishStorageKeys.delete(key); + + if (value === null || value === undefined) { + delete this.storageMap[key]; + return undefined; + } + this.storageMap[key] = value; return value; @@ -107,27 +148,18 @@ class OnyxCache { throw new Error('data passed to cache.merge() must be an Object of onyx key/value pairs'); } - this.storageMap = {...utils.fastMerge(this.storageMap, data, false)}; + this.storageMap = {...utils.fastMerge(this.storageMap, data)}; - const storageKeys = this.getAllKeys(); - const mergedKeys = Object.keys(data); - this.storageKeys = new Set([...storageKeys, ...mergedKeys]); - mergedKeys.forEach((key) => this.addToAccessedKeys(key)); - } + Object.entries(data).forEach(([key, value]) => { + this.addKey(key); + this.addToAccessedKeys(key); - /** - * Allows to set all the keys at once. - * This is useful when we are getting - * all the keys from the storage provider - * and we want to keep the cache in sync. - * - * Previously, we had to call `addKey` in a loop - * to achieve the same result. - * - * @param keys - an array of keys - */ - setAllKeys(keys: OnyxKey[]) { - this.storageKeys = new Set(keys); + if (value === null || value === undefined) { + this.addNullishStorageKey(key); + } else { + this.nullishStorageKeys.delete(key); + } + }); } /** diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index 68054766..7abdeeaa 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -209,7 +209,7 @@ function reduceCollectionWithSelector> { // When we already have the value in cache - resolve right away if (cache.hasCacheForKey(key)) { - return Promise.resolve(cache.getValue(key)); + return Promise.resolve(cache.get(key)); } const taskName = `get:${key}`; @@ -222,6 +222,11 @@ function get(key: OnyxKey): Promise> { // Otherwise retrieve the value from storage and capture a promise to aid concurrent usages const promise = Storage.getItem(key) .then((val) => { + if (val === undefined) { + cache.addNullishStorageKey(key); + return undefined; + } + cache.set(key, val); return val; }) @@ -233,9 +238,9 @@ function get(key: OnyxKey): Promise> { /** Returns current key names stored in persisted storage */ function getAllKeys(): Promise> { // When we've already read stored keys, resolve right away - const storedKeys = cache.getAllKeys(); - if (storedKeys.size > 0) { - return Promise.resolve(storedKeys); + const cachedKeys = cache.getAllKeys(); + if (cachedKeys.size > 0) { + return Promise.resolve(cachedKeys); } const taskName = 'getAllKeys'; @@ -248,6 +253,7 @@ function getAllKeys(): Promise> { // Otherwise retrieve the keys from storage and capture a promise to aid concurrent usages const promise = Storage.getAllKeys().then((keys) => { cache.setAllKeys(keys); + // return the updated set of keys return cache.getAllKeys(); }); @@ -300,7 +306,7 @@ function isSafeEvictionKey(testKey: OnyxKey): boolean { * If the requested key is a collection, it will return an object with all the collection members. */ function tryGetCachedValue(key: TKey, mapping?: Partial>): OnyxValue { - let val = cache.getValue(key); + let val = cache.get(key); if (isCollectionKey(key)) { const allCacheKeys = cache.getAllKeys(); @@ -313,7 +319,7 @@ function tryGetCachedValue(key: TKey, mapping?: Partial k.startsWith(key)); const values = matchingKeys.reduce((finalObject: NonNullable>, matchedKey) => { - const cachedValue = cache.getValue(matchedKey); + const cachedValue = cache.get(matchedKey); if (cachedValue) { // This is permissible because we're in the process of constructing the final object in a reduce function. // eslint-disable-next-line no-param-reassign @@ -414,7 +420,7 @@ function getCachedCollection(collectionKey: TKey return; } - collection[key] = cache.getValue(key); + collection[key] = cache.get(key); }); return collection; @@ -426,22 +432,21 @@ function getCachedCollection(collectionKey: TKey function keysChanged( collectionKey: TKey, partialCollection: OnyxCollection, - previousPartialCollection: OnyxCollection | undefined, + partialPreviousCollection: OnyxCollection | undefined, notifyRegularSubscibers = true, notifyWithOnyxSubscibers = true, ): void { - const previousCollectionWithoutNestedNulls = previousPartialCollection === undefined ? {} : (utils.removeNestedNullValues(previousPartialCollection) as Record); - // We prepare the "cached collection" which is the entire collection + the new partial data that // was merged in via mergeCollection(). const cachedCollection = getCachedCollection(collectionKey); - const cachedCollectionWithoutNestedNulls = utils.removeNestedNullValues(cachedCollection) as Record; // If the previous collection equals the new collection then we do not need to notify any subscribers. - if (previousPartialCollection !== undefined && deepEqual(cachedCollectionWithoutNestedNulls, previousCollectionWithoutNestedNulls)) { + if (partialPreviousCollection !== undefined && deepEqual(cachedCollection, partialPreviousCollection)) { return; } + const previousCollection = partialPreviousCollection ?? {}; + // We are iterating over all subscribers similar to keyChanged(). However, we are looking for subscribers who are subscribing to either a collection key or // individual collection key member for the collection that is being updated. It is important to note that the collection parameter cane be a PARTIAL collection // and does not represent all of the combined keys and values for a collection key. It is just the "new" data that was merged in via mergeCollection(). @@ -477,7 +482,7 @@ function keysChanged( // send the whole cached collection. if (isSubscribedToCollectionKey) { if (subscriber.waitForCollectionCallback) { - subscriber.callback(cachedCollectionWithoutNestedNulls); + subscriber.callback(cachedCollection); continue; } @@ -487,11 +492,11 @@ function keysChanged( for (let j = 0; j < dataKeys.length; j++) { const dataKey = dataKeys[j]; - if (deepEqual(cachedCollectionWithoutNestedNulls[dataKey], previousCollectionWithoutNestedNulls[dataKey])) { + if (deepEqual(cachedCollection[dataKey], previousCollection[dataKey])) { continue; } - subscriber.callback(cachedCollectionWithoutNestedNulls[dataKey], dataKey); + subscriber.callback(cachedCollection[dataKey], dataKey); } continue; } @@ -499,12 +504,12 @@ function keysChanged( // And if the subscriber is specifically only tracking a particular collection member key then we will // notify them with the cached data for that key only. if (isSubscribedToCollectionMemberKey) { - if (deepEqual(cachedCollectionWithoutNestedNulls[subscriber.key], previousCollectionWithoutNestedNulls[subscriber.key])) { + if (deepEqual(cachedCollection[subscriber.key], previousCollection[subscriber.key])) { continue; } const subscriberCallback = subscriber.callback as DefaultConnectCallback; - subscriberCallback(cachedCollectionWithoutNestedNulls[subscriber.key], subscriber.key as TKey); + subscriberCallback(cachedCollection[subscriber.key], subscriber.key as TKey); continue; } @@ -556,7 +561,7 @@ function keysChanged( // If a React component is only interested in a single key then we can set the cached value directly to the state name. if (isSubscribedToCollectionMemberKey) { - if (deepEqual(cachedCollectionWithoutNestedNulls[subscriber.key], previousCollectionWithoutNestedNulls[subscriber.key])) { + if (deepEqual(cachedCollection[subscriber.key], previousCollection[subscriber.key])) { continue; } @@ -647,16 +652,14 @@ function keyChanged( } if (isCollectionKey(subscriber.key) && subscriber.waitForCollectionCallback) { const cachedCollection = getCachedCollection(subscriber.key); - const cachedCollectionWithoutNestedNulls = utils.removeNestedNullValues(cachedCollection) as Record; - cachedCollectionWithoutNestedNulls[key] = value; - subscriber.callback(cachedCollectionWithoutNestedNulls); + cachedCollection[key] = value; + subscriber.callback(cachedCollection); continue; } - const valueWithoutNestedNulls = utils.removeNestedNullValues(value); const subscriberCallback = subscriber.callback as DefaultConnectCallback; - subscriberCallback(valueWithoutNestedNulls, key); + subscriberCallback(value, key); continue; } @@ -752,7 +755,7 @@ function keyChanged( * - sets state on the withOnyxInstances * - triggers the callback function */ -function sendDataToConnection(mapping: Mapping, val: OnyxValue, matchedKey: TKey | undefined, isBatched: boolean): void { +function sendDataToConnection(mapping: Mapping, value: OnyxValue | null, matchedKey: TKey | undefined, isBatched: boolean): void { // If the mapping no longer exists then we should not send any data. // This means our subscriber disconnected or withOnyx wrapped component unmounted. if (!callbackToStateMapping[mapping.connectionID]) { @@ -760,15 +763,15 @@ function sendDataToConnection(mapping: Mapping, val: } if ('withOnyxInstance' in mapping && mapping.withOnyxInstance) { - let newData: OnyxValue = val; + let newData: OnyxValue = value; // If the mapping has a selector, then the component's state must only be updated with the data // returned by the selector. if (mapping.selector) { if (isCollectionKey(mapping.key)) { - newData = reduceCollectionWithSelector(val as OnyxCollection, mapping.selector, mapping.withOnyxInstance.state); + newData = reduceCollectionWithSelector(value as OnyxCollection, mapping.selector, mapping.withOnyxInstance.state); } else { - newData = mapping.selector(val, mapping.withOnyxInstance.state); + newData = mapping.selector(value, mapping.withOnyxInstance.state); } } @@ -781,8 +784,13 @@ function sendDataToConnection(mapping: Mapping, val: return; } - const valuesWithoutNestedNulls = utils.removeNestedNullValues(val); - (mapping as DefaultConnectOptions).callback?.(valuesWithoutNestedNulls, matchedKey as TKey); + // When there are no matching keys in "Onyx.connect", we pass null to "sendDataToConnection" explicitly, + // to allow the withOnyx instance to set the value in the state initially and therefore stop the loading state once all + // required keys have been set. + // If we would pass undefined to setWithOnyxInstance instead, withOnyx would not set the value in the state. + // withOnyx will internally replace null values with undefined and never pass null values to wrapped components. + // For regular callbacks, we never want to pass null values, but always just undefined if a value is not set in cache or storage. + (mapping as DefaultConnectOptions).callback?.(value === null ? undefined : value, matchedKey as TKey); } /** @@ -830,7 +838,7 @@ function getCollectionDataAndSendAsObject(matchingKeys: Co * These missingKeys will be later to use to multiGet the data from the storage. */ matchingKeys.forEach((key) => { - const cacheValue = cache.getValue(key) as OnyxValue; + const cacheValue = cache.get(key) as OnyxValue; if (cacheValue) { data[key] = cacheValue; return; @@ -918,7 +926,7 @@ function scheduleNotifyCollectionSubscribers( * Remove a key from Onyx and update the subscribers */ function remove(key: TKey): Promise { - const prevValue = cache.getValue(key, false) as OnyxValue; + const prevValue = cache.get(key, false) as OnyxValue; cache.drop(key); scheduleSubscriberUpdate(key, null as OnyxValue, prevValue); return Storage.removeItem(key).then(() => undefined); @@ -972,12 +980,12 @@ function evictStorageAndRetry(key: TKey, value: OnyxValue, hasChanged?: boolean, wasRemoved = false): Promise { - const prevValue = cache.getValue(key, false) as OnyxValue; +function broadcastUpdate(key: TKey, value: OnyxValue, hasChanged?: boolean): Promise { + const prevValue = cache.get(key, false) as OnyxValue; // Update subscribers if the cached value has changed, or when the subscriber specifically requires // all updates regardless of value changes (indicated by initWithStoredValues set to false). - if (hasChanged && !wasRemoved) { + if (hasChanged) { cache.set(key, value); } else { cache.addToAccessedKeys(key); @@ -990,8 +998,8 @@ function hasPendingMergeForKey(key: OnyxKey): boolean { return !!mergeQueue[key]; } -type RemoveNullValuesOutput = { - value: Record | unknown[] | null; +type RemoveNullValuesOutput | null> = { + value: Value | null; wasRemoved: boolean; }; @@ -1002,16 +1010,16 @@ type RemoveNullValuesOutput = { * * @returns The value without null values and a boolean "wasRemoved", which indicates if the key got removed completely */ -function removeNullValues(key: OnyxKey, value: OnyxValue, shouldRemoveNestedNulls = true): RemoveNullValuesOutput { +function removeNullValues>(key: OnyxKey, value: Value | null, shouldRemoveNestedNulls = true): RemoveNullValuesOutput { if (value === null) { remove(key); - return {value, wasRemoved: true}; + return {value: null, wasRemoved: true}; } // We can remove all null values in an object by merging it with itself // utils.fastMerge recursively goes through the object and removes all null values // Passing two identical objects as source and target to fastMerge will not change it, but only remove the null values - return {value: shouldRemoveNestedNulls ? utils.removeNestedNullValues(value as Record) : (value as Record), wasRemoved: false}; + return {value: shouldRemoveNestedNulls ? utils.removeNestedNullValues(value) : value, wasRemoved: false}; } /** diff --git a/lib/storage/providers/IDBKeyValProvider.ts b/lib/storage/providers/IDBKeyValProvider.ts index f3539b50..9b749ab8 100644 --- a/lib/storage/providers/IDBKeyValProvider.ts +++ b/lib/storage/providers/IDBKeyValProvider.ts @@ -23,7 +23,13 @@ const provider: StorageProvider = { idbKeyValStore = newIdbKeyValStore; }, - setItem: (key, value) => set(key, value, idbKeyValStore), + setItem: (key, value) => { + if (value === null) { + provider.removeItem(key); + } + + return set(key, value, idbKeyValStore); + }, multiGet: (keysParam) => getMany(keysParam, idbKeyValStore).then((values) => values.map((value, index) => [keysParam[index], value])), multiMerge: (pairs) => idbKeyValStore('readwrite', (store) => { @@ -32,7 +38,16 @@ const provider: StorageProvider = { const getValues = Promise.all(pairs.map(([key]) => promisifyRequest>(store.get(key)))); return getValues.then((values) => { - const upsertMany = pairs.map(([key, value], index) => { + 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 promisifyRequest(store.put(newValue, key)); @@ -44,7 +59,18 @@ const provider: StorageProvider = { // Since Onyx also merged the existing value with the changes, we can just set the value directly return provider.setItem(key, preMergedValue); }, - multiSet: (pairs) => setMany(pairs, idbKeyValStore), + multiSet: (pairs) => { + const pairsWithoutNull = pairs.filter(([key, value]) => { + if (value === null) { + provider.removeItem(key); + return false; + } + + return true; + }); + + return setMany(pairsWithoutNull, idbKeyValStore); + }, clear: () => clear(idbKeyValStore), getAllKeys: () => keys(idbKeyValStore), getItem: (key) => diff --git a/lib/types.ts b/lib/types.ts index 7ea33e52..28f98a2b 100644 --- a/lib/types.ts +++ b/lib/types.ts @@ -178,7 +178,7 @@ type Selector = (value: OnyxEntry * })(Component); * ``` */ -type OnyxEntry = TOnyxValue | null | undefined; +type OnyxEntry = TOnyxValue | undefined; /** * Represents an Onyx collection of entries, that can be either a record of `TOnyxValue`s or `null` / `undefined` if it is empty or doesn't exist. @@ -208,7 +208,7 @@ type OnyxEntry = TOnyxValue | null | undefined; * })(Component); * ``` */ -type OnyxCollection = OnyxEntry>; +type OnyxCollection = OnyxEntry>; /** Utility type to extract `TOnyxValue` from `OnyxCollection` */ type ExtractOnyxCollectionValue = TOnyxCollection extends NonNullable> ? U : never; diff --git a/lib/utils.ts b/lib/utils.ts index fb64cea7..15faa4ac 100644 --- a/lib/utils.ts +++ b/lib/utils.ts @@ -1,6 +1,6 @@ /* eslint-disable @typescript-eslint/prefer-for-of */ -import type {OnyxKey} from './types'; +import type {OnyxKey, OnyxValue} from './types'; type EmptyObject = Record; type EmptyValue = EmptyObject | null | undefined; @@ -104,12 +104,13 @@ function fastMerge>(target: TObject | nu } /** Deep removes the nested null values from the given value. */ -function removeNestedNullValues>(value: unknown | unknown[] | TObject): Record | unknown[] | null { +function removeNestedNullValues | unknown | unknown[]>(value: TValue | null): TValue | null { if (typeof value === 'object' && !Array.isArray(value)) { - return fastMerge(value as Record | null, value as Record | null); + const objectValue = value as Record | null; + return fastMerge(objectValue, objectValue) as TValue | null; } - return value as Record | null; + return value; } /** Formats the action name by uppercasing and adding the key if provided. */ diff --git a/lib/withOnyx/index.tsx b/lib/withOnyx/index.tsx index 3245bc8d..8ecb9545 100644 --- a/lib/withOnyx/index.tsx +++ b/lib/withOnyx/index.tsx @@ -10,6 +10,7 @@ import * as Str from '../Str'; import type {GenericFunction, OnyxKey, WithOnyxConnectOptions} from '../types'; import utils from '../utils'; import type {MapOnyxToState, WithOnyxInstance, WithOnyxMapping, WithOnyxProps, WithOnyxState} from './types'; +import cache from '../OnyxCache'; // This is a list of keys that can exist on a `mapping`, but are not directly related to loading data from Onyx. When the keys of a mapping are looped over to check // if a key has changed, it's a good idea to skip looking at these properties since they would have unexpected results. @@ -84,7 +85,9 @@ export default function ( const cachedState = mapOnyxToStateEntries(mapOnyxToState).reduce>((resultObj, [propName, mapping]) => { const key = Str.result(mapping.key as GenericFunction, props); let value = OnyxUtils.tryGetCachedValue(key, mapping as Partial>); - if (!value && mapping.initialValue) { + const hasCacheForKey = cache.hasCacheForKey(key); + + if (!hasCacheForKey && !value && mapping.initialValue) { value = mapping.initialValue; } @@ -99,7 +102,11 @@ export default function ( * In reality, Onyx.merge() will only update the subscriber after all merges have been batched and the previous value is retrieved via a get() (returns a promise). * So, we won't use the cache optimization here as it will lead us to arbitrarily defer various actions in the application code. */ - if (mapping.initWithStoredValues !== false && ((value !== undefined && !OnyxUtils.hasPendingMergeForKey(key)) || mapping.allowStaleData)) { + const hasPendingMergeForKey = OnyxUtils.hasPendingMergeForKey(key); + const hasValueInCache = hasCacheForKey || value !== undefined; + const shouldSetState = mapping.initWithStoredValues !== false && ((hasValueInCache && !hasPendingMergeForKey) || !!mapping.allowStaleData); + + if (shouldSetState) { // eslint-disable-next-line no-param-reassign resultObj[propName] = value as WithOnyxState[keyof TOnyxProps]; } @@ -197,7 +204,7 @@ export default function ( * initialValue is there, we just check if the update is different than that and then try to handle it as best as we can. */ setWithOnyxState(statePropertyName: T, val: WithOnyxState[T]) { - const prevValue = this.state[statePropertyName]; + const prevVal = this.state[statePropertyName]; // If the component is not loading (read "mounting"), then we can just update the state // There is a small race condition. @@ -208,11 +215,13 @@ export default function ( // This simply bypasses the loading check if the tempState is gone and the update can be safely queued with a normal setStateProxy. if (!this.state.loading || !this.tempState) { // Performance optimization, do not trigger update with same values - if (prevValue === val || (utils.isEmptyObject(prevValue) && utils.isEmptyObject(val))) { + if (prevVal === val || (utils.isEmptyObject(prevVal) && utils.isEmptyObject(val))) { return; } - this.setStateProxy({[statePropertyName]: val} as WithOnyxState); + const valueWithoutNull = val === null ? undefined : val; + + this.setStateProxy({[statePropertyName]: valueWithoutNull} as WithOnyxState); return; } @@ -240,15 +249,14 @@ export default function ( // If initialValue is there and the state contains something different it means // an update has already been received and we can discard the value we are trying to hydrate - if (initialValue !== undefined && prevState[key] !== undefined && prevState[key] !== initialValue) { + if (initialValue !== undefined && prevState[key] !== undefined && prevState[key] !== initialValue && prevState[key] !== null) { // eslint-disable-next-line no-param-reassign result[key] = prevState[key]; - + } else if (prevState[key] !== undefined && prevState[key] !== null) { // if value is already there (without initial value) then we can discard the value we are trying to hydrate - } else if (prevState[key] !== undefined) { // eslint-disable-next-line no-param-reassign result[key] = prevState[key]; - } else { + } else if (stateUpdate[key] !== null) { // eslint-disable-next-line no-param-reassign result[key] = stateUpdate[key]; } @@ -344,7 +352,6 @@ export default function ( // Remove any internal state properties used by withOnyx // that should not be passed to a wrapped component const stateToPass = utils.omit(this.state as WithOnyxState, ([stateKey, stateValue]) => stateKey === 'loading' || stateValue === null); - const stateToPassWithoutNestedNulls = utils.removeNestedNullValues(stateToPass); // Spreading props and state is necessary in an HOC where the data cannot be predicted return ( @@ -353,7 +360,7 @@ export default function ( // eslint-disable-next-line react/jsx-props-no-spreading {...(propsToPass as TComponentProps)} // eslint-disable-next-line react/jsx-props-no-spreading - {...stateToPassWithoutNestedNulls} + {...stateToPass} ref={this.props.forwardedRef} /> ); diff --git a/tests/unit/onyxCacheTest.tsx b/tests/unit/onyxCacheTest.tsx index 28ca1fa7..9487b0b4 100644 --- a/tests/unit/onyxCacheTest.tsx +++ b/tests/unit/onyxCacheTest.tsx @@ -79,7 +79,7 @@ describe('Onyx', () => { // Given empty cache // When a value is retrieved - const result = cache.getValue('mockKey'); + const result = cache.get('mockKey'); // Then it should be undefined expect(result).not.toBeDefined(); @@ -92,8 +92,8 @@ describe('Onyx', () => { // When a value is retrieved // Then it should be the correct value - expect(cache.getValue('mockKey')).toEqual({items: ['mockValue', 'mockValue2']}); - expect(cache.getValue('mockKey2')).toEqual('mockValue3'); + expect(cache.get('mockKey')).toEqual({items: ['mockValue', 'mockValue2']}); + expect(cache.get('mockKey2')).toEqual('mockValue3'); }); }); @@ -155,7 +155,7 @@ describe('Onyx', () => { cache.set('mockKey', {value: 'mockValue'}); // Then data should be cached - const data = cache.getValue('mockKey'); + const data = cache.get('mockKey'); expect(data).toEqual({value: 'mockValue'}); }); @@ -178,7 +178,7 @@ describe('Onyx', () => { cache.set('mockKey2', {value: []}); // Then the value should be overwritten - expect(cache.getValue('mockKey2')).toEqual({value: []}); + expect(cache.get('mockKey2')).toEqual({value: []}); }); }); @@ -193,7 +193,7 @@ describe('Onyx', () => { // Then a value should not be available in cache expect(cache.hasCacheForKey('mockKey')).toBe(false); - expect(cache.getValue('mockKey')).not.toBeDefined(); + expect(cache.get('mockKey')).not.toBeDefined(); expect(cache.getAllKeys().has('mockKey')).toBe(false); }); }); @@ -209,8 +209,8 @@ describe('Onyx', () => { }); // Then data should be created in cache - expect(cache.getValue('mockKey')).toEqual({value: 'mockValue'}); - expect(cache.getValue('mockKey2')).toEqual({value: 'mockValue2'}); + expect(cache.get('mockKey')).toEqual({value: 'mockValue'}); + expect(cache.get('mockKey2')).toEqual({value: 'mockValue2'}); }); it('Should merge data to existing cache value', () => { @@ -225,12 +225,12 @@ describe('Onyx', () => { }); // Then the values should be merged together in cache - expect(cache.getValue('mockKey')).toEqual({ + expect(cache.get('mockKey')).toEqual({ value: 'mockValue', mockItems: [], }); - expect(cache.getValue('mockKey2')).toEqual({ + expect(cache.get('mockKey2')).toEqual({ other: 'overwrittenMockValue', items: [1, 2], mock: 'mock', @@ -247,7 +247,7 @@ describe('Onyx', () => { }); // Then the values should be merged together in cache - expect(cache.getValue('mockKey')).toEqual({ + expect(cache.get('mockKey')).toEqual({ value: 'mockValue', mockItems: [], otherValue: 'overwritten', @@ -264,7 +264,7 @@ describe('Onyx', () => { }); // Then the arrays should be replaced as expected - expect(cache.getValue('mockKey')).toEqual([{ID: 3}, {added: 'field'}, {}, {ID: 1000}]); + expect(cache.get('mockKey')).toEqual([{ID: 3}, {added: 'field'}, {}, {ID: 1000}]); }); it('Should merge arrays inside objects correctly', () => { @@ -277,7 +277,7 @@ describe('Onyx', () => { }); // Then the first array is completely replaced by the second array - expect(cache.getValue('mockKey')).toEqual({ID: [2]}); + expect(cache.get('mockKey')).toEqual({ID: [2]}); }); it('Should work with primitive values', () => { @@ -288,31 +288,31 @@ describe('Onyx', () => { cache.merge({mockKey: false}); // Then the object should be overwritten with a bool value - expect(cache.getValue('mockKey')).toEqual(false); + expect(cache.get('mockKey')).toEqual(false); // When merge is called with number cache.merge({mockKey: 0}); // Then the value should be overwritten - expect(cache.getValue('mockKey')).toEqual(0); + expect(cache.get('mockKey')).toEqual(0); // When merge is called with string cache.merge({mockKey: '123'}); // Then the value should be overwritten - expect(cache.getValue('mockKey')).toEqual('123'); + expect(cache.get('mockKey')).toEqual('123'); // When merge is called with string again cache.merge({mockKey: '123'}); // Then strings should not have been concatenated - expect(cache.getValue('mockKey')).toEqual('123'); + expect(cache.get('mockKey')).toEqual('123'); // When merge is called with an object cache.merge({mockKey: {value: 'myMockObject'}}); // Then the old primitive value should be overwritten with the object - expect(cache.getValue('mockKey')).toEqual({value: 'myMockObject'}); + expect(cache.get('mockKey')).toEqual({value: 'myMockObject'}); }); it('Should ignore `undefined` values', () => { @@ -323,12 +323,12 @@ describe('Onyx', () => { cache.merge({mockKey: {ID: undefined}}); // Then the key should still be in cache and the value unchanged - expect(cache.getValue('mockKey')).toEqual({ID: 5}); + expect(cache.get('mockKey')).toEqual({ID: 5}); cache.merge({mockKey: undefined}); // Then the key should still be in cache and the value unchanged - expect(cache.getValue('mockKey')).toEqual({ID: 5}); + expect(cache.get('mockKey')).toEqual({ID: 5}); }); it('Should update storageKeys when new keys are created', () => { @@ -357,16 +357,14 @@ describe('Onyx', () => { expect(() => cache.merge({})).not.toThrow(); }); - it('Should merge `null` values', () => { - // `null` values can override existing values and should also - // be preserved during merges. + it('Should remove `null` values when merging', () => { cache.set('mockKey', {ID: 5}); cache.set('mockNullKey', null); cache.merge({mockKey: null}); - expect(cache.getValue('mockKey')).toEqual(null); - expect(cache.getValue('mockNullKey')).toEqual(null); + expect(cache.get('mockKey')).toEqual(undefined); + expect(cache.get('mockNullKey')).toEqual(undefined); }); }); diff --git a/tests/unit/onyxClearNativeStorageTest.ts b/tests/unit/onyxClearNativeStorageTest.ts index 090a7f78..28605adc 100644 --- a/tests/unit/onyxClearNativeStorageTest.ts +++ b/tests/unit/onyxClearNativeStorageTest.ts @@ -54,7 +54,7 @@ describe('Set data while storage is clearing', () => { return waitForPromisesToResolve().then(() => { // Then the value in Onyx, the cache, and Storage is the default key state expect(onyxValue).toBe(DEFAULT_VALUE); - const cachedValue = cache.getValue(ONYX_KEYS.DEFAULT_KEY); + const cachedValue = cache.get(ONYX_KEYS.DEFAULT_KEY); expect(cachedValue).toBe(DEFAULT_VALUE); return StorageMock.getItem(ONYX_KEYS.DEFAULT_KEY).then((storedValue) => expect(parseInt(storedValue as string, 10)).toBe(DEFAULT_VALUE)); }); @@ -70,7 +70,7 @@ describe('Set data while storage is clearing', () => { return waitForPromisesToResolve().then(() => { // Then the value in Onyx, the cache, and Storage is the default key state expect(onyxValue).toBe(DEFAULT_VALUE); - const cachedValue = cache.getValue(ONYX_KEYS.DEFAULT_KEY); + const cachedValue = cache.get(ONYX_KEYS.DEFAULT_KEY); expect(cachedValue).toBe(DEFAULT_VALUE); return StorageMock.getItem(ONYX_KEYS.DEFAULT_KEY).then((storedValue) => expect(parseInt(storedValue as string, 10)).toBe(DEFAULT_VALUE)); }); @@ -94,7 +94,7 @@ describe('Set data while storage is clearing', () => { return waitForPromisesToResolve().then(() => { // Then the value of the preserved key is also still set in both the cache and storage expect(regularKeyOnyxValue).toBe(SET_VALUE); - expect(cache.getValue(ONYX_KEYS.REGULAR_KEY)).toBe(SET_VALUE); + expect(cache.get(ONYX_KEYS.REGULAR_KEY)).toBe(SET_VALUE); return StorageMock.getItem(ONYX_KEYS.REGULAR_KEY).then((storedValue) => expect(parseInt(storedValue as string, 10)).toBe(SET_VALUE)); }); }); @@ -111,7 +111,7 @@ describe('Set data while storage is clearing', () => { return waitForPromisesToResolve().then(() => { // Then the value in Onyx, the cache, and Storage is the set value expect(onyxValue).toBe(SET_VALUE); - const cachedValue = cache.getValue(ONYX_KEYS.DEFAULT_KEY); + const cachedValue = cache.get(ONYX_KEYS.DEFAULT_KEY); expect(cachedValue).toBe(SET_VALUE); return StorageMock.getItem(ONYX_KEYS.DEFAULT_KEY).then((storedValue) => expect(parseInt(storedValue as string, 10)).toBe(SET_VALUE)); }); diff --git a/tests/unit/onyxClearWebStorageTest.ts b/tests/unit/onyxClearWebStorageTest.ts index d9117612..6fb52fbc 100644 --- a/tests/unit/onyxClearWebStorageTest.ts +++ b/tests/unit/onyxClearWebStorageTest.ts @@ -56,7 +56,7 @@ describe('Set data while storage is clearing', () => { return waitForPromisesToResolve().then(() => { // Then the value in Onyx, the cache, and Storage is the default key state expect(onyxValue).toBe(DEFAULT_VALUE); - const cachedValue = cache.getValue(ONYX_KEYS.DEFAULT_KEY); + const cachedValue = cache.get(ONYX_KEYS.DEFAULT_KEY); expect(cachedValue).toBe(DEFAULT_VALUE); const storedValue = StorageMock.getItem(ONYX_KEYS.DEFAULT_KEY); return expect(storedValue).resolves.toBe(DEFAULT_VALUE); @@ -73,7 +73,7 @@ describe('Set data while storage is clearing', () => { return waitForPromisesToResolve().then(() => { // Then the value in Onyx, the cache, and Storage is the default key state expect(onyxValue).toBe(DEFAULT_VALUE); - const cachedValue = cache.getValue(ONYX_KEYS.DEFAULT_KEY); + const cachedValue = cache.get(ONYX_KEYS.DEFAULT_KEY); expect(cachedValue).toBe(DEFAULT_VALUE); const storedValue = StorageMock.getItem(ONYX_KEYS.DEFAULT_KEY); return expect(storedValue).resolves.toBe(DEFAULT_VALUE); @@ -98,7 +98,7 @@ describe('Set data while storage is clearing', () => { return waitForPromisesToResolve().then(() => { // Then the value of the preserved key is also still set in both the cache and storage expect(regularKeyOnyxValue).toBe(SET_VALUE); - const regularKeyCachedValue = cache.getValue(ONYX_KEYS.REGULAR_KEY); + const regularKeyCachedValue = cache.get(ONYX_KEYS.REGULAR_KEY); expect(regularKeyCachedValue).toBe(SET_VALUE); const regularKeyStoredValue = StorageMock.getItem(ONYX_KEYS.REGULAR_KEY); return expect(regularKeyStoredValue).resolves.toBe(SET_VALUE); @@ -117,7 +117,7 @@ describe('Set data while storage is clearing', () => { return waitForPromisesToResolve().then(() => { // Then the value in Onyx, the cache, and Storage is the set value expect(onyxValue).toBe(SET_VALUE); - const cachedValue = cache.getValue(ONYX_KEYS.DEFAULT_KEY); + const cachedValue = cache.get(ONYX_KEYS.DEFAULT_KEY); expect(cachedValue).toBe(SET_VALUE); const storedValue = StorageMock.getItem(ONYX_KEYS.DEFAULT_KEY); return expect(storedValue).resolves.toBe(SET_VALUE); @@ -158,7 +158,7 @@ describe('Set data while storage is clearing', () => { expect(collectionCallback).toHaveBeenCalledTimes(3); // And it should be called with the expected parameters each time - expect(collectionCallback).toHaveBeenNthCalledWith(1, null, undefined); + expect(collectionCallback).toHaveBeenNthCalledWith(1, undefined, undefined); expect(collectionCallback).toHaveBeenNthCalledWith(2, { test_1: 1, test_2: 2, diff --git a/tests/unit/onyxMultiMergeWebStorageTest.ts b/tests/unit/onyxMultiMergeWebStorageTest.ts index a9984caa..c3f939c7 100644 --- a/tests/unit/onyxMultiMergeWebStorageTest.ts +++ b/tests/unit/onyxMultiMergeWebStorageTest.ts @@ -45,9 +45,9 @@ describe('Onyx.mergeCollection() and WebStorage', () => { expect(StorageMock.getMockStore().test_3).toEqual(initialTestObject); // And an empty cache values for the collection keys - expect(OnyxCache.getValue('test_1')).not.toBeDefined(); - expect(OnyxCache.getValue('test_2')).not.toBeDefined(); - expect(OnyxCache.getValue('test_3')).not.toBeDefined(); + expect(OnyxCache.get('test_1')).not.toBeDefined(); + expect(OnyxCache.get('test_2')).not.toBeDefined(); + expect(OnyxCache.get('test_3')).not.toBeDefined(); // When we merge additional data const additionalDataOne = {b: 'b', c: 'c', e: [1, 2]}; @@ -75,9 +75,9 @@ describe('Onyx.mergeCollection() and WebStorage', () => { }; // Then our new data should merge with the existing data in the cache - expect(OnyxCache.getValue('test_1')).toEqual(finalObject); - expect(OnyxCache.getValue('test_2')).toEqual(finalObject); - expect(OnyxCache.getValue('test_3')).toEqual(finalObject); + expect(OnyxCache.get('test_1')).toEqual(finalObject); + expect(OnyxCache.get('test_2')).toEqual(finalObject); + expect(OnyxCache.get('test_3')).toEqual(finalObject); // And the storage should reflect the same state expect(StorageMock.getMockStore().test_1).toEqual(finalObject); @@ -93,9 +93,9 @@ describe('Onyx.mergeCollection() and WebStorage', () => { expect(StorageMock.getMockStore().test_3).toBeFalsy(); // And an empty cache values for the collection keys - expect(OnyxCache.getValue('test_1')).toBeFalsy(); - expect(OnyxCache.getValue('test_2')).toBeFalsy(); - expect(OnyxCache.getValue('test_3')).toBeFalsy(); + expect(OnyxCache.get('test_1')).toBeFalsy(); + expect(OnyxCache.get('test_2')).toBeFalsy(); + expect(OnyxCache.get('test_3')).toBeFalsy(); // When we merge additional data and wait for the change const data = {a: 'a', b: 'b'}; @@ -108,9 +108,9 @@ describe('Onyx.mergeCollection() and WebStorage', () => { return waitForPromisesToResolve() .then(() => { // Then the cache and storage should match - expect(OnyxCache.getValue('test_1')).toEqual(data); - expect(OnyxCache.getValue('test_2')).toEqual(data); - expect(OnyxCache.getValue('test_3')).toEqual(data); + expect(OnyxCache.get('test_1')).toEqual(data); + expect(OnyxCache.get('test_2')).toEqual(data); + expect(OnyxCache.get('test_3')).toEqual(data); expect(StorageMock.getMockStore().test_1).toEqual(data); expect(StorageMock.getMockStore().test_2).toEqual(data); expect(StorageMock.getMockStore().test_3).toEqual(data); @@ -137,9 +137,9 @@ describe('Onyx.mergeCollection() and WebStorage', () => { }; // Then our new data should merge with the existing data in the cache - expect(OnyxCache.getValue('test_1')).toEqual(finalObject); - expect(OnyxCache.getValue('test_2')).toEqual(finalObject); - expect(OnyxCache.getValue('test_3')).toEqual(finalObject); + expect(OnyxCache.get('test_1')).toEqual(finalObject); + expect(OnyxCache.get('test_2')).toEqual(finalObject); + expect(OnyxCache.get('test_3')).toEqual(finalObject); // And the storage should reflect the same state expect(StorageMock.getMockStore().test_1).toEqual(finalObject); @@ -178,7 +178,7 @@ describe('Onyx.mergeCollection() and WebStorage', () => { f: 'f', }; - expect(OnyxCache.getValue('test_1')).toEqual(finalObject); + expect(OnyxCache.get('test_1')).toEqual(finalObject); expect(StorageMock.getMockStore().test_1).toEqual(finalObject); }); }); diff --git a/tests/unit/onyxTest.ts b/tests/unit/onyxTest.ts index 0f4e6784..95c189be 100644 --- a/tests/unit/onyxTest.ts +++ b/tests/unit/onyxTest.ts @@ -816,7 +816,7 @@ describe('Onyx', () => { expect(mockCallback).toHaveBeenCalledTimes(2); // AND the value for the first call should be null since the collection was not initialized at that point - expect(mockCallback).toHaveBeenNthCalledWith(1, null, undefined); + expect(mockCallback).toHaveBeenNthCalledWith(1, undefined, undefined); // AND the value for the second call should be collectionUpdate since the collection was updated expect(mockCallback).toHaveBeenNthCalledWith(2, collectionUpdate); @@ -845,7 +845,7 @@ describe('Onyx', () => { expect(mockCallback).toHaveBeenCalledTimes(2); // AND the value for the first call should be null since the collection was not initialized at that point - expect(mockCallback).toHaveBeenNthCalledWith(1, null, undefined); + expect(mockCallback).toHaveBeenNthCalledWith(1, undefined, undefined); // AND the value for the second call should be collectionUpdate since the collection was updated expect(mockCallback).toHaveBeenNthCalledWith(2, collectionUpdate.testPolicy_1, 'testPolicy_1'); @@ -987,11 +987,11 @@ describe('Onyx', () => { {onyxMethod: Onyx.METHOD.MERGE_COLLECTION, key: ONYX_KEYS.COLLECTION.TEST_UPDATE, value: {[itemKey]: {a: 'a'}}}, ]).then(() => { expect(collectionCallback).toHaveBeenCalledTimes(2); - expect(collectionCallback).toHaveBeenNthCalledWith(1, null, undefined); + expect(collectionCallback).toHaveBeenNthCalledWith(1, undefined, undefined); expect(collectionCallback).toHaveBeenNthCalledWith(2, {[itemKey]: {a: 'a'}}); expect(testCallback).toHaveBeenCalledTimes(2); - expect(testCallback).toHaveBeenNthCalledWith(1, null, undefined); + expect(testCallback).toHaveBeenNthCalledWith(1, undefined, undefined); expect(testCallback).toHaveBeenNthCalledWith(2, 'taco', ONYX_KEYS.TEST_KEY); expect(otherTestCallback).toHaveBeenCalledTimes(2);