From 4e1cb68fa383efbbc2d953010dd491bee07ff305 Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Tue, 4 Jun 2024 14:16:52 +0200 Subject: [PATCH 01/23] fix: check if kez exists rather than undefined --- lib/withOnyx/index.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/withOnyx/index.tsx b/lib/withOnyx/index.tsx index 8ecb9545..fdc5bed5 100644 --- a/lib/withOnyx/index.tsx +++ b/lib/withOnyx/index.tsx @@ -228,7 +228,8 @@ export default function ( this.tempState[statePropertyName] = val; // If some key does not have a value yet, do not update the state yet - const tempStateIsMissingKey = requiredKeysForInit.some((key) => this.tempState?.[key as keyof TOnyxProps] === undefined); + const tempStateIsMissingKey = requiredKeysForInit.some((key) => !(key in (this.tempState ?? {}))); + if (tempStateIsMissingKey) { return; } From 138eca1a1a7897900f4240a78fac83b7b1e12e78 Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Tue, 4 Jun 2024 15:19:59 +0200 Subject: [PATCH 02/23] fix: only add undefined property to collection returned by "getCachedCollection" if it's set as nullish in cache --- lib/OnyxCache.ts | 8 +++++- lib/OnyxUtils.ts | 66 ++++++++++++++++++++++++++++-------------------- 2 files changed, 45 insertions(+), 29 deletions(-) diff --git a/lib/OnyxCache.ts b/lib/OnyxCache.ts index 2d323369..530f8e66 100644 --- a/lib/OnyxCache.ts +++ b/lib/OnyxCache.ts @@ -44,6 +44,7 @@ class OnyxCache { 'hasCacheForKey', 'addKey', 'addNullishStorageKey', + 'hasNullishStorageKey', 'clearNullishStorageKeys', 'set', 'drop', @@ -89,6 +90,11 @@ class OnyxCache { this.nullishStorageKeys.add(key); } + /** Used to set keys that are null/undefined in storage without adding null to the storage map */ + hasNullishStorageKey(key: OnyxKey): boolean { + return this.nullishStorageKeys.has(key); + } + /** Used to clear keys that are null/undefined in cache */ clearNullishStorageKeys(): void { this.nullishStorageKeys = new Set(); @@ -96,7 +102,7 @@ class OnyxCache { /** Check whether cache has data for the given key */ hasCacheForKey(key: OnyxKey): boolean { - return this.storageMap[key] !== undefined || this.nullishStorageKeys.has(key); + return this.storageMap[key] !== undefined || this.hasNullishStorageKey(key); } /** diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index 8403676f..8cd6d475 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -419,6 +419,12 @@ function getCachedCollection(collectionKey: TKey return; } + const cachedValue = cache.get(key); + + if (cachedValue === undefined && !cache.hasNullishStorageKey(key)) { + return; + } + collection[key] = cache.get(key); }); @@ -439,11 +445,6 @@ function keysChanged( // was merged in via mergeCollection(). const cachedCollection = getCachedCollection(collectionKey); - // If the previous collection equals the new collection then we do not need to notify any subscribers. - 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 @@ -532,12 +533,13 @@ function keysChanged( const previousData = prevState[subscriber.statePropertyName]; const newData = reduceCollectionWithSelector(cachedCollection, collectionSelector, subscriber.withOnyxInstance.state); - if (!deepEqual(previousData, newData)) { - return { - [subscriber.statePropertyName]: newData, - }; + if (deepEqual(previousData, newData)) { + return null; } - return null; + + return { + [subscriber.statePropertyName]: newData, + }; }); continue; } @@ -550,6 +552,10 @@ function keysChanged( finalCollection[dataKey] = cachedCollection[dataKey]; } + if (!prevState.loading && deepEqual(cachedCollection, finalCollection)) { + return null; + } + PerformanceUtils.logSetStateCall(subscriber, prevState?.[subscriber.statePropertyName], finalCollection, 'keysChanged', collectionKey); return { [subscriber.statePropertyName]: finalCollection, @@ -579,14 +585,15 @@ function keysChanged( subscriber.withOnyxInstance.setStateProxy((prevState) => { const prevData = prevState[subscriber.statePropertyName]; const newData = selector(cachedCollection[subscriber.key], subscriber.withOnyxInstance.state); - if (!deepEqual(prevData, newData)) { - PerformanceUtils.logSetStateCall(subscriber, prevData, newData, 'keysChanged', collectionKey); - return { - [subscriber.statePropertyName]: newData, - }; + + if (deepEqual(prevData, newData)) { + return null; } - return null; + PerformanceUtils.logSetStateCall(subscriber, prevData, newData, 'keysChanged', collectionKey); + return { + [subscriber.statePropertyName]: newData, + }; }); continue; } @@ -683,13 +690,15 @@ function keyChanged( ...prevWithOnyxData, ...newWithOnyxData, }; - if (!deepEqual(prevWithOnyxData, prevDataWithNewData)) { - PerformanceUtils.logSetStateCall(subscriber, prevWithOnyxData, newWithOnyxData, 'keyChanged', key); - return { - [subscriber.statePropertyName]: prevDataWithNewData, - }; + + if (deepEqual(prevWithOnyxData, prevDataWithNewData)) { + return null; } - return null; + + PerformanceUtils.logSetStateCall(subscriber, prevWithOnyxData, newWithOnyxData, 'keyChanged', key); + return { + [subscriber.statePropertyName]: prevDataWithNewData, + }; }); continue; } @@ -711,16 +720,17 @@ function keyChanged( // If the subscriber has a selector, then the component's state must only be updated with the data // returned by the selector and only if the selected data has changed. if (selector) { - subscriber.withOnyxInstance.setStateProxy(() => { + subscriber.withOnyxInstance.setStateProxy((prevState) => { const prevValue = selector(previousValue, subscriber.withOnyxInstance.state); const newValue = selector(value, subscriber.withOnyxInstance.state); - if (!deepEqual(prevValue, newValue)) { - return { - [subscriber.statePropertyName]: newValue, - }; + if (!prevState.loading && deepEqual(prevValue, newValue)) { + return null; } - return null; + + return { + [subscriber.statePropertyName]: newValue, + }; }); continue; } From da593f163031fcc40d9b89017ffc24a222628d33 Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Tue, 4 Jun 2024 17:02:28 +0200 Subject: [PATCH 03/23] fix: improve types and add undefined handling --- lib/Onyx.ts | 43 +++++++++++++---------------- lib/OnyxUtils.ts | 36 +++++++++++++------------ lib/types.ts | 70 +++++++++++++++++++++++++----------------------- lib/utils.ts | 24 +++++++++-------- 4 files changed, 86 insertions(+), 87 deletions(-) diff --git a/lib/Onyx.ts b/lib/Onyx.ts index 59d89b99..edbf058e 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -13,7 +13,7 @@ import type { InitOptions, KeyValueMapping, Mapping, - NullableKeyValueMapping, + OnyxInputKeyValueMapping, OnyxCollection, OnyxKey, OnyxMergeCollectionInput, @@ -22,6 +22,7 @@ import type { OnyxSetInput, OnyxUpdate, OnyxValue, + OnyxInput, } from './types'; import OnyxUtils from './OnyxUtils'; import logMessages from './logMessages'; @@ -216,10 +217,14 @@ function set(key: TKey, value: OnyxSetInput): Promis delete OnyxUtils.getMergeQueue()[key]; } - const existingValue = cache.get(key, false); + // Onyx.set will ignore `undefined` values as inputs, therefore we can return early. + if (value === undefined) { + return Promise.resolve(); + } + 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) { + if (existingValue === undefined && value === null) { return Promise.resolve(); } @@ -274,21 +279,9 @@ function set(key: TKey, value: OnyxSetInput): Promis * @param data object keyed by ONYXKEYS and the values to set */ function multiSet(data: OnyxMultiSetInput): Promise { - 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 keyValuePairsToSet = OnyxUtils.prepareKeyValuePairsForStorage(data, true); - const updatePromises = keyValuePairsToUpdate.map(([key, value]) => { + const updatePromises = keyValuePairsToSet.map(([key, value]) => { const prevValue = cache.get(key, false); // Update cache and optimistically inform subscribers on the next tick @@ -296,11 +289,11 @@ function multiSet(data: OnyxMultiSetInput): Promise { return OnyxUtils.scheduleSubscriberUpdate(key, value, prevValue); }); - return Storage.multiSet(allKeyValuePairs) + return Storage.multiSet(keyValuePairsToSet) .catch((error) => OnyxUtils.evictStorageAndRetry(error, multiSet, data)) .then(() => { OnyxUtils.sendActionToDevTools(OnyxUtils.METHOD.MULTI_SET, undefined, data); - return Promise.all([removePromises, updatePromises]); + return Promise.all(updatePromises); }) .then(() => undefined); } @@ -354,7 +347,7 @@ function merge(key: TKey, changes: OnyxMergeInput): Logger.logAlert(logMessages.incompatibleUpdateAlert(key, 'merge', existingValueType, newValueType)); } return isCompatible; - }); + }) as Array>; if (!validChanges.length) { return Promise.resolve(); @@ -435,7 +428,7 @@ function mergeCollection(collectionKey: TK Logger.logInfo('mergeCollection() called with invalid or empty value. Skipping this update.'); return Promise.resolve(); } - const mergedCollection: NullableKeyValueMapping = collection; + const mergedCollection: OnyxInputKeyValueMapping = collection; // Confirm all the collection keys belong to the same parent let hasCollectionKeyCheckFailed = false; @@ -474,7 +467,7 @@ function mergeCollection(collectionKey: TK const newKeys = keys.filter((key) => !persistedKeys.has(key)); - const existingKeyCollection = existingKeys.reduce((obj: NullableKeyValueMapping, key) => { + const existingKeyCollection = existingKeys.reduce((obj: OnyxInputKeyValueMapping, key) => { const {isCompatible, existingValueType, newValueType} = utils.checkCompatibilityWithExistingValue(mergedCollection[key], cachedCollectionForExistingKeys[key]); if (!isCompatible) { Logger.logAlert(logMessages.incompatibleUpdateAlert(key, 'mergeCollection', existingValueType, newValueType)); @@ -483,13 +476,13 @@ function mergeCollection(collectionKey: TK // eslint-disable-next-line no-param-reassign obj[key] = mergedCollection[key]; return obj; - }, {}); + }, {}) as Record>; - const newCollection = newKeys.reduce((obj: NullableKeyValueMapping, key) => { + const newCollection = newKeys.reduce((obj: OnyxInputKeyValueMapping, key) => { // eslint-disable-next-line no-param-reassign obj[key] = mergedCollection[key]; return obj; - }, {}); + }, {}) as Record>; // When (multi-)merging the values with the existing values in storage, // we don't want to remove nested null values from the data that we pass to the storage layer, diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index 8cd6d475..a1cf51fb 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -21,6 +21,7 @@ import type { Mapping, OnyxCollection, OnyxEntry, + OnyxInput, OnyxKey, OnyxValue, Selector, @@ -205,17 +206,17 @@ function reduceCollectionWithSelector> { +function get>(key: TKey): Promise { // When we already have the value in cache - resolve right away if (cache.hasCacheForKey(key)) { - return Promise.resolve(cache.get(key)); + return Promise.resolve(cache.get(key) as TValue); } const taskName = `get:${key}`; // When a value retrieving task for this key is still running hook to it if (cache.hasPendingTask(taskName)) { - return cache.getTaskPromise(taskName) as Promise>; + return cache.getTaskPromise(taskName) as Promise; } // Otherwise retrieve the value from storage and capture a promise to aid concurrent usages @@ -231,7 +232,7 @@ function get(key: OnyxKey): Promise> { }) .catch((err) => Logger.logInfo(`Unable to get item from persistent storage. Key: ${key} Error: ${err}`)); - return cache.captureTask(taskName, promise); + return cache.captureTask(taskName, promise) as Promise; } /** Returns current key names stored in persisted storage */ @@ -1007,8 +1008,8 @@ function hasPendingMergeForKey(key: OnyxKey): boolean { return !!mergeQueue[key]; } -type RemoveNullValuesOutput | null> = { - value: Value | null; +type RemoveNullValuesOutput | undefined> = { + value: Value; wasRemoved: boolean; }; @@ -1019,10 +1020,14 @@ type RemoveNullValuesOutput | null> = { * * @returns The value without null values and a boolean "wasRemoved", which indicates if the key got removed completely */ -function removeNullValues>(key: OnyxKey, value: Value | null, shouldRemoveNestedNulls = true): RemoveNullValuesOutput { +function removeNullValues | undefined>(key: OnyxKey, value: Value, shouldRemoveNestedNulls = true): RemoveNullValuesOutput { if (value === null) { remove(key); - return {value: null, wasRemoved: true}; + return {value, wasRemoved: true}; + } + + if (value === undefined) { + return {value, wasRemoved: false}; } // We can remove all null values in an object by merging it with itself @@ -1038,11 +1043,11 @@ function removeNullValues>(key: OnyxKey, value: * @return an array of key - value pairs <[key, value]> */ -function prepareKeyValuePairsForStorage(data: Record>, shouldRemoveNestedNulls: boolean): Array<[OnyxKey, OnyxValue]> { - return Object.entries(data).reduce]>>((pairs, [key, value]) => { +function prepareKeyValuePairsForStorage(data: Record>, shouldRemoveNestedNulls: boolean): Array<[OnyxKey, OnyxInput]> { + return Object.entries(data).reduce]>>((pairs, [key, value]) => { const {value: valueAfterRemoving, wasRemoved} = removeNullValues(key, value, shouldRemoveNestedNulls); - if (!wasRemoved) { + if (!wasRemoved && valueAfterRemoving !== undefined) { pairs.push([key, valueAfterRemoving]); } @@ -1055,7 +1060,7 @@ function prepareKeyValuePairsForStorage(data: Record * * @param changes Array of changes that should be applied to the existing value */ -function applyMerge(existingValue: OnyxValue, changes: Array>, shouldRemoveNestedNulls: boolean): OnyxValue { +function applyMerge | undefined>(existingValue: TValue, changes: TValue[], shouldRemoveNestedNulls: boolean): TValue { const lastChange = changes?.at(-1); if (Array.isArray(lastChange)) { @@ -1064,15 +1069,12 @@ function applyMerge(existingValue: OnyxValue, changes: Array change && typeof change === 'object')) { // Object values are then merged one after the other - return changes.reduce( - (modifiedData, change) => utils.fastMerge(modifiedData as Record, change as Record, shouldRemoveNestedNulls), - existingValue || {}, - ); + return changes.reduce((modifiedData, change) => utils.fastMerge(modifiedData, change, shouldRemoveNestedNulls), (existingValue || {}) as TValue); } // If we have anything else we can't merge it so we'll // simply return the last value that was queued - return lastChange; + return lastChange as TValue; } /** diff --git a/lib/types.ts b/lib/types.ts index f1635026..2f8dd723 100644 --- a/lib/types.ts +++ b/lib/types.ts @@ -112,33 +112,6 @@ type CollectionKey = `${CollectionKeyBase}${string}`; */ type OnyxKey = Key | CollectionKey; -/** - * Represents a Onyx value that can be either a single entry or a collection of entries, depending on the `TKey` provided. - */ -type OnyxValue = string extends TKey ? unknown : TKey extends CollectionKeyBase ? OnyxCollection : OnyxEntry; - -/** - * Represents a mapping of Onyx keys to values, where keys are either normal or collection Onyx keys - * and values are the corresponding values in Onyx's state. - * - * For collection keys, `KeyValueMapping` allows any string to be appended - * to the key (e.g., 'report_some-id', 'download_some-id'). - * - * The mapping is derived from the `values` property of the `TypeOptions` type. - */ -type KeyValueMapping = { - [TKey in keyof TypeOptions['values'] as TKey extends CollectionKeyBase ? `${TKey}${string}` : TKey]: TypeOptions['values'][TKey]; -}; - -/** - * Represents a mapping object where each `OnyxKey` maps to either a value of its corresponding type in `KeyValueMapping` or `null`. - * - * It's very similar to `KeyValueMapping` but this type accepts using `null` as well. - */ -type NullableKeyValueMapping = { - [TKey in OnyxKey]: NonUndefined> | null; -}; - /** * Represents a selector function type which operates based on the provided `TKey` and `ReturnType`. * @@ -185,7 +158,8 @@ type OnyxEntry = TOnyxValue | undefined; * Setting a key to `null` will remove the key from the store. * `undefined` is not allowed for setting values, because it will have no effect on the data. */ -type OnyxInput = TOnyxValue | null; +// type OnyxInput = TOnyxValue | null; +type OnyxInput = string extends TKey ? unknown : TKey extends CollectionKeyBase ? OnyxCollection : NullishDeep | null; /** * 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. @@ -217,6 +191,24 @@ type OnyxInput = TOnyxValue | null; */ type OnyxCollection = OnyxEntry>; +/** + * Represents a mapping of Onyx keys to values, where keys are either normal or collection Onyx keys + * and values are the corresponding values in Onyx's state. + * + * For collection keys, `KeyValueMapping` allows any string to be appended + * to the key (e.g., 'report_some-id', 'download_some-id'). + * + * The mapping is derived from the `values` property of the `TypeOptions` type. + */ +type KeyValueMapping = { + [TKey in keyof TypeOptions['values'] as TKey extends CollectionKeyBase ? `${TKey}${string}` : TKey]: TypeOptions['values'][TKey]; +}; + +/** + * Represents a Onyx value that can be either a single entry or a collection of entries, depending on the `TKey` provided. + */ +type OnyxValue = string extends TKey ? unknown : TKey extends CollectionKeyBase ? OnyxCollection : OnyxEntry; + /** Utility type to extract `TOnyxValue` from `OnyxCollection` */ type ExtractOnyxCollectionValue = TOnyxCollection extends NonNullable> ? U : never; @@ -328,25 +320,35 @@ type Mapping = ConnectOptions & { connectionID: number; }; +/** + * Represents a mapping object where each `OnyxKey` maps to either a value of its corresponding type in `KeyValueMapping` or `null`. + * + * It's very similar to `KeyValueMapping` but this type is used for inputs to Onyx + * (set, merge, mergeCollection) and therefore accepts using `null` as well. + */ +type OnyxInputKeyValueMapping = { + [TKey in OnyxKey]: OnyxInput; +}; + /** * This represents the value that can be passed to `Onyx.set` and to `Onyx.update` with the method "SET" */ -type OnyxSetInput = OnyxInput; +type OnyxSetInput = OnyxInput; /** * This represents the value that can be passed to `Onyx.multiSet` and to `Onyx.update` with the method "MULTI_SET" */ -type OnyxMultiSetInput = Partial; +type OnyxMultiSetInput = Partial; /** * This represents the value that can be passed to `Onyx.merge` and to `Onyx.update` with the method "MERGE" */ -type OnyxMergeInput = OnyxInput>; +type OnyxMergeInput = OnyxInput; /** * This represents the value that can be passed to `Onyx.merge` and to `Onyx.update` with the method "MERGE" */ -type OnyxMergeCollectionInput = Collection, TMap>; +type OnyxMergeCollectionInput = Collection>, TMap>; /** * Represents different kinds of updates that can be passed to `Onyx.update()` method. It is a discriminated union of @@ -392,7 +394,7 @@ type InitOptions = { keys?: DeepRecord; /** initial data to set when `init()` and `clear()` is called */ - initialKeyStates?: Partial; + initialKeyStates?: Partial; /** * This is an array of keys (individual or collection patterns) that when provided to Onyx are flagged @@ -440,7 +442,7 @@ export type { Mapping, NonNull, NonUndefined, - NullableKeyValueMapping, + OnyxInputKeyValueMapping, NullishDeep, OnyxCollection, OnyxEntry, diff --git a/lib/utils.ts b/lib/utils.ts index 15faa4ac..76bd46dc 100644 --- a/lib/utils.ts +++ b/lib/utils.ts @@ -1,6 +1,6 @@ /* eslint-disable @typescript-eslint/prefer-for-of */ -import type {OnyxKey, OnyxValue} from './types'; +import type {OnyxInput, OnyxKey} from './types'; type EmptyObject = Record; type EmptyValue = EmptyObject | null | undefined; @@ -27,19 +27,21 @@ function isMergeableObject(value: unknown): value is Record { * @param shouldRemoveNestedNulls - If true, null object values will be removed. * @returns - The merged object. */ -function mergeObject>(target: TObject | null, source: TObject, shouldRemoveNestedNulls = true): TObject { +function mergeObject>(target: TObject | unknown | null | undefined, source: TObject, shouldRemoveNestedNulls = true): TObject { const destination: Record = {}; + const targetObject = isMergeableObject(target) ? target : undefined; + // First we want to copy over all keys from the target into the destination object, // in case "target" is a mergable object. // If "shouldRemoveNestedNulls" is true, we want to remove null values from the merged object // and therefore we need to omit keys where either the source or target value is null. - if (isMergeableObject(target)) { - const targetKeys = Object.keys(target); + if (targetObject) { + const targetKeys = Object.keys(targetObject); for (let i = 0; i < targetKeys.length; ++i) { const key = targetKeys[i]; const sourceValue = source?.[key]; - const targetValue = target?.[key]; + const targetValue = targetObject?.[key]; // If "shouldRemoveNestedNulls" is true, we want to remove null values from the merged object. // Therefore, if either target or source value is null, we want to prevent the key from being set. @@ -57,7 +59,7 @@ function mergeObject>(target: TObject | for (let i = 0; i < sourceKeys.length; ++i) { const key = sourceKeys[i]; const sourceValue = source?.[key]; - const targetValue = target?.[key]; + const targetValue = targetObject?.[key]; // If undefined is passed as the source value for a key, we want to generally ignore it. // If "shouldRemoveNestedNulls" is set to true and the source value is null, @@ -92,7 +94,7 @@ function mergeObject>(target: TObject | * On native, when merging an existing value with new changes, SQLite will use JSON_PATCH, which removes top-level nullish values. * To be consistent with the behaviour for merge, we'll also want to remove null values for "set" operations. */ -function fastMerge>(target: TObject | null, source: TObject | null, shouldRemoveNestedNulls = true): TObject | null { +function fastMerge(target: TValue, source: TValue, shouldRemoveNestedNulls = true): TValue { // We have to ignore arrays and nullish values here, // otherwise "mergeObject" will throw an error, // because it expects an object as "source" @@ -100,14 +102,14 @@ function fastMerge>(target: TObject | nu return source; } - return mergeObject(target, source, shouldRemoveNestedNulls); + return mergeObject(target, source as Record, shouldRemoveNestedNulls) as TValue; } /** Deep removes the nested null values from the given value. */ -function removeNestedNullValues | unknown | unknown[]>(value: TValue | null): TValue | null { +function removeNestedNullValues | null>(value: TValue): TValue { if (typeof value === 'object' && !Array.isArray(value)) { - const objectValue = value as Record | null; - return fastMerge(objectValue, objectValue) as TValue | null; + const objectValue = value as Record; + return fastMerge(objectValue, objectValue) as TValue; } return value; From b680541297cfaa6ed980adfccdde3dc5e8d5f053 Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Tue, 4 Jun 2024 17:10:19 +0200 Subject: [PATCH 04/23] fix: onyx connect callback might return undefined --- lib/types.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/types.ts b/lib/types.ts index 2f8dd723..8cfb55c9 100644 --- a/lib/types.ts +++ b/lib/types.ts @@ -283,7 +283,7 @@ type WithOnyxConnectOptions = { canEvict?: boolean; }; -type DefaultConnectCallback = (value: NonUndefined>, key: TKey) => void; +type DefaultConnectCallback = (value: OnyxEntry, key: TKey) => void; type CollectionConnectCallback = (value: NonUndefined>) => void; From 540480f2e33e11164187870a424df78ed70498b7 Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Tue, 4 Jun 2024 17:30:41 +0200 Subject: [PATCH 05/23] fix: input types --- lib/types.ts | 90 +++++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 78 insertions(+), 12 deletions(-) diff --git a/lib/types.ts b/lib/types.ts index 8cfb55c9..d251c1b8 100644 --- a/lib/types.ts +++ b/lib/types.ts @@ -124,7 +124,7 @@ type OnyxKey = Key | CollectionKey; type Selector = (value: OnyxEntry, state?: WithOnyxState) => TReturnType; /** - * Represents a single Onyx entry, that can be either `TOnyxValue` or `null` / `undefined` if it doesn't exist. + * Represents a single Onyx entry, that can be either `TOnyxValue` or `undefined` if it doesn't exist. * * It can be used to specify data retrieved from Onyx e.g. `withOnyx` HOC mappings. * @@ -154,15 +154,7 @@ type Selector = (value: OnyxEntry type OnyxEntry = TOnyxValue | undefined; /** - * Represents an input value that can be passed to Onyx methods, that can be either `TOnyxValue` or `null`. - * Setting a key to `null` will remove the key from the store. - * `undefined` is not allowed for setting values, because it will have no effect on the data. - */ -// type OnyxInput = TOnyxValue | null; -type OnyxInput = string extends TKey ? unknown : TKey extends CollectionKeyBase ? OnyxCollection : NullishDeep | null; - -/** - * 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. + * Represents an Onyx collection of entries, that can be either a record of `TOnyxValue`s or `undefined` if it is empty or doesn't exist. * * It can be used to specify collection data retrieved from Onyx e.g. `withOnyx` HOC mappings. * @@ -320,11 +312,83 @@ type Mapping = ConnectOptions & { connectionID: number; }; +/** + * Represents a single Onyx input value, that can be either `TOnyxValue` or `null` if the key should be deleted. + * + * It can be used to specify data retrieved from Onyx e.g. `withOnyx` HOC mappings. + * + * @example + * ```ts + * import Onyx, {OnyxEntry, withOnyx} from 'react-native-onyx'; + * + * type OnyxProps = { + * userAccount: OnyxEntry; + * }; + * + * type Props = OnyxProps & { + * prop1: string; + * }; + * + * function Component({prop1, userAccount}: Props) { + * // ... + * } + * + * export default withOnyx({ + * userAccount: { + * key: ONYXKEYS.ACCOUNT, + * }, + * })(Component); + * ``` + */ +type OnyxInputValue = TOnyxValue | null; + +/** + * Represents an Onyx collection of inputs, that can be either a record of `TOnyxValue`s or `null` if the key should be deleted. + * + * It can be used to specify collection data retrieved from Onyx e.g. `withOnyx` HOC mappings. + * + * @example + * ```ts + * import Onyx, {OnyxCollection, withOnyx} from 'react-native-onyx'; + * + * type OnyxProps = { + * reports: OnyxCollection; + * }; + * + * type Props = OnyxProps & { + * prop1: string; + * }; + * + * function Component({prop1, reports}: Props) { + * // ... + * } + * + * export default withOnyx({ + * reports: { + * key: ONYXKEYS.COLLECTION.REPORT, + * }, + * })(Component); + * ``` + */ +type OnyxCollectionInput = OnyxInputValue>; + +/** + * Represents an input value that can be passed to Onyx methods, that can be either `TOnyxValue` or `null`. + * Setting a key to `null` will remove the key from the store. + * `undefined` is not allowed for setting values, because it will have no effect on the data. + */ +// type OnyxInput = TOnyxValue | null; +type OnyxInput = string extends TKey + ? unknown + : TKey extends CollectionKeyBase + ? OnyxCollectionInput + : OnyxInputValue>; + /** * Represents a mapping object where each `OnyxKey` maps to either a value of its corresponding type in `KeyValueMapping` or `null`. * * It's very similar to `KeyValueMapping` but this type is used for inputs to Onyx - * (set, merge, mergeCollection) and therefore accepts using `null` as well. + * (set, merge, mergeCollection) and therefore accepts using `null` to remove a key from Onyx. */ type OnyxInputKeyValueMapping = { [TKey in OnyxKey]: OnyxInput; @@ -446,8 +510,10 @@ export type { NullishDeep, OnyxCollection, OnyxEntry, - OnyxInput, OnyxKey, + OnyxInputValue, + OnyxCollectionInput, + OnyxInput, OnyxSetInput, OnyxMultiSetInput, OnyxMergeInput, From b0d25058249977eceb4b4fcfa08a96566d242a55 Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Tue, 4 Jun 2024 17:34:01 +0200 Subject: [PATCH 06/23] fix: export types --- lib/OnyxUtils.ts | 10 +++++++--- lib/index.ts | 8 ++++++-- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index a1cf51fb..cb8df704 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -1060,7 +1060,11 @@ function prepareKeyValuePairsForStorage(data: Record * * @param changes Array of changes that should be applied to the existing value */ -function applyMerge | undefined>(existingValue: TValue, changes: TValue[], shouldRemoveNestedNulls: boolean): TValue { +function applyMerge | undefined, TChange extends OnyxInput | undefined>( + existingValue: TValue, + changes: TChange[], + shouldRemoveNestedNulls: boolean, +): TChange { const lastChange = changes?.at(-1); if (Array.isArray(lastChange)) { @@ -1069,12 +1073,12 @@ function applyMerge | undefined>(existingValue if (changes.some((change) => change && typeof change === 'object')) { // Object values are then merged one after the other - return changes.reduce((modifiedData, change) => utils.fastMerge(modifiedData, change, shouldRemoveNestedNulls), (existingValue || {}) as TValue); + return changes.reduce((modifiedData, change) => utils.fastMerge(modifiedData, change, shouldRemoveNestedNulls), (existingValue || {}) as TChange); } // If we have anything else we can't merge it so we'll // simply return the last value that was queued - return lastChange as TValue; + return lastChange as TChange; } /** diff --git a/lib/index.ts b/lib/index.ts index 3352d39e..a4e3cc5f 100644 --- a/lib/index.ts +++ b/lib/index.ts @@ -6,10 +6,12 @@ import type { NullishDeep, OnyxCollection, OnyxEntry, - OnyxInput, OnyxKey, OnyxValue, Selector, + OnyxInputValue, + OnyxCollectionInput, + OnyxInput, OnyxSetInput, OnyxMultiSetInput, OnyxMergeInput, @@ -30,8 +32,10 @@ export type { NullishDeep, OnyxCollection, OnyxEntry, - OnyxInput, OnyxKey, + OnyxInputValue, + OnyxCollectionInput, + OnyxInput, OnyxSetInput, OnyxMultiSetInput, OnyxMergeInput, From d44df567c955712126183cb208478e8390d9f86e Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Tue, 4 Jun 2024 18:01:40 +0200 Subject: [PATCH 07/23] fix: input type --- lib/index.ts | 2 -- lib/types.ts | 37 +------------------------------------ 2 files changed, 1 insertion(+), 38 deletions(-) diff --git a/lib/index.ts b/lib/index.ts index a4e3cc5f..bf08c29d 100644 --- a/lib/index.ts +++ b/lib/index.ts @@ -10,7 +10,6 @@ import type { OnyxValue, Selector, OnyxInputValue, - OnyxCollectionInput, OnyxInput, OnyxSetInput, OnyxMultiSetInput, @@ -34,7 +33,6 @@ export type { OnyxEntry, OnyxKey, OnyxInputValue, - OnyxCollectionInput, OnyxInput, OnyxSetInput, OnyxMultiSetInput, diff --git a/lib/types.ts b/lib/types.ts index d251c1b8..f193336d 100644 --- a/lib/types.ts +++ b/lib/types.ts @@ -342,47 +342,13 @@ type Mapping = ConnectOptions & { */ type OnyxInputValue = TOnyxValue | null; -/** - * Represents an Onyx collection of inputs, that can be either a record of `TOnyxValue`s or `null` if the key should be deleted. - * - * It can be used to specify collection data retrieved from Onyx e.g. `withOnyx` HOC mappings. - * - * @example - * ```ts - * import Onyx, {OnyxCollection, withOnyx} from 'react-native-onyx'; - * - * type OnyxProps = { - * reports: OnyxCollection; - * }; - * - * type Props = OnyxProps & { - * prop1: string; - * }; - * - * function Component({prop1, reports}: Props) { - * // ... - * } - * - * export default withOnyx({ - * reports: { - * key: ONYXKEYS.COLLECTION.REPORT, - * }, - * })(Component); - * ``` - */ -type OnyxCollectionInput = OnyxInputValue>; - /** * Represents an input value that can be passed to Onyx methods, that can be either `TOnyxValue` or `null`. * Setting a key to `null` will remove the key from the store. * `undefined` is not allowed for setting values, because it will have no effect on the data. */ // type OnyxInput = TOnyxValue | null; -type OnyxInput = string extends TKey - ? unknown - : TKey extends CollectionKeyBase - ? OnyxCollectionInput - : OnyxInputValue>; +type OnyxInput = OnyxInputValue>; /** * Represents a mapping object where each `OnyxKey` maps to either a value of its corresponding type in `KeyValueMapping` or `null`. @@ -512,7 +478,6 @@ export type { OnyxEntry, OnyxKey, OnyxInputValue, - OnyxCollectionInput, OnyxInput, OnyxSetInput, OnyxMultiSetInput, From cdf0c7850400b0b0134827b6dc0ef2825bc391a8 Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Tue, 4 Jun 2024 18:37:17 +0200 Subject: [PATCH 08/23] fix: check for equality --- lib/OnyxUtils.ts | 33 ++++++++++++++++++++------------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index cb8df704..13787161 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -546,14 +546,15 @@ function keysChanged( } subscriber.withOnyxInstance.setStateProxy((prevState) => { - const finalCollection = lodashClone(prevState?.[subscriber.statePropertyName] ?? {}); + const prevCollection = prevState?.[subscriber.statePropertyName] ?? {}; + const finalCollection = lodashClone(prevCollection); const dataKeys = Object.keys(partialCollection ?? {}); for (let j = 0; j < dataKeys.length; j++) { const dataKey = dataKeys[j]; finalCollection[dataKey] = cachedCollection[dataKey]; } - if (!prevState.loading && deepEqual(cachedCollection, finalCollection)) { + if (deepEqual(prevCollection, finalCollection)) { return null; } @@ -600,20 +601,21 @@ function keysChanged( } subscriber.withOnyxInstance.setStateProxy((prevState) => { - const data = cachedCollection[subscriber.key]; - const previousData = prevState[subscriber.statePropertyName]; + const prevData = prevState[subscriber.statePropertyName]; + const newData = cachedCollection[subscriber.key]; // Avoids triggering unnecessary re-renders when feeding empty objects - if (utils.isEmptyObject(data) && utils.isEmptyObject(previousData)) { + if (utils.isEmptyObject(newData) && utils.isEmptyObject(prevData)) { return null; } - if (data === previousData) { + + if (deepEqual(prevData, newData)) { return null; } - PerformanceUtils.logSetStateCall(subscriber, previousData, data, 'keysChanged', collectionKey); + PerformanceUtils.logSetStateCall(subscriber, prevData, newData, 'keysChanged', collectionKey); return { - [subscriber.statePropertyName]: data, + [subscriber.statePropertyName]: newData, }; }); } @@ -705,12 +707,17 @@ function keyChanged( } subscriber.withOnyxInstance.setStateProxy((prevState) => { - const collection = prevState[subscriber.statePropertyName] || {}; + const prevCollection = prevState[subscriber.statePropertyName] || {}; const newCollection = { - ...collection, + ...prevCollection, [key]: value, }; - PerformanceUtils.logSetStateCall(subscriber, collection, newCollection, 'keyChanged', key); + + if (deepEqual(prevCollection, newCollection)) { + return null; + } + + PerformanceUtils.logSetStateCall(subscriber, prevCollection, newCollection, 'keyChanged', key); return { [subscriber.statePropertyName]: newCollection, }; @@ -721,11 +728,11 @@ function keyChanged( // If the subscriber has a selector, then the component's state must only be updated with the data // returned by the selector and only if the selected data has changed. if (selector) { - subscriber.withOnyxInstance.setStateProxy((prevState) => { + subscriber.withOnyxInstance.setStateProxy(() => { const prevValue = selector(previousValue, subscriber.withOnyxInstance.state); const newValue = selector(value, subscriber.withOnyxInstance.state); - if (!prevState.loading && deepEqual(prevValue, newValue)) { + if (deepEqual(prevValue, newValue)) { return null; } From c8dd37593b4e3a9bef84bb4875ba5e9714bcb3e4 Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Wed, 5 Jun 2024 09:47:20 +0200 Subject: [PATCH 09/23] fix: removing items won't return undefined --- lib/OnyxUtils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index 13787161..2e8fbda6 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -945,7 +945,7 @@ function scheduleNotifyCollectionSubscribers( function remove(key: TKey): Promise { const prevValue = cache.get(key, false) as OnyxValue; cache.drop(key); - scheduleSubscriberUpdate(key, null as OnyxValue, prevValue); + scheduleSubscriberUpdate(key, undefined as OnyxValue, prevValue); return Storage.removeItem(key).then(() => undefined); } From 1095fa469e0a620cbf3f314ba81749c0184a5037 Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Wed, 5 Jun 2024 09:53:15 +0200 Subject: [PATCH 10/23] check for undefined values in fastMerge --- lib/utils.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/utils.ts b/lib/utils.ts index 76bd46dc..502b4da7 100644 --- a/lib/utils.ts +++ b/lib/utils.ts @@ -45,7 +45,10 @@ function mergeObject>(target: TObject | // If "shouldRemoveNestedNulls" is true, we want to remove null values from the merged object. // Therefore, if either target or source value is null, we want to prevent the key from being set. - const isSourceOrTargetNull = targetValue === null || sourceValue === null; + // targetValue should techincally never be "undefined", because it will always be a value from cache or storage + // and we never set "undefined" there. Still, if there targetValue is undefined we don't want to set + // the key explicitly to prevent loose undefined values in objects in cache and storage. + const isSourceOrTargetNull = targetValue === undefined || targetValue === null || sourceValue === null; const shouldOmitTargetKey = shouldRemoveNestedNulls && isSourceOrTargetNull; if (!shouldOmitTargetKey) { From da1bd7e200605cc4d5e6dabeda35f49a44c00413 Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Wed, 5 Jun 2024 09:53:26 +0200 Subject: [PATCH 11/23] add tests regarding undefined handling --- tests/unit/onyxTest.ts | 117 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 116 insertions(+), 1 deletion(-) diff --git a/tests/unit/onyxTest.ts b/tests/unit/onyxTest.ts index 95c189be..756c4de7 100644 --- a/tests/unit/onyxTest.ts +++ b/tests/unit/onyxTest.ts @@ -298,6 +298,7 @@ describe('Onyx', () => { }, }, }); + return Onyx.merge(ONYX_KEYS.TEST_KEY, { test1: { test3: { @@ -313,6 +314,7 @@ describe('Onyx', () => { test3: {}, }, }); + return Onyx.merge(ONYX_KEYS.TEST_KEY, { test1: { test3: null, @@ -321,6 +323,7 @@ describe('Onyx', () => { }) .then(() => { expect(testKeyValue).toEqual({test1: {test2: 'test2'}}); + return Onyx.merge(ONYX_KEYS.TEST_KEY, {test1: null}); }) .then(() => { @@ -328,7 +331,114 @@ describe('Onyx', () => { }); }); - it('should ignore `undefined` values when merging', () => { + it('should ignore top-level and remove nested `undefined` values in Onyx.set', () => { + let testKeyValue: unknown; + + connectionID = Onyx.connect({ + key: ONYX_KEYS.TEST_KEY, + initWithStoredValues: false, + callback: (value) => { + testKeyValue = value; + }, + }); + + return Onyx.set(ONYX_KEYS.TEST_KEY, { + test1: { + test2: 'test2', + test3: 'test3', + }, + }) + .then(() => { + expect(testKeyValue).toEqual({ + test1: { + test2: 'test2', + test3: 'test3', + }, + }); + + return Onyx.set(ONYX_KEYS.TEST_KEY, { + test1: { + test2: undefined, + test3: 'test3', + }, + }); + }) + .then(() => { + expect(testKeyValue).toEqual({test1: {test3: 'test3'}}); + + return Onyx.set(ONYX_KEYS.TEST_KEY, {test1: undefined}); + }) + .then(() => { + expect(testKeyValue).toEqual({}); + + return Onyx.set(ONYX_KEYS.TEST_KEY, undefined); + }) + .then(() => { + expect(testKeyValue).toEqual({}); + + return Onyx.set(ONYX_KEYS.TEST_KEY, {test1: undefined}); + }); + }); + + it('should ignore top-level and remove nested `undefined` values in Onyx.multiSet', () => { + let testKeyValue: unknown; + connectionID = Onyx.connect({ + key: ONYX_KEYS.TEST_KEY, + initWithStoredValues: false, + callback: (value) => { + testKeyValue = value; + }, + }); + + let otherTestKeyValue: unknown; + connectionID = Onyx.connect({ + key: ONYX_KEYS.OTHER_TEST, + initWithStoredValues: false, + callback: (value) => { + otherTestKeyValue = value; + }, + }); + + return Onyx.multiSet({ + [ONYX_KEYS.TEST_KEY]: { + test1: 'test1', + test2: 'test2', + }, + [ONYX_KEYS.OTHER_TEST]: 'otherTest', + }) + .then(() => { + expect(testKeyValue).toEqual({ + test1: 'test1', + test2: 'test2', + }); + expect(otherTestKeyValue).toEqual('otherTest'); + + return Onyx.multiSet({ + [ONYX_KEYS.TEST_KEY]: { + test1: 'test1', + test2: undefined, + }, + [ONYX_KEYS.OTHER_TEST]: undefined, + }); + }) + .then(() => { + expect(testKeyValue).toEqual({ + test1: 'test1', + }); + expect(otherTestKeyValue).toEqual('otherTest'); + + return Onyx.multiSet({ + [ONYX_KEYS.TEST_KEY]: null, + [ONYX_KEYS.OTHER_TEST]: null, + }); + }) + .then(() => { + expect(testKeyValue).toEqual(undefined); + expect(otherTestKeyValue).toEqual(undefined); + }); + }); + + it('should ignore top-level and remove nested `undefined` values in Onyx.merge', () => { let testKeyValue: unknown; connectionID = Onyx.connect({ @@ -364,6 +474,11 @@ describe('Onyx', () => { }) .then(() => { expect(testKeyValue).toEqual({test1: {test2: 'test2', test3: 'test3'}}); + return Onyx.merge(ONYX_KEYS.TEST_KEY, undefined); + }) + .then(() => { + expect(testKeyValue).toEqual({test1: {test2: 'test2', test3: 'test3'}}); + return Onyx.merge(ONYX_KEYS.TEST_KEY, undefined); }); }); From ae64bfcf2621840aded89191c1dbaeeb84d6f8fe Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Wed, 5 Jun 2024 10:02:03 +0200 Subject: [PATCH 12/23] add test for mergeCollection --- tests/unit/onyxTest.ts | 91 +++++++++++++++++++----------------------- 1 file changed, 41 insertions(+), 50 deletions(-) diff --git a/tests/unit/onyxTest.ts b/tests/unit/onyxTest.ts index 756c4de7..d5e93382 100644 --- a/tests/unit/onyxTest.ts +++ b/tests/unit/onyxTest.ts @@ -249,27 +249,6 @@ describe('Onyx', () => { }); }); - it('should ignore top-level undefined values', () => { - let testKeyValue: unknown; - - connectionID = Onyx.connect({ - key: ONYX_KEYS.TEST_KEY, - initWithStoredValues: false, - callback: (value) => { - testKeyValue = value; - }, - }); - - return Onyx.set(ONYX_KEYS.TEST_KEY, {test1: 'test1'}) - .then(() => { - expect(testKeyValue).toEqual({test1: 'test1'}); - return Onyx.merge(ONYX_KEYS.TEST_KEY, undefined); - }) - .then(() => { - expect(testKeyValue).toEqual({test1: 'test1'}); - }); - }); - it('should remove keys that are set to null when merging', () => { let testKeyValue: unknown; @@ -478,10 +457,49 @@ describe('Onyx', () => { }) .then(() => { expect(testKeyValue).toEqual({test1: {test2: 'test2', test3: 'test3'}}); - return Onyx.merge(ONYX_KEYS.TEST_KEY, undefined); }); }); + it('should ignore top-level and remove nested `undefined` values in Onyx.mergeCollection', () => { + let result: OnyxCollection; + + const routineRoute = `${ONYX_KEYS.COLLECTION.TEST_KEY}routine`; + const holidayRoute = `${ONYX_KEYS.COLLECTION.TEST_KEY}holiday`; + const workRoute = `${ONYX_KEYS.COLLECTION.TEST_KEY}work`; + + connectionID = Onyx.connect({ + key: ONYX_KEYS.COLLECTION.TEST_KEY, + initWithStoredValues: false, + callback: (value) => (result = value), + waitForCollectionCallback: true, + }); + + return Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, { + [routineRoute]: { + waypoints: { + 1: 'Home', + 2: 'Work', + 3: undefined, + }, + }, + [holidayRoute]: { + waypoints: undefined, + }, + [workRoute]: undefined, + } as GenericCollection).then(() => { + expect(result).toEqual({ + [routineRoute]: { + waypoints: { + 1: 'Home', + 2: 'Work', + }, + }, + [holidayRoute]: {}, + [workRoute]: undefined, + }); + }); + }); + it('should overwrite an array key nested inside an object', () => { let testKeyValue: unknown; connectionID = Onyx.connect({ @@ -1202,7 +1220,7 @@ describe('Onyx', () => { }); }); - it('should merge a non-existing key with a nested null removed', () => { + it("should not set null values in Onyx.merge, when the key doesn't exist yet", () => { let testKeyValue: unknown; connectionID = Onyx.connect({ @@ -1261,33 +1279,6 @@ describe('Onyx', () => { }); }); - it('should merge a non-existing key with a nested null removed', () => { - let testKeyValue: unknown; - - connectionID = Onyx.connect({ - key: ONYX_KEYS.TEST_KEY, - initWithStoredValues: false, - callback: (value) => { - testKeyValue = value; - }, - }); - - return Onyx.merge(ONYX_KEYS.TEST_KEY, { - waypoints: { - 1: 'Home', - 2: 'Work', - 3: null, - }, - }).then(() => { - expect(testKeyValue).toEqual({ - waypoints: { - 1: 'Home', - 2: 'Work', - }, - }); - }); - }); - it('mergeCollection should omit nested null values', () => { let result: OnyxCollection; From 8292acb8f09e655da15a9ecc25b0a28b4453c3d0 Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Wed, 5 Jun 2024 10:04:59 +0200 Subject: [PATCH 13/23] fix: callbacks always return undefined instead of null --- tests/unit/onyxTest.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/unit/onyxTest.ts b/tests/unit/onyxTest.ts index d5e93382..bd9212e7 100644 --- a/tests/unit/onyxTest.ts +++ b/tests/unit/onyxTest.ts @@ -165,7 +165,7 @@ describe('Onyx', () => { }) .then(() => { // Test key should be cleared - expect(testKeyValue).toBeNull(); + expect(testKeyValue).toBeUndefined(); // Other test key should be returned to its default state expect(otherTestValue).toBe(42); @@ -1212,7 +1212,7 @@ describe('Onyx', () => { return waitForPromisesToResolve(); }) .then(() => { - expect(testKeyValue).toEqual(null); + expect(testKeyValue).toEqual(undefined); return Onyx.merge(ONYX_KEYS.TEST_KEY, 2); }) .then(() => { @@ -1275,7 +1275,7 @@ describe('Onyx', () => { return waitForPromisesToResolve(); }) .then(() => { - expect(testKeyValue).toEqual(null); + expect(testKeyValue).toEqual(undefined); }); }); From 81291e76826607c16cc5ea7315e8deeb9c481013 Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Wed, 5 Jun 2024 10:07:48 +0200 Subject: [PATCH 14/23] fix: tests after null -> undefined changes --- lib/Onyx.ts | 2 +- tests/unit/withOnyxTest.tsx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/Onyx.ts b/lib/Onyx.ts index edbf058e..5d2acdec 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -575,7 +575,7 @@ function clear(keysToPreserve: OnyxKey[] = []): Promise { // since collection key subscribers need to be updated differently if (!isKeyToPreserve) { const oldValue = cache.get(key); - const newValue = defaultKeyStates[key] ?? null; + const newValue = defaultKeyStates[key] ?? undefined; if (newValue !== oldValue) { cache.set(key, newValue); const collectionKey = key.substring(0, key.indexOf('_') + 1); diff --git a/tests/unit/withOnyxTest.tsx b/tests/unit/withOnyxTest.tsx index 34eb9de7..b29e8f40 100644 --- a/tests/unit/withOnyxTest.tsx +++ b/tests/unit/withOnyxTest.tsx @@ -810,7 +810,7 @@ describe('withOnyxTest', () => { // Correct data has been passed to the selector expect(selector).not.toHaveBeenCalledWith('world', expect.anything()); expect(selector).not.toHaveBeenCalledWith('dougal', expect.anything()); - expect(selector).toHaveBeenLastCalledWith(sourceData, {loading: false, text: 'world'}); + expect(selector).toHaveBeenLastCalledWith(undefined, {loading: false, text: 'world'}); // Default text has been rendered expect(onRender).toHaveBeenCalledTimes(1); From 790e2b799b46915f39f711774708d7ae43e56b94 Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Wed, 5 Jun 2024 14:02:22 +0200 Subject: [PATCH 15/23] remove custom UseOnyxValue type --- lib/useOnyx.ts | 20 +++++--------------- 1 file changed, 5 insertions(+), 15 deletions(-) diff --git a/lib/useOnyx.ts b/lib/useOnyx.ts index 958170bb..230cbafa 100644 --- a/lib/useOnyx.ts +++ b/lib/useOnyx.ts @@ -2,21 +2,11 @@ import {deepEqual} from 'fast-equals'; import {useCallback, useEffect, useRef, useSyncExternalStore} from 'react'; import type {IsEqual} from 'type-fest'; import OnyxUtils from './OnyxUtils'; -import type {CollectionKeyBase, KeyValueMapping, NonNull, OnyxCollection, OnyxEntry, OnyxKey, Selector} from './types'; +import type {CollectionKeyBase, OnyxCollection, OnyxKey, OnyxValue, Selector} from './types'; import useLiveRef from './useLiveRef'; import usePrevious from './usePrevious'; import Onyx from './Onyx'; -/** - * Represents a Onyx value that can be either a single entry or a collection of entries, depending on the `TKey` provided. - * It's a variation of `OnyxValue` type that is read-only and excludes the `null` type. - */ -type UseOnyxValue = string extends TKey - ? unknown - : TKey extends CollectionKeyBase - ? NonNull> - : NonNull>; - type BaseUseOnyxOptions = { /** * Determines if this key in this subscription is safe to be evicted. @@ -55,7 +45,7 @@ type UseOnyxOptions = BaseUseOnyxOptions & U type FetchStatus = 'loading' | 'loaded'; -type CachedValue = IsEqual> extends true ? TValue : TKey extends CollectionKeyBase ? NonNullable> : TValue; +type CachedValue = IsEqual> extends true ? TValue : TKey extends CollectionKeyBase ? NonNullable> : TValue; type ResultMetadata = { status: FetchStatus; @@ -67,15 +57,15 @@ function getCachedValue(key: TKey, selector?: Sele return OnyxUtils.tryGetCachedValue(key, {selector}) as CachedValue | undefined; } -function useOnyx>( +function useOnyx>( key: TKey, options?: BaseUseOnyxOptions & UseOnyxInitialValueOption & Required>, ): UseOnyxResult; -function useOnyx>( +function useOnyx>( key: TKey, options?: BaseUseOnyxOptions & UseOnyxInitialValueOption>, ): UseOnyxResult; -function useOnyx>(key: TKey, options?: UseOnyxOptions): UseOnyxResult { +function useOnyx>(key: TKey, options?: UseOnyxOptions): UseOnyxResult { const connectionIDRef = useRef(null); const previousKey = usePrevious(key); From b386809bb5249d9951ab8f0236add16b9402aeeb Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Wed, 5 Jun 2024 14:04:40 +0200 Subject: [PATCH 16/23] update comments --- lib/types.ts | 27 +-------------------------- 1 file changed, 1 insertion(+), 26 deletions(-) diff --git a/lib/types.ts b/lib/types.ts index f193336d..8ba92f68 100644 --- a/lib/types.ts +++ b/lib/types.ts @@ -314,31 +314,7 @@ type Mapping = ConnectOptions & { /** * Represents a single Onyx input value, that can be either `TOnyxValue` or `null` if the key should be deleted. - * - * It can be used to specify data retrieved from Onyx e.g. `withOnyx` HOC mappings. - * - * @example - * ```ts - * import Onyx, {OnyxEntry, withOnyx} from 'react-native-onyx'; - * - * type OnyxProps = { - * userAccount: OnyxEntry; - * }; - * - * type Props = OnyxProps & { - * prop1: string; - * }; - * - * function Component({prop1, userAccount}: Props) { - * // ... - * } - * - * export default withOnyx({ - * userAccount: { - * key: ONYXKEYS.ACCOUNT, - * }, - * })(Component); - * ``` + * This type is used for data passed to Onyx e.g. in `Onyx.merge` and `Onyx.set`. */ type OnyxInputValue = TOnyxValue | null; @@ -347,7 +323,6 @@ type OnyxInputValue = TOnyxValue | null; * Setting a key to `null` will remove the key from the store. * `undefined` is not allowed for setting values, because it will have no effect on the data. */ -// type OnyxInput = TOnyxValue | null; type OnyxInput = OnyxInputValue>; /** From b1315b3733d5508869270481c19a9cce21dc49cc Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Wed, 5 Jun 2024 16:51:53 +0200 Subject: [PATCH 17/23] add a OnyxCollectionInput --- lib/index.ts | 2 ++ lib/types.ts | 9 ++++++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/lib/index.ts b/lib/index.ts index bf08c29d..a4e3cc5f 100644 --- a/lib/index.ts +++ b/lib/index.ts @@ -10,6 +10,7 @@ import type { OnyxValue, Selector, OnyxInputValue, + OnyxCollectionInput, OnyxInput, OnyxSetInput, OnyxMultiSetInput, @@ -33,6 +34,7 @@ export type { OnyxEntry, OnyxKey, OnyxInputValue, + OnyxCollectionInput, OnyxInput, OnyxSetInput, OnyxMultiSetInput, diff --git a/lib/types.ts b/lib/types.ts index 8ba92f68..e0c34779 100644 --- a/lib/types.ts +++ b/lib/types.ts @@ -318,12 +318,18 @@ type Mapping = ConnectOptions & { */ type OnyxInputValue = TOnyxValue | null; +/** + * Represents an Onyx collection input, that can be either a record of `TOnyxValue`s or `null` if the key should be deleted. + */ +type OnyxCollectionInput = OnyxInputValue>; + /** * Represents an input value that can be passed to Onyx methods, that can be either `TOnyxValue` or `null`. * Setting a key to `null` will remove the key from the store. * `undefined` is not allowed for setting values, because it will have no effect on the data. */ -type OnyxInput = OnyxInputValue>; +// type OnyxInput = OnyxInputValue>; +type OnyxInput = string extends TKey ? unknown : TKey extends CollectionKeyBase ? OnyxCollectionInput : OnyxInputValue; /** * Represents a mapping object where each `OnyxKey` maps to either a value of its corresponding type in `KeyValueMapping` or `null`. @@ -453,6 +459,7 @@ export type { OnyxEntry, OnyxKey, OnyxInputValue, + OnyxCollectionInput, OnyxInput, OnyxSetInput, OnyxMultiSetInput, From 08c2bfbabd4fb60d3fbf9a5dc3ebf715e934d4d3 Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Wed, 5 Jun 2024 16:53:47 +0200 Subject: [PATCH 18/23] fix: OnyxInput type --- lib/types.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/types.ts b/lib/types.ts index e0c34779..0ec6fb1b 100644 --- a/lib/types.ts +++ b/lib/types.ts @@ -328,8 +328,7 @@ type OnyxCollectionInput = OnyxInputValue = OnyxInputValue>; -type OnyxInput = string extends TKey ? unknown : TKey extends CollectionKeyBase ? OnyxCollectionInput : OnyxInputValue; +type OnyxInput = OnyxInputValue>; /** * Represents a mapping object where each `OnyxKey` maps to either a value of its corresponding type in `KeyValueMapping` or `null`. From 4e893aa277ceacce0a903afb765ed1e724ec0d5a Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Wed, 5 Jun 2024 17:05:26 +0200 Subject: [PATCH 19/23] update mergeCollection input type --- lib/Onyx.ts | 4 ++-- lib/types.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/Onyx.ts b/lib/Onyx.ts index 5d2acdec..74ff100e 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -423,12 +423,12 @@ function merge(key: TKey, changes: OnyxMergeInput): * @param collectionKey e.g. `ONYXKEYS.COLLECTION.REPORT` * @param collection Object collection keyed by individual collection member keys and values */ -function mergeCollection(collectionKey: TKey, collection: OnyxMergeCollectionInput): Promise { +function mergeCollection(collectionKey: TKey, collection: OnyxMergeCollectionInput): Promise { if (typeof collection !== 'object' || Array.isArray(collection) || utils.isEmptyObject(collection)) { Logger.logInfo('mergeCollection() called with invalid or empty value. Skipping this update.'); return Promise.resolve(); } - const mergedCollection: OnyxInputKeyValueMapping = collection; + const mergedCollection = collection; // Confirm all the collection keys belong to the same parent let hasCollectionKeyCheckFailed = false; diff --git a/lib/types.ts b/lib/types.ts index 0ec6fb1b..605e19a5 100644 --- a/lib/types.ts +++ b/lib/types.ts @@ -358,7 +358,7 @@ type OnyxMergeInput = OnyxInput; /** * This represents the value that can be passed to `Onyx.merge` and to `Onyx.update` with the method "MERGE" */ -type OnyxMergeCollectionInput = Collection>, TMap>; +type OnyxMergeCollectionInput = OnyxCollectionInput>; /** * Represents different kinds of updates that can be passed to `Onyx.update()` method. It is a discriminated union of From 4b1dccc8d14366c53448a2b2e6127ffaa97837b2 Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Wed, 5 Jun 2024 17:05:48 +0200 Subject: [PATCH 20/23] remove unused ts-expect-error pragmas --- tests/unit/useOnyxTest.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/unit/useOnyxTest.ts b/tests/unit/useOnyxTest.ts index 04160c51..d45eef97 100644 --- a/tests/unit/useOnyxTest.ts +++ b/tests/unit/useOnyxTest.ts @@ -163,7 +163,6 @@ describe('useOnyx', () => { }); it('should return selected data from a collection key', async () => { - // @ts-expect-error bypass Onyx.mergeCollection(ONYXKEYS.COLLECTION.TEST_KEY, { [`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`]: {id: 'entry1_id', name: 'entry1_name'}, [`${ONYXKEYS.COLLECTION.TEST_KEY}entry2`]: {id: 'entry2_id', name: 'entry2_name'}, @@ -196,7 +195,6 @@ describe('useOnyx', () => { }); it('should not change selected data if a property outside the selector was changed', async () => { - // @ts-expect-error bypass Onyx.mergeCollection(ONYXKEYS.COLLECTION.TEST_KEY, { [`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`]: {id: 'entry1_id', name: 'entry1_name'}, [`${ONYXKEYS.COLLECTION.TEST_KEY}entry2`]: {id: 'entry2_id', name: 'entry2_name'}, From 7b72e607eba5828a7befc4baea76e96d2c2a69e6 Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Wed, 5 Jun 2024 17:13:59 +0200 Subject: [PATCH 21/23] rename type --- lib/index.ts | 4 ++-- lib/types.ts | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/index.ts b/lib/index.ts index a4e3cc5f..04ec3fb9 100644 --- a/lib/index.ts +++ b/lib/index.ts @@ -10,7 +10,7 @@ import type { OnyxValue, Selector, OnyxInputValue, - OnyxCollectionInput, + OnyxCollectionInputValue, OnyxInput, OnyxSetInput, OnyxMultiSetInput, @@ -34,7 +34,7 @@ export type { OnyxEntry, OnyxKey, OnyxInputValue, - OnyxCollectionInput, + OnyxCollectionInputValue, OnyxInput, OnyxSetInput, OnyxMultiSetInput, diff --git a/lib/types.ts b/lib/types.ts index 605e19a5..3b098b07 100644 --- a/lib/types.ts +++ b/lib/types.ts @@ -321,7 +321,7 @@ type OnyxInputValue = TOnyxValue | null; /** * Represents an Onyx collection input, that can be either a record of `TOnyxValue`s or `null` if the key should be deleted. */ -type OnyxCollectionInput = OnyxInputValue>; +type OnyxCollectionInputValue = OnyxInputValue>; /** * Represents an input value that can be passed to Onyx methods, that can be either `TOnyxValue` or `null`. @@ -358,7 +358,7 @@ type OnyxMergeInput = OnyxInput; /** * This represents the value that can be passed to `Onyx.merge` and to `Onyx.update` with the method "MERGE" */ -type OnyxMergeCollectionInput = OnyxCollectionInput>; +type OnyxMergeCollectionInput = OnyxCollectionInputValue>; /** * Represents different kinds of updates that can be passed to `Onyx.update()` method. It is a discriminated union of @@ -458,7 +458,7 @@ export type { OnyxEntry, OnyxKey, OnyxInputValue, - OnyxCollectionInput, + OnyxCollectionInputValue, OnyxInput, OnyxSetInput, OnyxMultiSetInput, From 5fc63a9cfae1f11dfd55ea2fb644f1e1147dedbd Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Thu, 6 Jun 2024 11:53:07 +0200 Subject: [PATCH 22/23] Revert "update mergeCollection input type" This reverts commit 4e893aa277ceacce0a903afb765ed1e724ec0d5a. --- lib/Onyx.ts | 4 ++-- lib/types.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/Onyx.ts b/lib/Onyx.ts index 74ff100e..5d2acdec 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -423,12 +423,12 @@ function merge(key: TKey, changes: OnyxMergeInput): * @param collectionKey e.g. `ONYXKEYS.COLLECTION.REPORT` * @param collection Object collection keyed by individual collection member keys and values */ -function mergeCollection(collectionKey: TKey, collection: OnyxMergeCollectionInput): Promise { +function mergeCollection(collectionKey: TKey, collection: OnyxMergeCollectionInput): Promise { if (typeof collection !== 'object' || Array.isArray(collection) || utils.isEmptyObject(collection)) { Logger.logInfo('mergeCollection() called with invalid or empty value. Skipping this update.'); return Promise.resolve(); } - const mergedCollection = collection; + const mergedCollection: OnyxInputKeyValueMapping = collection; // Confirm all the collection keys belong to the same parent let hasCollectionKeyCheckFailed = false; diff --git a/lib/types.ts b/lib/types.ts index 3b098b07..f7e491cc 100644 --- a/lib/types.ts +++ b/lib/types.ts @@ -358,7 +358,7 @@ type OnyxMergeInput = OnyxInput; /** * This represents the value that can be passed to `Onyx.merge` and to `Onyx.update` with the method "MERGE" */ -type OnyxMergeCollectionInput = OnyxCollectionInputValue>; +type OnyxMergeCollectionInput = Collection>, TMap>; /** * Represents different kinds of updates that can be passed to `Onyx.update()` method. It is a discriminated union of From 2cabca8264b556bed6f0b6af05e030ac20d4a420 Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Thu, 6 Jun 2024 12:00:51 +0200 Subject: [PATCH 23/23] add back @ts-expect-error --- tests/unit/useOnyxTest.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/unit/useOnyxTest.ts b/tests/unit/useOnyxTest.ts index d45eef97..04160c51 100644 --- a/tests/unit/useOnyxTest.ts +++ b/tests/unit/useOnyxTest.ts @@ -163,6 +163,7 @@ describe('useOnyx', () => { }); it('should return selected data from a collection key', async () => { + // @ts-expect-error bypass Onyx.mergeCollection(ONYXKEYS.COLLECTION.TEST_KEY, { [`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`]: {id: 'entry1_id', name: 'entry1_name'}, [`${ONYXKEYS.COLLECTION.TEST_KEY}entry2`]: {id: 'entry2_id', name: 'entry2_name'}, @@ -195,6 +196,7 @@ describe('useOnyx', () => { }); it('should not change selected data if a property outside the selector was changed', async () => { + // @ts-expect-error bypass Onyx.mergeCollection(ONYXKEYS.COLLECTION.TEST_KEY, { [`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`]: {id: 'entry1_id', name: 'entry1_name'}, [`${ONYXKEYS.COLLECTION.TEST_KEY}entry2`]: {id: 'entry2_id', name: 'entry2_name'},