From 23a135c2d3d0282cd28b2bd735e88194eedba0a8 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Tue, 4 Oct 2022 17:38:24 -1000 Subject: [PATCH 01/11] Use simple deep equal idea instead --- lib/Onyx.js | 17 ++++++++++---- lib/OnyxCache.js | 11 +++++++++ lib/deepIsEqual.js | 58 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 82 insertions(+), 4 deletions(-) create mode 100644 lib/deepIsEqual.js diff --git a/lib/Onyx.js b/lib/Onyx.js index 0337555e8..8138c5f80 100644 --- a/lib/Onyx.js +++ b/lib/Onyx.js @@ -359,8 +359,9 @@ function keysChanged(collectionKey, partialCollection) { * @private * @param {String} key * @param {*} data + * @param {Boolean} onlyUpdateInitWithStoredValuesFalse */ -function keyChanged(key, data) { +function keyChanged(key, data, onlyUpdateInitWithStoredValuesFalse = false) { // Add or remove this key from the recentlyAccessedKeys lists if (!_.isNull(data)) { addLastAccessedKey(key); @@ -374,7 +375,7 @@ function keyChanged(key, data) { const stateMappingKeys = _.keys(callbackToStateMapping); for (let i = stateMappingKeys.length; i--;) { const subscriber = callbackToStateMapping[stateMappingKeys[i]]; - if (!subscriber || !isKeyMatch(subscriber.key, key)) { + if (!subscriber || !isKeyMatch(subscriber.key, key) || (onlyUpdateInitWithStoredValuesFalse && subscriber.initWithStoredValues !== false)) { continue; } @@ -611,10 +612,11 @@ function disconnect(connectionID, keyToRemoveFromEvictionBlocklist) { * * @param {String} key * @param {*} value + * @param {Boolean} [onlyUpdateInitWithStoredValuesFalse] */ // eslint-disable-next-line rulesdir/no-negated-variables -function notifySubscribersOnNextTick(key, value) { - Promise.resolve().then(() => keyChanged(key, value)); +function notifySubscribersOnNextTick(key, value, onlyUpdateInitWithStoredValuesFalse = false) { + Promise.resolve().then(() => keyChanged(key, value, onlyUpdateInitWithStoredValuesFalse)); } /** @@ -681,6 +683,13 @@ function set(key, value) { Logger.logAlert(`Onyx.set() called after Onyx.merge() for key: ${key}. It is recommended to use set() or merge() not both.`); } + // If the value in the cache is the same as what we have then do not update subscribers unless they + // have initWithStoredValues: false then they MUST get all updates even if nothing has changed. + if (!cache.hasValueChanged(key, value)) { + notifySubscribersOnNextTick(key, value, true); + return; + } + // Adds the key to cache when it's not available cache.set(key, value); notifySubscribersOnNextTick(key, value); diff --git a/lib/OnyxCache.js b/lib/OnyxCache.js index 35a2eaaa3..c6f21deab 100644 --- a/lib/OnyxCache.js +++ b/lib/OnyxCache.js @@ -1,5 +1,6 @@ import _ from 'underscore'; import mergeWithCustomized from './mergeWithCustomized'; +import deepIsEqual from './deepIsEqual'; const isDefined = _.negate(_.isUndefined); @@ -189,6 +190,16 @@ class OnyxCache { setRecentKeysLimit(limit) { this.maxRecentKeysSize = limit; } + + /** + * @param {String} key + * @param {*} value + * @returns {Boolean} + */ + hasValueChanged(key, value) { + this.addToAccessedKeys(key); + return !deepIsEqual(this.storageMap[key], value); + } } const instance = new OnyxCache(); diff --git a/lib/deepIsEqual.js b/lib/deepIsEqual.js new file mode 100644 index 000000000..0a7111115 --- /dev/null +++ b/lib/deepIsEqual.js @@ -0,0 +1,58 @@ +import _ from 'underscore'; + +/** + * @param {*} oldValue + * @param {*} newValue + * @returns {Boolean} + */ +export default function deepIsEqual(oldValue, newValue) { + if (newValue === oldValue) { + return true; + } + + if ((newValue === null && oldValue === undefined) || (oldValue === null && newValue === undefined)) { + return false; + } + + if ((newValue !== null && newValue !== undefined) && (oldValue === null || oldValue === undefined)) { + return false; + } + + if ((oldValue !== null && oldValue !== undefined) && (newValue === null || newValue === undefined)) { + return false; + } + + const newValueProperties = Object.getOwnPropertyNames(newValue); + const oldValueProperties = Object.getOwnPropertyNames(oldValue); + + if (_.isNumber(oldValue) && _.isNumber(newValue)) { + return oldValue === newValue; + } + + if (newValueProperties.length !== oldValueProperties.length) { + return false; + } + + for (let i = 0; i < newValueProperties.length; i++) { + const property = newValueProperties[i]; + if (_.isObject(newValue[property])) { + // eslint-disable-next-line no-unused-vars + if (!deepIsEqual(newValue[property], oldValue[property])) { + return false; + } + break; + } + + if (_.isNumber(newValue[property])) { + if (_.isNaN(newValue[property]) && _.isNaN(oldValue[property])) { + break; + } + } + + if (newValue[property] !== oldValue[property]) { + return false; + } + } + + return true; +} From 5e93a95e6cbcfa5f14c71f62f514bad039b1328f Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Tue, 4 Oct 2022 17:48:26 -1000 Subject: [PATCH 02/11] just use underscore --- lib/OnyxCache.js | 3 +-- lib/deepIsEqual.js | 58 ---------------------------------------------- 2 files changed, 1 insertion(+), 60 deletions(-) delete mode 100644 lib/deepIsEqual.js diff --git a/lib/OnyxCache.js b/lib/OnyxCache.js index c6f21deab..0e4cac5b2 100644 --- a/lib/OnyxCache.js +++ b/lib/OnyxCache.js @@ -1,6 +1,5 @@ import _ from 'underscore'; import mergeWithCustomized from './mergeWithCustomized'; -import deepIsEqual from './deepIsEqual'; const isDefined = _.negate(_.isUndefined); @@ -198,7 +197,7 @@ class OnyxCache { */ hasValueChanged(key, value) { this.addToAccessedKeys(key); - return !deepIsEqual(this.storageMap[key], value); + return !_.isEqual(this.storageMap[key], value); } } diff --git a/lib/deepIsEqual.js b/lib/deepIsEqual.js deleted file mode 100644 index 0a7111115..000000000 --- a/lib/deepIsEqual.js +++ /dev/null @@ -1,58 +0,0 @@ -import _ from 'underscore'; - -/** - * @param {*} oldValue - * @param {*} newValue - * @returns {Boolean} - */ -export default function deepIsEqual(oldValue, newValue) { - if (newValue === oldValue) { - return true; - } - - if ((newValue === null && oldValue === undefined) || (oldValue === null && newValue === undefined)) { - return false; - } - - if ((newValue !== null && newValue !== undefined) && (oldValue === null || oldValue === undefined)) { - return false; - } - - if ((oldValue !== null && oldValue !== undefined) && (newValue === null || newValue === undefined)) { - return false; - } - - const newValueProperties = Object.getOwnPropertyNames(newValue); - const oldValueProperties = Object.getOwnPropertyNames(oldValue); - - if (_.isNumber(oldValue) && _.isNumber(newValue)) { - return oldValue === newValue; - } - - if (newValueProperties.length !== oldValueProperties.length) { - return false; - } - - for (let i = 0; i < newValueProperties.length; i++) { - const property = newValueProperties[i]; - if (_.isObject(newValue[property])) { - // eslint-disable-next-line no-unused-vars - if (!deepIsEqual(newValue[property], oldValue[property])) { - return false; - } - break; - } - - if (_.isNumber(newValue[property])) { - if (_.isNaN(newValue[property]) && _.isNaN(oldValue[property])) { - break; - } - } - - if (newValue[property] !== oldValue[property]) { - return false; - } - } - - return true; -} From 0e63c889b78529dfea5e2a53445405146fa63759 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Wed, 5 Oct 2022 09:19:40 -1000 Subject: [PATCH 03/11] Add comments for onlyUpdateSubscribersWithoutInitialValues --- lib/Onyx.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/Onyx.js b/lib/Onyx.js index 68713bbf8..30b341ebc 100644 --- a/lib/Onyx.js +++ b/lib/Onyx.js @@ -359,9 +359,11 @@ function keysChanged(collectionKey, partialCollection) { * @private * @param {String} key * @param {*} data - * @param {Boolean} onlyUpdateInitWithStoredValuesFalse + * @param {Boolean} onlyUpdateSubscribersWithoutInitialValues - When true we will only update the subscribers that have elected to initialize without the values from storage. + * This is used when we skip updates to subscribers because the values in the cache have not changed. We don't want to skip updates to the subscribers that did not get the + * data when they connected or else they will only get new data when the value changes. */ -function keyChanged(key, data, onlyUpdateInitWithStoredValuesFalse = false) { +function keyChanged(key, data, onlyUpdateSubscribersWithoutInitialValues = false) { // Add or remove this key from the recentlyAccessedKeys lists if (!_.isNull(data)) { addLastAccessedKey(key); @@ -375,7 +377,7 @@ function keyChanged(key, data, onlyUpdateInitWithStoredValuesFalse = false) { const stateMappingKeys = _.keys(callbackToStateMapping); for (let i = stateMappingKeys.length; i--;) { const subscriber = callbackToStateMapping[stateMappingKeys[i]]; - if (!subscriber || !isKeyMatch(subscriber.key, key) || (onlyUpdateInitWithStoredValuesFalse && subscriber.initWithStoredValues !== false)) { + if (!subscriber || !isKeyMatch(subscriber.key, key) || (onlyUpdateSubscribersWithoutInitialValues && subscriber.initWithStoredValues !== false)) { continue; } From 8c7b7d96757892ed6d982f478dc8dbe5ddc9394e Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Wed, 5 Oct 2022 09:28:35 -1000 Subject: [PATCH 04/11] Use a filter for the subscribers --- lib/Onyx.js | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/lib/Onyx.js b/lib/Onyx.js index 30b341ebc..3bb3ed48d 100644 --- a/lib/Onyx.js +++ b/lib/Onyx.js @@ -359,11 +359,9 @@ function keysChanged(collectionKey, partialCollection) { * @private * @param {String} key * @param {*} data - * @param {Boolean} onlyUpdateSubscribersWithoutInitialValues - When true we will only update the subscribers that have elected to initialize without the values from storage. - * This is used when we skip updates to subscribers because the values in the cache have not changed. We don't want to skip updates to the subscribers that did not get the - * data when they connected or else they will only get new data when the value changes. + * @param {Function} [canUpdateSubscriber] only subscribers that pass this truth test will be updated */ -function keyChanged(key, data, onlyUpdateSubscribersWithoutInitialValues = false) { +function keyChanged(key, data, canUpdateSubscriber) { // Add or remove this key from the recentlyAccessedKeys lists if (!_.isNull(data)) { addLastAccessedKey(key); @@ -377,7 +375,7 @@ function keyChanged(key, data, onlyUpdateSubscribersWithoutInitialValues = false const stateMappingKeys = _.keys(callbackToStateMapping); for (let i = stateMappingKeys.length; i--;) { const subscriber = callbackToStateMapping[stateMappingKeys[i]]; - if (!subscriber || !isKeyMatch(subscriber.key, key) || (onlyUpdateSubscribersWithoutInitialValues && subscriber.initWithStoredValues !== false)) { + if (!subscriber || !isKeyMatch(subscriber.key, key) || (_.isFunction(canUpdateSubscriber) && !canUpdateSubscriber(subscriber))) { continue; } @@ -614,11 +612,11 @@ function disconnect(connectionID, keyToRemoveFromEvictionBlocklist) { * * @param {String} key * @param {*} value - * @param {Boolean} [onlyUpdateInitWithStoredValuesFalse] + * @param {Function} [canUpdateSubscriber] only subscribers that pass this truth test will be updated */ // eslint-disable-next-line rulesdir/no-negated-variables -function notifySubscribersOnNextTick(key, value, onlyUpdateInitWithStoredValuesFalse = false) { - Promise.resolve().then(() => keyChanged(key, value, onlyUpdateInitWithStoredValuesFalse)); +function notifySubscribersOnNextTick(key, value, canUpdateSubscriber) { + Promise.resolve().then(() => keyChanged(key, value, canUpdateSubscriber)); } /** @@ -688,7 +686,7 @@ function set(key, value) { // If the value in the cache is the same as what we have then do not update subscribers unless they // have initWithStoredValues: false then they MUST get all updates even if nothing has changed. if (!cache.hasValueChanged(key, value)) { - notifySubscribersOnNextTick(key, value, true); + notifySubscribersOnNextTick(key, value, subscriber => subscriber.initWithStoredValues === false); return; } From 2fbf183c952a306685f91c46e5c105d8fe91a6f3 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Wed, 5 Oct 2022 09:33:13 -1000 Subject: [PATCH 05/11] Add test for preventing updates when values do not change --- tests/unit/onyxTest.js | 46 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/tests/unit/onyxTest.js b/tests/unit/onyxTest.js index c0c900d03..8202db5da 100644 --- a/tests/unit/onyxTest.js +++ b/tests/unit/onyxTest.js @@ -1,3 +1,4 @@ +import _ from 'underscore'; import Onyx from '../../lib'; import waitForPromisesToResolve from '../utils/waitForPromisesToResolve'; @@ -630,4 +631,49 @@ describe('Onyx', () => { expect(mockCallback.mock.calls[1][0]).toEqual(collectionUpdate); }); }); + + it('should not update a subscriber if the value in the cache has not changed at all', () => { + const mockCallback = jest.fn(); + const collectionUpdate = { + test_policy_1: {ID: 234, value: 'one'}, + }; + + // Given an Onyx.connect call with waitForCollectionCallback=true + connectionID = Onyx.connect({ + key: ONYX_KEYS.COLLECTION.TEST_POLICY, + waitForCollectionCallback: true, + callback: mockCallback, + }); + return waitForPromisesToResolve() + .then(() => { + // When merge is called with an updated collection + Onyx.merge(`${ONYX_KEYS.COLLECTION.TEST_POLICY}${1}`, collectionUpdate.test_policy_1); + return waitForPromisesToResolve(); + }) + .then(() => { + // Then we expect the callback to have called twice, once for the initial connect call + once for the collection update + expect(mockCallback.mock.calls.length).toBe(2); + + // And the value for the second call should be collectionUpdate + expect(mockCallback.mock.calls[1][0]).toEqual(collectionUpdate); + }) + .then(() => { + // When merge is called again with the same collection not modified + Onyx.merge(`${ONYX_KEYS.COLLECTION.TEST_POLICY}${1}`, collectionUpdate.test_policy_1); + return waitForPromisesToResolve(); + }) + .then(() => { + // Then we should not expect another invocation of the callback + expect(mockCallback.mock.calls.length).toBe(2); + }) + .then(() => { + // WHEN merge is called again with an object of equivalent value but not the same reference + Onyx.merge(`${ONYX_KEYS.COLLECTION.TEST_POLICY}${1}`, _.clone(collectionUpdate.test_policy_1)); + return waitForPromisesToResolve(); + }) + .then(() => { + // Then we should not expect another invocation of the callback + expect(mockCallback.mock.calls.length).toBe(2); + }); + }); }); From 1ee97daef0507f65902190dbb6125dc685e4c63a Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Wed, 5 Oct 2022 09:36:06 -1000 Subject: [PATCH 06/11] Add new test --- tests/unit/onyxTest.js | 46 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/tests/unit/onyxTest.js b/tests/unit/onyxTest.js index 8202db5da..e42703853 100644 --- a/tests/unit/onyxTest.js +++ b/tests/unit/onyxTest.js @@ -676,4 +676,50 @@ describe('Onyx', () => { expect(mockCallback.mock.calls.length).toBe(2); }); }); + + it('should update subscriber if the value in the cache has not changed at all but initWithStoredValues === false', () => { + const mockCallback = jest.fn(); + const collectionUpdate = { + test_policy_1: {ID: 234, value: 'one'}, + }; + + // Given an Onyx.connect call with waitForCollectionCallback=true + connectionID = Onyx.connect({ + key: ONYX_KEYS.COLLECTION.TEST_POLICY, + waitForCollectionCallback: true, + callback: mockCallback, + initWithStoredValues: false, + }); + return waitForPromisesToResolve() + .then(() => { + // When merge is called with an updated collection + Onyx.merge(`${ONYX_KEYS.COLLECTION.TEST_POLICY}${1}`, collectionUpdate.test_policy_1); + return waitForPromisesToResolve(); + }) + .then(() => { + // Then we expect the callback to have called once. 0 times the initial connect call + 1 time for the merge() + expect(mockCallback.mock.calls.length).toBe(1); + + // And the value for the second call should be collectionUpdate + expect(mockCallback.mock.calls[0][0]).toEqual(collectionUpdate); + }) + .then(() => { + // When merge is called again with the same collection not modified + Onyx.merge(`${ONYX_KEYS.COLLECTION.TEST_POLICY}${1}`, collectionUpdate.test_policy_1); + return waitForPromisesToResolve(); + }) + .then(() => { + // Then we should expect another invocation of the callback because initWithStoredValues = false + expect(mockCallback.mock.calls.length).toBe(2); + }) + .then(() => { + // WHEN merge is called again with an object of equivalent value but not the same reference + Onyx.merge(`${ONYX_KEYS.COLLECTION.TEST_POLICY}${1}`, _.clone(collectionUpdate.test_policy_1)); + return waitForPromisesToResolve(); + }) + .then(() => { + // Then we should expect another invocation of the callback because initWithStoredValues = false + expect(mockCallback.mock.calls.length).toBe(3); + }); + }); }); From 17e09034bab94f41fc3ecbc00480940201f65f4e Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Wed, 5 Oct 2022 09:36:51 -1000 Subject: [PATCH 07/11] Update docs --- API.md | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/API.md b/API.md index 4b0155187..d4f842943 100644 --- a/API.md +++ b/API.md @@ -13,7 +13,7 @@
disconnect(connectionID, [keyToRemoveFromEvictionBlocklist])

Remove the listener for a react component

-
notifySubscribersOnNextTick(key, value)
+
notifySubscribersOnNextTick(key, value, [canUpdateSubscriber])

This method mostly exists for historical reasons as this library was initially designed without a memory cache and one was added later. For this reason, Onyx works more similar to what you might expect from a native AsyncStorage with reads, writes, etc all becoming available async. Since we have code in our main applications that might expect things to work this way it's not safe to change this @@ -115,7 +115,7 @@ Onyx.disconnect(connectionID); ``` -## notifySubscribersOnNextTick(key, value) +## notifySubscribersOnNextTick(key, value, [canUpdateSubscriber]) This method mostly exists for historical reasons as this library was initially designed without a memory cache and one was added later. For this reason, Onyx works more similar to what you might expect from a native AsyncStorage with reads, writes, etc all becoming available async. Since we have code in our main applications that might expect things to work this way it's not safe to change this @@ -123,10 +123,11 @@ behavior just yet. **Kind**: global function -| Param | Type | -| --- | --- | -| key | String | -| value | \* | +| Param | Type | Description | +| --- | --- | --- | +| key | String | | +| value | \* | | +| [canUpdateSubscriber] | function | only subscribers that pass this truth test will be updated | From d049e6d332a3c6597ef474f3c46e6e2ee14e90af Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Wed, 5 Oct 2022 11:32:18 -1000 Subject: [PATCH 08/11] Clean up tests --- tests/unit/onyxCacheTest.js | 6 +-- tests/unit/onyxTest.js | 93 ++++++++++++++----------------------- tests/unit/withOnyxTest.js | 34 ++++++++------ 3 files changed, 56 insertions(+), 77 deletions(-) diff --git a/tests/unit/onyxCacheTest.js b/tests/unit/onyxCacheTest.js index 704cacaf6..393738f6f 100644 --- a/tests/unit/onyxCacheTest.js +++ b/tests/unit/onyxCacheTest.js @@ -613,10 +613,8 @@ describe('Onyx', () => { .then(() => { // Then Async storage `getItem` should be called exactly two times (once for each key) expect(AsyncStorageMock.getItem).toHaveBeenCalledTimes(2); - expect(AsyncStorageMock.getItem.mock.calls).toEqual([ - [ONYX_KEYS.TEST_KEY], - [ONYX_KEYS.OTHER_TEST], - ]); + expect(AsyncStorageMock.getItem).toHaveBeenNthCalledWith(1, ONYX_KEYS.TEST_KEY); + expect(AsyncStorageMock.getItem).toHaveBeenNthCalledWith(2, ONYX_KEYS.OTHER_TEST); }); }); diff --git a/tests/unit/onyxTest.js b/tests/unit/onyxTest.js index e42703853..43e1d3942 100644 --- a/tests/unit/onyxTest.js +++ b/tests/unit/onyxTest.js @@ -264,29 +264,14 @@ describe('Onyx', () => { )) .then(() => { // 3 items on the first mergeCollection + 4 items the next mergeCollection - expect(mockCallback.mock.calls.length).toBe(7); - - expect(mockCallback.mock.calls[0][0]).toEqual({ID: 123, value: 'one'}); - expect(mockCallback.mock.calls[0][1]).toEqual('test_1'); - - expect(mockCallback.mock.calls[1][0]).toEqual({ID: 234, value: 'two'}); - expect(mockCallback.mock.calls[1][1]).toEqual('test_2'); - - expect(mockCallback.mock.calls[2][0]).toEqual({ID: 345, value: 'three'}); - expect(mockCallback.mock.calls[2][1]).toEqual('test_3'); - - expect(mockCallback.mock.calls[3][0]).toEqual({ID: 123, value: 'five'}); - expect(mockCallback.mock.calls[3][1]).toEqual('test_1'); - - expect(mockCallback.mock.calls[4][0]).toEqual({ID: 234, value: 'four'}); - expect(mockCallback.mock.calls[4][1]).toEqual('test_2'); - - expect(mockCallback.mock.calls[5][0]).toEqual({ID: 456, value: 'two'}); - expect(mockCallback.mock.calls[5][1]).toEqual('test_4'); - - expect(mockCallback.mock.calls[6][0]).toEqual({ID: 567, value: 'one'}); - expect(mockCallback.mock.calls[6][1]).toEqual('test_5'); - + expect(mockCallback).toHaveBeenCalledTimes(7); + expect(mockCallback).toHaveBeenNthCalledWith(1, {ID: 123, value: 'one'}, 'test_1'); + expect(mockCallback).toHaveBeenNthCalledWith(2, {ID: 234, value: 'two'}, 'test_2'); + expect(mockCallback).toHaveBeenNthCalledWith(3, {ID: 345, value: 'three'}, 'test_3'); + expect(mockCallback).toHaveBeenNthCalledWith(4, {ID: 123, value: 'five'}, 'test_1'); + expect(mockCallback).toHaveBeenNthCalledWith(5, {ID: 234, value: 'four'}, 'test_2'); + expect(mockCallback).toHaveBeenNthCalledWith(6, {ID: 456, value: 'two'}, 'test_4'); + expect(mockCallback).toHaveBeenNthCalledWith(7, {ID: 567, value: 'one'}, 'test_5'); expect(valuesReceived[123]).toEqual('five'); expect(valuesReceived[234]).toEqual('four'); expect(valuesReceived[345]).toEqual('three'); @@ -420,11 +405,8 @@ describe('Onyx', () => { return waitForPromisesToResolve(); }) .then(() => { - expect(mockCallback.mock.calls[0][0]).toEqual({existingData: 'test'}); - expect(mockCallback.mock.calls[0][1]).toEqual('test_1'); - - expect(mockCallback.mock.calls[1][0]).toEqual({existingData: 'test'}); - expect(mockCallback.mock.calls[1][1]).toEqual('test_2'); + expect(mockCallback).toHaveBeenNthCalledWith(1, {existingData: 'test'}, 'test_1'); + expect(mockCallback).toHaveBeenNthCalledWith(2, {existingData: 'test'}, 'test_2'); // WHEN we pass a mergeCollection data object to Onyx.update Onyx.update([ @@ -469,14 +451,9 @@ describe('Onyx', () => { } */ - expect(mockCallback.mock.calls[2][0]).toEqual({ID: 123, value: 'one', existingData: 'test'}); - expect(mockCallback.mock.calls[2][1]).toEqual('test_1'); - - expect(mockCallback.mock.calls[3][0]).toEqual({ID: 234, value: 'two', existingData: 'test'}); - expect(mockCallback.mock.calls[3][1]).toEqual('test_2'); - - expect(mockCallback.mock.calls[4][0]).toEqual({ID: 345, value: 'three'}); - expect(mockCallback.mock.calls[4][1]).toEqual('test_3'); + expect(mockCallback).toHaveBeenNthCalledWith(3, {ID: 123, value: 'one', existingData: 'test'}, 'test_1'); + expect(mockCallback).toHaveBeenNthCalledWith(4, {ID: 234, value: 'two', existingData: 'test'}, 'test_2'); + expect(mockCallback).toHaveBeenNthCalledWith(5, {ID: 345, value: 'three'}, 'test_3'); }); }); @@ -539,8 +516,8 @@ describe('Onyx', () => { }) .then(() => { // Then we expect the callback to be called only once and the initial stored value to be initialCollectionData - expect(mockCallback.mock.calls.length).toBe(1); - expect(mockCallback.mock.calls[0][0]).toEqual(initialCollectionData); + expect(mockCallback).toHaveBeenCalledTimes(1); + expect(mockCallback).toHaveBeenCalledWith(initialCollectionData, undefined); }); }); @@ -565,13 +542,13 @@ describe('Onyx', () => { }) .then(() => { // Then we expect the callback to have called twice, once for the initial connect call + once for the collection update - expect(mockCallback.mock.calls.length).toBe(2); + 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.mock.calls[0][0]).toBe(null); + expect(mockCallback).toHaveBeenNthCalledWith(1, null, undefined); // AND the value for the second call should be collectionUpdate since the collection was updated - expect(mockCallback.mock.calls[1][0]).toEqual(collectionUpdate); + expect(mockCallback).toHaveBeenNthCalledWith(2, collectionUpdate); }); }); @@ -595,13 +572,13 @@ describe('Onyx', () => { }) .then(() => { // Then we expect the callback to have called twice, once for the initial connect call + once for the collection update - expect(mockCallback.mock.calls.length).toBe(2); + 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.mock.calls[0][0]).toBe(null); + expect(mockCallback).toHaveBeenNthCalledWith(1, null, 'test_policy_1'); // AND the value for the second call should be collectionUpdate since the collection was updated - expect(mockCallback.mock.calls[1][0]).toEqual(collectionUpdate.test_policy_1); + expect(mockCallback).toHaveBeenNthCalledWith(2, collectionUpdate.test_policy_1, 'test_policy_1'); }); }); @@ -618,17 +595,15 @@ describe('Onyx', () => { callback: mockCallback, }); return waitForPromisesToResolve() - .then(() => { - // WHEN mergeCollection is called with an updated collection - Onyx.merge(`${ONYX_KEYS.COLLECTION.TEST_POLICY}${1}`, collectionUpdate.test_policy_1); - return waitForPromisesToResolve(); - }) + + // WHEN mergeCollection is called with an updated collection + .then(() => Onyx.merge(`${ONYX_KEYS.COLLECTION.TEST_POLICY}${1}`, collectionUpdate.test_policy_1)) .then(() => { // Then we expect the callback to have called twice, once for the initial connect call + once for the collection update - expect(mockCallback.mock.calls.length).toBe(2); + expect(mockCallback).toHaveBeenCalledTimes(2); // AND the value for the second call should be collectionUpdate - expect(mockCallback.mock.calls[1][0]).toEqual(collectionUpdate); + expect(mockCallback).toHaveBeenLastCalledWith(collectionUpdate); }); }); @@ -652,10 +627,10 @@ describe('Onyx', () => { }) .then(() => { // Then we expect the callback to have called twice, once for the initial connect call + once for the collection update - expect(mockCallback.mock.calls.length).toBe(2); + expect(mockCallback).toHaveBeenCalledTimes(2); // And the value for the second call should be collectionUpdate - expect(mockCallback.mock.calls[1][0]).toEqual(collectionUpdate); + expect(mockCallback).toHaveBeenNthCalledWith(2, collectionUpdate); }) .then(() => { // When merge is called again with the same collection not modified @@ -664,7 +639,7 @@ describe('Onyx', () => { }) .then(() => { // Then we should not expect another invocation of the callback - expect(mockCallback.mock.calls.length).toBe(2); + expect(mockCallback).toHaveBeenCalledTimes(2); }) .then(() => { // WHEN merge is called again with an object of equivalent value but not the same reference @@ -673,7 +648,7 @@ describe('Onyx', () => { }) .then(() => { // Then we should not expect another invocation of the callback - expect(mockCallback.mock.calls.length).toBe(2); + expect(mockCallback).toHaveBeenCalledTimes(2); }); }); @@ -698,10 +673,10 @@ describe('Onyx', () => { }) .then(() => { // Then we expect the callback to have called once. 0 times the initial connect call + 1 time for the merge() - expect(mockCallback.mock.calls.length).toBe(1); + expect(mockCallback).toHaveBeenCalledTimes(1); // And the value for the second call should be collectionUpdate - expect(mockCallback.mock.calls[0][0]).toEqual(collectionUpdate); + expect(mockCallback).toHaveBeenNthCalledWith(1, collectionUpdate); }) .then(() => { // When merge is called again with the same collection not modified @@ -710,7 +685,7 @@ describe('Onyx', () => { }) .then(() => { // Then we should expect another invocation of the callback because initWithStoredValues = false - expect(mockCallback.mock.calls.length).toBe(2); + expect(mockCallback).toHaveBeenCalledTimes(2); }) .then(() => { // WHEN merge is called again with an object of equivalent value but not the same reference @@ -719,7 +694,7 @@ describe('Onyx', () => { }) .then(() => { // Then we should expect another invocation of the callback because initWithStoredValues = false - expect(mockCallback.mock.calls.length).toBe(3); + expect(mockCallback).toHaveBeenCalledTimes(3); }); }); }); diff --git a/tests/unit/withOnyxTest.js b/tests/unit/withOnyxTest.js index 94efa1f08..e3a104357 100644 --- a/tests/unit/withOnyxTest.js +++ b/tests/unit/withOnyxTest.js @@ -56,7 +56,7 @@ describe('withOnyx', () => { Onyx.merge(`${ONYX_KEYS.COLLECTION.TEST_KEY}3`, {ID: 345}); return waitForPromisesToResolve() .then(() => { - expect(onRender.mock.calls.length).toBe(4); + expect(onRender).toHaveBeenCalledTimes(4); }); }); @@ -76,7 +76,7 @@ describe('withOnyx', () => { return waitForPromisesToResolve(); }) .then(() => { - expect(onRender.mock.calls.length).toBe(2); + expect(onRender).toHaveBeenCalledTimes(2); }); }); @@ -97,7 +97,7 @@ describe('withOnyx', () => { return waitForPromisesToResolve(); }) .then(() => { - expect(onRender.mock.calls.length).toBe(2); + expect(onRender).toHaveBeenCalledTimes(2); }); }); @@ -124,8 +124,10 @@ describe('withOnyx', () => { return waitForPromisesToResolve(); }) .then(() => { - expect(onRender.mock.calls.length).toBe(3); - expect(onRender.mock.instances[2].text).toEqual({list: [7]}); + expect(onRender).toHaveBeenCalledTimes(3); + expect(onRender).toHaveBeenLastCalledWith({ + collections: {}, onRender, testObject: {isDefaultProp: true}, text: {list: [7]}, + }); }); }); @@ -149,8 +151,10 @@ describe('withOnyx', () => { return waitForPromisesToResolve(); }) .then(() => { - expect(onRender.mock.calls.length).toBe(3); - expect(onRender.mock.instances[2].text).toEqual({ID: 456, Name: 'Test4'}); + expect(onRender).toHaveBeenCalledTimes(3); + expect(onRender).toHaveBeenLastCalledWith({ + collections: {}, onRender, testObject: {isDefaultProp: true}, text: {ID: 456, Name: 'Test4'}, + }); }); }); @@ -226,7 +230,9 @@ describe('withOnyx', () => { return waitForPromisesToResolve(); }) .then(() => { - expect(onRender.mock.instances[0].testThing).toBe('Test'); + expect(onRender).toHaveBeenLastCalledWith({ + collections: {}, onRender, testObject: {id: 1}, testThing: 'Test', + }); }); }); @@ -281,14 +287,14 @@ describe('withOnyx', () => { // Note: each component is rendered twice. Once when it is initially rendered, and then again // when the collection is updated. That's why there are two checks here for each component. expect(onRender1).toHaveBeenCalledTimes(2); - expect(onRender1.mock.calls[0][0].testObject).toStrictEqual({ID: 1}); - expect(onRender1.mock.calls[1][0].testObject).toStrictEqual({ID: 1, newProperty: 'yay'}); + expect(onRender1).toHaveBeenNthCalledWith(1, {collections: {}, onRender: onRender1, testObject: {ID: 1}}); + expect(onRender1).toHaveBeenNthCalledWith(2, {collections: {}, onRender: onRender1, testObject: {ID: 1, newProperty: 'yay'}}); expect(onRender2).toHaveBeenCalledTimes(1); - expect(onRender2.mock.calls[0][0].testObject).toStrictEqual({ID: 2}); + expect(onRender2).toHaveBeenNthCalledWith(1, {collections: {}, onRender: onRender2, testObject: {ID: 2}}); expect(onRender3).toHaveBeenCalledTimes(1); - expect(onRender3.mock.calls[0][0].testObject).toStrictEqual({ID: 3}); + expect(onRender3).toHaveBeenNthCalledWith(1, {collections: {}, onRender: onRender3, testObject: {ID: 3}}); }); }); @@ -324,8 +330,8 @@ describe('withOnyx', () => { // The first time it will render with number === 1 // The second time it will render with number === 2 expect(onRender).toHaveBeenCalledTimes(2); - expect(onRender.mock.calls[0][0].testObject).toStrictEqual({ID: 1, number: 1}); - expect(onRender.mock.calls[1][0].testObject).toStrictEqual({ID: 1, number: 2}); + expect(onRender).toHaveBeenNthCalledWith(1, {collections: {}, onRender, testObject: {ID: 1, number: 1}}); + expect(onRender).toHaveBeenNthCalledWith(2, {collections: {}, onRender, testObject: {ID: 1, number: 2}}); }); }); }); From 194e555a50a8bbb6cbb53c2c257a6c32d3e3f196 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Wed, 5 Oct 2022 11:37:20 -1000 Subject: [PATCH 09/11] Clear up some waitForPromisesToResolve --- tests/unit/onyxTest.js | 65 ++++++++++++++++-------------------------- 1 file changed, 25 insertions(+), 40 deletions(-) diff --git a/tests/unit/onyxTest.js b/tests/unit/onyxTest.js index 43e1d3942..27a7d3b86 100644 --- a/tests/unit/onyxTest.js +++ b/tests/unit/onyxTest.js @@ -60,8 +60,7 @@ describe('Onyx', () => { return Onyx.set(ONYX_KEYS.TEST_KEY, {test1: 'test1'}) .then(() => { expect(testKeyValue).toEqual({test1: 'test1'}); - Onyx.merge(ONYX_KEYS.TEST_KEY, {test2: 'test2'}); - return waitForPromisesToResolve(); + return Onyx.merge(ONYX_KEYS.TEST_KEY, {test2: 'test2'}); }) .then(() => { expect(testKeyValue).toEqual({test1: 'test1', test2: 'test2'}); @@ -135,8 +134,7 @@ describe('Onyx', () => { return Onyx.set(ONYX_KEYS.TEST_KEY, ['test1']) .then(() => { expect(testKeyValue).toStrictEqual(['test1']); - Onyx.merge(ONYX_KEYS.TEST_KEY, ['test2', 'test3', 'test4']); - return waitForPromisesToResolve(); + return Onyx.merge(ONYX_KEYS.TEST_KEY, ['test2', 'test3', 'test4']); }) .then(() => { expect(testKeyValue).toStrictEqual(['test1', 'test2', 'test3', 'test4']); @@ -535,11 +533,9 @@ describe('Onyx', () => { callback: mockCallback, }); return waitForPromisesToResolve() - .then(() => { - // WHEN mergeCollection is called with an updated collection - Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_POLICY, collectionUpdate); - return waitForPromisesToResolve(); - }) + + // WHEN mergeCollection is called with an updated collection + .then(() => Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_POLICY, collectionUpdate)) .then(() => { // Then we expect the callback to have called twice, once for the initial connect call + once for the collection update expect(mockCallback).toHaveBeenCalledTimes(2); @@ -565,8 +561,9 @@ describe('Onyx', () => { callback: mockCallback, }); return waitForPromisesToResolve() + + // WHEN mergeCollection is called with an updated collection .then(() => { - // WHEN mergeCollection is called with an updated collection Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_POLICY, collectionUpdate); return waitForPromisesToResolve(); }) @@ -620,11 +617,9 @@ describe('Onyx', () => { callback: mockCallback, }); return waitForPromisesToResolve() - .then(() => { - // When merge is called with an updated collection - Onyx.merge(`${ONYX_KEYS.COLLECTION.TEST_POLICY}${1}`, collectionUpdate.test_policy_1); - return waitForPromisesToResolve(); - }) + + // When merge is called with an updated collection + .then(() => Onyx.merge(`${ONYX_KEYS.COLLECTION.TEST_POLICY}${1}`, collectionUpdate.test_policy_1)) .then(() => { // Then we expect the callback to have called twice, once for the initial connect call + once for the collection update expect(mockCallback).toHaveBeenCalledTimes(2); @@ -632,20 +627,16 @@ describe('Onyx', () => { // And the value for the second call should be collectionUpdate expect(mockCallback).toHaveBeenNthCalledWith(2, collectionUpdate); }) - .then(() => { - // When merge is called again with the same collection not modified - Onyx.merge(`${ONYX_KEYS.COLLECTION.TEST_POLICY}${1}`, collectionUpdate.test_policy_1); - return waitForPromisesToResolve(); - }) + + // When merge is called again with the same collection not modified + .then(() => Onyx.merge(`${ONYX_KEYS.COLLECTION.TEST_POLICY}${1}`, collectionUpdate.test_policy_1)) .then(() => { // Then we should not expect another invocation of the callback expect(mockCallback).toHaveBeenCalledTimes(2); }) - .then(() => { - // WHEN merge is called again with an object of equivalent value but not the same reference - Onyx.merge(`${ONYX_KEYS.COLLECTION.TEST_POLICY}${1}`, _.clone(collectionUpdate.test_policy_1)); - return waitForPromisesToResolve(); - }) + + // WHEN merge is called again with an object of equivalent value but not the same reference + .then(() => Onyx.merge(`${ONYX_KEYS.COLLECTION.TEST_POLICY}${1}`, _.clone(collectionUpdate.test_policy_1))) .then(() => { // Then we should not expect another invocation of the callback expect(mockCallback).toHaveBeenCalledTimes(2); @@ -666,11 +657,9 @@ describe('Onyx', () => { initWithStoredValues: false, }); return waitForPromisesToResolve() - .then(() => { - // When merge is called with an updated collection - Onyx.merge(`${ONYX_KEYS.COLLECTION.TEST_POLICY}${1}`, collectionUpdate.test_policy_1); - return waitForPromisesToResolve(); - }) + + // When merge is called with an updated collection + .then(() => Onyx.merge(`${ONYX_KEYS.COLLECTION.TEST_POLICY}${1}`, collectionUpdate.test_policy_1)) .then(() => { // Then we expect the callback to have called once. 0 times the initial connect call + 1 time for the merge() expect(mockCallback).toHaveBeenCalledTimes(1); @@ -678,20 +667,16 @@ describe('Onyx', () => { // And the value for the second call should be collectionUpdate expect(mockCallback).toHaveBeenNthCalledWith(1, collectionUpdate); }) - .then(() => { - // When merge is called again with the same collection not modified - Onyx.merge(`${ONYX_KEYS.COLLECTION.TEST_POLICY}${1}`, collectionUpdate.test_policy_1); - return waitForPromisesToResolve(); - }) + + // When merge is called again with the same collection not modified + .then(() => Onyx.merge(`${ONYX_KEYS.COLLECTION.TEST_POLICY}${1}`, collectionUpdate.test_policy_1)) .then(() => { // Then we should expect another invocation of the callback because initWithStoredValues = false expect(mockCallback).toHaveBeenCalledTimes(2); }) - .then(() => { - // WHEN merge is called again with an object of equivalent value but not the same reference - Onyx.merge(`${ONYX_KEYS.COLLECTION.TEST_POLICY}${1}`, _.clone(collectionUpdate.test_policy_1)); - return waitForPromisesToResolve(); - }) + + // WHEN merge is called again with an object of equivalent value but not the same reference + .then(() => Onyx.merge(`${ONYX_KEYS.COLLECTION.TEST_POLICY}${1}`, _.clone(collectionUpdate.test_policy_1))) .then(() => { // Then we should expect another invocation of the callback because initWithStoredValues = false expect(mockCallback).toHaveBeenCalledTimes(3); From b24806b6d9c24dc30d85cbd776ef3c582ed80b20 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Wed, 5 Oct 2022 12:40:54 -1000 Subject: [PATCH 10/11] Add test for ensuring that we return a promise from Onyx.set() --- lib/Onyx.js | 2 +- tests/unit/onyxTest.js | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/lib/Onyx.js b/lib/Onyx.js index 3bb3ed48d..1bd101ce7 100644 --- a/lib/Onyx.js +++ b/lib/Onyx.js @@ -687,7 +687,7 @@ function set(key, value) { // have initWithStoredValues: false then they MUST get all updates even if nothing has changed. if (!cache.hasValueChanged(key, value)) { notifySubscribersOnNextTick(key, value, subscriber => subscriber.initWithStoredValues === false); - return; + return Promise.resolve(); } // Adds the key to cache when it's not available diff --git a/tests/unit/onyxTest.js b/tests/unit/onyxTest.js index 27a7d3b86..d1ce3bab7 100644 --- a/tests/unit/onyxTest.js +++ b/tests/unit/onyxTest.js @@ -604,6 +604,16 @@ describe('Onyx', () => { }); }); + it('should return a promise when set() called with the same value and there is no change', () => { + const promiseOne = Onyx.set('test', 'pizza'); + expect(promiseOne).toBeInstanceOf(Promise); + return promiseOne + .then(() => { + const promiseTwo = Onyx.set('test', 'pizza'); + expect(promiseTwo).toBeInstanceOf(Promise); + }); + }); + it('should not update a subscriber if the value in the cache has not changed at all', () => { const mockCallback = jest.fn(); const collectionUpdate = { From 0181cf0088ad9757138912ad6647fbdff52001de Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Thu, 6 Oct 2022 08:49:52 -1000 Subject: [PATCH 11/11] Make requested changes. De-capitalize test comments --- API.md | 5 ++ lib/Onyx.js | 7 ++ lib/OnyxCache.js | 4 +- package-lock.json | 5 ++ package.json | 1 + tests/unit/metricsTest.js | 92 ++++++++++++------------ tests/unit/onyxCacheTest.js | 78 ++++++++++---------- tests/unit/onyxClearNativeStorageTest.js | 24 +++---- tests/unit/onyxClearWebStorageTest.js | 24 +++---- tests/unit/onyxMetricsDecorationTest.js | 18 ++--- tests/unit/onyxTest.js | 20 +++--- tests/unit/withOnyxTest.js | 20 +++--- 12 files changed, 158 insertions(+), 140 deletions(-) diff --git a/API.md b/API.md index d4f842943..efa33602f 100644 --- a/API.md +++ b/API.md @@ -129,6 +129,10 @@ behavior just yet. | value | \* | | | [canUpdateSubscriber] | function | only subscribers that pass this truth test will be updated | +**Example** +```js +notifySubscribersOnNextTick(key, value, subscriber => subscriber.initWithStoredValues === false) +``` ## set(key, value) ⇒ Promise @@ -253,6 +257,7 @@ Initialize the store with actions and listening for storage events | [options.captureMetrics] | Boolean | | Enables Onyx benchmarking and exposes the get/print/reset functions | | [options.shouldSyncMultipleInstances] | Boolean | | Auto synchronize storage events between multiple instances of Onyx running in different tabs/windows. Defaults to true for platforms that support local storage (web/desktop) | | [option.keysToDisableSyncEvents] | Array.<String> | [] | Contains keys for which we want to disable sync event across tabs. | +| [options.debugSetState] | Boolean | | Enables debugging setState() calls to connected components. | **Example** ```js diff --git a/lib/Onyx.js b/lib/Onyx.js index 0e2f9a7ec..a7ff87933 100644 --- a/lib/Onyx.js +++ b/lib/Onyx.js @@ -367,6 +367,9 @@ function keysChanged(collectionKey, partialCollection) { /** * When a key change happens, search for any callbacks matching the key or collection key and trigger those callbacks * + * @example + * keyChanged(key, value, subscriber => subscriber.initWithStoredValues === false) + * * @private * @param {String} key * @param {*} data @@ -632,6 +635,9 @@ function disconnect(connectionID, keyToRemoveFromEvictionBlocklist) { * available async. Since we have code in our main applications that might expect things to work this way it's not safe to change this * behavior just yet. * + * @example + * notifySubscribersOnNextTick(key, value, subscriber => subscriber.initWithStoredValues === false) + * * @param {String} key * @param {*} value * @param {Function} [canUpdateSubscriber] only subscribers that pass this truth test will be updated @@ -708,6 +714,7 @@ function set(key, value) { // If the value in the cache is the same as what we have then do not update subscribers unless they // have initWithStoredValues: false then they MUST get all updates even if nothing has changed. if (!cache.hasValueChanged(key, value)) { + cache.addToAccessedKeys(key); notifySubscribersOnNextTick(key, value, subscriber => subscriber.initWithStoredValues === false); return Promise.resolve(); } diff --git a/lib/OnyxCache.js b/lib/OnyxCache.js index f7ca341c0..39ca01f22 100644 --- a/lib/OnyxCache.js +++ b/lib/OnyxCache.js @@ -1,4 +1,5 @@ import _ from 'underscore'; +import {deepEqual} from 'fast-equals'; import fastMerge from './fastMerge'; const isDefined = _.negate(_.isUndefined); @@ -198,8 +199,7 @@ class OnyxCache { * @returns {Boolean} */ hasValueChanged(key, value) { - this.addToAccessedKeys(key); - return !_.isEqual(this.storageMap[key], value); + return !deepEqual(this.storageMap[key], value); } } diff --git a/package-lock.json b/package-lock.json index de7851a03..2bbd4897e 100644 --- a/package-lock.json +++ b/package-lock.json @@ -7177,6 +7177,11 @@ "integrity": "sha512-xJuoT5+L99XlZ8twedaRf6Ax2TgQVxvgZOYoPKqZufmJib0tL2tegPBOZb1pVNgIhlqDlA0eO0c3wBvQcmzx4w==", "dev": true }, + "fast-equals": { + "version": "4.0.3", + "resolved": "https://registry.npmjs.org/fast-equals/-/fast-equals-4.0.3.tgz", + "integrity": "sha512-G3BSX9cfKttjr+2o1O22tYMLq0DPluZnYtq1rXumE1SpL/F/SLIfHx08WYQoWSIpeMYf8sRbJ8++71+v6Pnxfg==" + }, "fast-json-stable-stringify": { "version": "2.1.0", "resolved": "https://registry.npmjs.org/fast-json-stable-stringify/-/fast-json-stable-stringify-2.1.0.tgz", diff --git a/package.json b/package.json index 1044f89d8..944d090b0 100644 --- a/package.json +++ b/package.json @@ -40,6 +40,7 @@ }, "dependencies": { "ascii-table": "0.0.9", + "fast-equals": "^4.0.3", "lodash": "^4.17.21", "underscore": "^1.13.1" }, diff --git a/tests/unit/metricsTest.js b/tests/unit/metricsTest.js index 0c17d518d..ac8aeb5d5 100644 --- a/tests/unit/metricsTest.js +++ b/tests/unit/metricsTest.js @@ -13,17 +13,17 @@ describe('decorateWithMetrics', () => { }); it('Should collect metrics for a single method, single call', () => { - // GIVEN an async function that resolves with an object + // Given an async function that resolves with an object const mockedResult = {mockedKey: 'mockedValue'}; let mockFn = jest.fn().mockResolvedValueOnce(mockedResult); - // WHEN it is decorated and executed + // When it is decorated and executed mockFn = decorateWithMetrics(mockFn, 'mockFn'); mockFn('mockedKey'); return waitForPromisesToResolve() .then(() => { - // THEN stats should contain expected data regarding the call: timings, args and result + // Then stats should contain expected data regarding the call: timings, args and result const metrics = getMetrics(); expect(metrics).toEqual(expect.objectContaining({ totalTime: expect.any(Number), @@ -40,19 +40,19 @@ describe('decorateWithMetrics', () => { }); it('Should use function.name when alias was not provided', () => { - // GIVEN a regular JS function + // Given a regular JS function function mockFunc() { return Promise.resolve(); } - // WHEN decorated without passing an "alias" parameter + // When decorated without passing an "alias" parameter // eslint-disable-next-line no-func-assign mockFunc = decorateWithMetrics(mockFunc); mockFunc(); waitForPromisesToResolve() .then(() => { - // THEN the alias should be inferred from the function name + // Then the alias should be inferred from the function name const stats = getMetrics(); expect(stats).toHaveLength(1); expect(stats).toEqual([ @@ -62,13 +62,13 @@ describe('decorateWithMetrics', () => { }); it('Should collect metrics for multiple calls', () => { - // GIVEN an async function that resolves with an object + // Given an async function that resolves with an object let mockFn = jest.fn() .mockResolvedValueOnce({mock: 'value'}) .mockResolvedValueOnce({mock: 'value'}) .mockResolvedValueOnce({mock: 'value'}); - // WHEN it is decorated and executed + // When it is decorated and executed mockFn = decorateWithMetrics(mockFn, 'mockFn'); mockFn('mockedKey'); mockFn('mockedKey3'); @@ -76,7 +76,7 @@ describe('decorateWithMetrics', () => { return waitForPromisesToResolve() .then(() => { - // THEN stats should have data regarding each call + // Then stats should have data regarding each call const calls = getMetrics().summaries.mockFn.calls; expect(calls).toHaveLength(3); expect(calls).toEqual([ @@ -94,19 +94,19 @@ describe('decorateWithMetrics', () => { }); it('Should work for methods that return Promise', () => { - // GIVEN an async function that resolves with no data + // Given an async function that resolves with no data let mockFn = jest.fn() .mockResolvedValueOnce() .mockResolvedValueOnce(); - // WHEN it is decorated and executed + // When it is decorated and executed mockFn = decorateWithMetrics(mockFn, 'mockFn'); mockFn('mockedKey', {ids: [1, 2, 3]}); mockFn('mockedKey', {ids: [4, 5, 6]}); return waitForPromisesToResolve() .then(() => { - // THEN stats should still contain data about the calls + // Then stats should still contain data about the calls const calls = getMetrics().summaries.mockFn.calls; expect(calls).toHaveLength(2); expect(calls).toEqual([ @@ -121,27 +121,27 @@ describe('decorateWithMetrics', () => { }); it('Should not affect the returned value from the original method', () => { - // GIVEN an async function that resolves with an object + // Given an async function that resolves with an object const mockedResult = {mockedKey: 'mockedValue'}; let mockFn = jest.fn().mockResolvedValueOnce(mockedResult); - // WHEN it is decorated and executed + // When it is decorated and executed mockFn = decorateWithMetrics(mockFn, 'mockFn'); return mockFn('mockedKey') .then((result) => { - // THEN the result should be the same with the result of the non decorated version + // Then the result should be the same with the result of the non decorated version expect(result).toEqual(mockedResult); }) .then(waitForPromisesToResolve); }); it('Should collect metrics for a multiple functions, single call', () => { - // GIVEN multiple async functions that resolves with objects + // Given multiple async functions that resolves with objects let mockGet = jest.fn().mockResolvedValueOnce({mock: 'value'}); let mockGetAllKeys = jest.fn().mockResolvedValueOnce(['my', 'mock', 'keys']); - // WHEN each is decorated and executed one time + // When each is decorated and executed one time mockGet = decorateWithMetrics(mockGet, 'mockGet'); mockGetAllKeys = decorateWithMetrics(mockGetAllKeys, 'mockGetAllKeys'); @@ -150,7 +150,7 @@ describe('decorateWithMetrics', () => { return waitForPromisesToResolve() .then(() => { - // THEN stats should contain data for each function and each call under the correct function alias + // Then stats should contain data for each function and each call under the correct function alias const stats = getMetrics().summaries; expect(_.keys(stats)).toHaveLength(2); @@ -165,7 +165,7 @@ describe('decorateWithMetrics', () => { }); it('Should collect metrics for a multiple functions, multiple call', () => { - // GIVEN multiple async functions that resolves with objects + // Given multiple async functions that resolves with objects let mockGetAllKeys = jest.fn() .mockResolvedValueOnce(['my', 'mock', 'keys']) .mockResolvedValueOnce(['my', 'mock', 'keys', 'and']) @@ -175,11 +175,11 @@ describe('decorateWithMetrics', () => { .mockResolvedValueOnce() .mockResolvedValueOnce(); - // WHEN they are decorated + // When they are decorated mockGetAllKeys = decorateWithMetrics(mockGetAllKeys, 'mockGetAllKeys'); mockSetItem = decorateWithMetrics(mockSetItem, 'mockSetItem'); - // WHEN each is executed multiple times + // When each is executed multiple times mockGetAllKeys(); mockSetItem('and', 'Mock value'); mockGetAllKeys(); @@ -188,7 +188,7 @@ describe('decorateWithMetrics', () => { return waitForPromisesToResolve() .then(() => { - // THEN stats should contain data for each function and each call under the correct function alias + // Then stats should contain data for each function and each call under the correct function alias const stats = getMetrics().summaries; expect(_.keys(stats)).toHaveLength(2); @@ -203,19 +203,19 @@ describe('decorateWithMetrics', () => { }); it('Attempting to decorate already decorated method should throw', () => { - // GIVEN a function that is decorated + // Given a function that is decorated let mockFn = jest.fn(); mockFn = decorateWithMetrics(mockFn, 'mockFn'); - // WHEN you try to decorate again the same function + // When you try to decorate again the same function expect(() => decorateWithMetrics(mockFn, 'mockFn')) - // THEN it should throw an exception + // Then it should throw an exception .toThrow('"mockFn" is already decorated'); }); it('Adding more data after clearing should work', () => { - // GIVEN an async function that is decorated + // Given an async function that is decorated let mockFn = jest.fn() .mockResolvedValueOnce() .mockResolvedValueOnce() @@ -223,42 +223,42 @@ describe('decorateWithMetrics', () => { mockFn = decorateWithMetrics(mockFn, 'mockFn'); - // GIVEN some call made with the decorated function + // Given some call made with the decorated function mockFn('mockedKey', {ids: [1, 2, 3]}); mockFn('mockedKey', {ids: [4, 5, 6]}); return waitForPromisesToResolve() .then(() => { - // WHEN metrics are reset + // When metrics are reset expect(getMetrics().summaries.mockFn.calls).toHaveLength(2); resetMetrics(); - // THEN no data regarding the calls that happened before should exist + // Then no data regarding the calls that happened before should exist expect(getMetrics().summaries.mockFn).not.toBeDefined(); - // WHEN more calls are made + // When more calls are made mockFn('mockedKey', {ids: [1, 2, 3]}); return waitForPromisesToResolve(); }) .then(() => { - // THEN only these calls should appear in stats + // Then only these calls should appear in stats expect(getMetrics().summaries.mockFn.calls).toHaveLength(1); }); }); it('Should work with custom alias', () => { - // GIVEN an async function that resolves with an object + // Given an async function that resolves with an object const mockedResult = {mockedKey: 'mockedValue'}; let mockFn = jest.fn().mockResolvedValueOnce(mockedResult); - // WHEN it is decorated with a custom alias as a 2nd parameter + // When it is decorated with a custom alias as a 2nd parameter mockFn = decorateWithMetrics(mockFn, 'mock:get'); mockFn('mockKey'); return waitForPromisesToResolve() .then(() => { - // THEN stats should contain data regarding the calls under that custom alias + // Then stats should contain data regarding the calls under that custom alias const stats = getMetrics().summaries; expect(stats).toEqual(expect.objectContaining({ 'mock:get': expect.any(Object), @@ -268,35 +268,35 @@ describe('decorateWithMetrics', () => { }); it('Should return 0 total time and stats when no stats are collected yet', () => { - // GIVEN no calls made + // Given no calls made - // WHEN getMetrics is called + // When getMetrics is called const result = getMetrics(); - // THEN stats should be empty and the total time 0 + // Then stats should be empty and the total time 0 expect(result.summaries).toEqual({}); expect(result.totalTime).toEqual(0); expect(result.lastCompleteCall).not.toBeDefined(); }); it('Should calculate total and average correctly', () => { - // GIVEN an async function that resolves with an object + // Given an async function that resolves with an object const mockedResult = {mockedKey: 'mockedValue'}; let mockFn = jest.fn().mockResolvedValue(mockedResult); - // GIVEN mocked performance than returns +250ms for each consecutive call + // Given mocked performance than returns +250ms for each consecutive call let i = 0; jest.spyOn(global.performance, 'now') .mockImplementation(() => 250 * i++); - // WHEN it is decorated + // When it is decorated mockFn = decorateWithMetrics(mockFn, 'mockFn'); - // WHEN the decorated function is executed + // When the decorated function is executed mockFn('mockedKey') .then(waitForPromisesToResolve) .then(() => { - // THEN metrics should contain correctly calculated data + // Then metrics should contain correctly calculated data const metrics = getMetrics(); expect(metrics).toEqual(expect.objectContaining({ @@ -305,11 +305,11 @@ describe('decorateWithMetrics', () => { })); }); - // WHEN the decorated function is executed again + // When the decorated function is executed again mockFn('mockedKey') .then(waitForPromisesToResolve) .then(() => { - // THEN metrics should contain correctly calculated data + // Then metrics should contain correctly calculated data const metrics = getMetrics(); expect(metrics).toEqual(expect.objectContaining({ @@ -318,11 +318,11 @@ describe('decorateWithMetrics', () => { })); }); - // WHEN the decorated function is executed again + // When the decorated function is executed again mockFn('mockedKey') .then(waitForPromisesToResolve) .then(() => { - // THEN metrics should contain correctly calculated data + // Then metrics should contain correctly calculated data const metrics = getMetrics(); expect(metrics).toEqual(expect.objectContaining({ diff --git a/tests/unit/onyxCacheTest.js b/tests/unit/onyxCacheTest.js index 393738f6f..1e142e561 100644 --- a/tests/unit/onyxCacheTest.js +++ b/tests/unit/onyxCacheTest.js @@ -21,7 +21,7 @@ describe('Onyx', () => { it('Should be empty initially', () => { // Given empty cache - // WHEN all keys are retrieved + // When all keys are retrieved const allKeys = cache.getAllKeys(); // Then the result should be empty @@ -56,7 +56,7 @@ describe('Onyx', () => { cache.set('mockKey2', 'mockValue'); cache.set('mockKey3', 'mockValue'); - // WHEN an existing keys is later updated + // When an existing keys is later updated cache.set('mockKey2', 'new mock value'); // Then getAllKeys should not include a duplicate value @@ -69,7 +69,7 @@ describe('Onyx', () => { it('Should return undefined when there is no stored value', () => { // Given empty cache - // WHEN a value is retrieved + // When a value is retrieved const result = cache.getValue('mockKey'); // Then it should be undefined @@ -81,7 +81,7 @@ describe('Onyx', () => { cache.set('mockKey', {items: ['mockValue', 'mockValue2']}); cache.set('mockKey2', 'mockValue3'); - // WHEN a value is retrieved + // 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'); @@ -92,7 +92,7 @@ describe('Onyx', () => { it('Should return false when there is no stored value', () => { // Given empty cache - // WHEN a value does not exist in cache + // When a value does not exist in cache // Then it should return false expect(cache.hasCacheForKey('mockKey')).toBe(false); }); @@ -102,7 +102,7 @@ describe('Onyx', () => { cache.set('mockKey', {items: ['mockValue', 'mockValue2']}); cache.set('mockKey2', 'mockValue3'); - // WHEN a value exists in cache + // When a value exists in cache // Then it should return true expect(cache.hasCacheForKey('mockKey')).toBe(true); expect(cache.hasCacheForKey('mockKey2')).toBe(true); @@ -113,7 +113,7 @@ describe('Onyx', () => { it('Should store the key so that it is returned by `getAllKeys`', () => { // Given empty cache - // WHEN set is called with key and value + // When set is called with key and value cache.addKey('mockKey'); // Then there should be no cached value @@ -126,7 +126,7 @@ describe('Onyx', () => { it('Should not make duplicate keys', () => { // Given empty cache - // WHEN the same item is added multiple times + // When the same item is added multiple times cache.addKey('mockKey'); cache.addKey('mockKey'); cache.addKey('mockKey2'); @@ -142,7 +142,7 @@ describe('Onyx', () => { it('Should add data to cache when both key and value are provided', () => { // Given empty cache - // WHEN set is called with key and value + // When set is called with key and value cache.set('mockKey', {value: 'mockValue'}); // Then data should be cached @@ -153,7 +153,7 @@ describe('Onyx', () => { it('Should store the key so that it is returned by `getAllKeys`', () => { // Given empty cache - // WHEN set is called with key and value + // When set is called with key and value cache.set('mockKey', {value: 'mockValue'}); // Then but a key should be available @@ -165,7 +165,7 @@ describe('Onyx', () => { cache.set('mockKey', {value: 'mockValue'}); cache.set('mockKey2', {other: 'otherMockValue'}); - // WHEN set is called for an existing key + // When set is called for an existing key cache.set('mockKey2', {value: []}); // Then the value should be overwritten @@ -180,7 +180,7 @@ describe('Onyx', () => { cache.set('mockKey2', 'mockValue'); cache.set('mockKey3', 'mockValue'); - // WHEN an key is removed + // When an key is removed cache.drop('mockKey2'); // Then getAllKeys should still include the key @@ -193,7 +193,7 @@ describe('Onyx', () => { cache.set('mockKey', {items: ['mockValue', 'mockValue2']}); cache.set('mockKey2', 'mockValue3'); - // WHEN a key is removed + // When a key is removed cache.drop('mockKey'); // Then a value should not be available in cache @@ -206,7 +206,7 @@ describe('Onyx', () => { it('Should create the value in cache when it does not exist', () => { // Given empty cache - // WHEN merge is called with new key value pairs + // When merge is called with new key value pairs cache.merge({ mockKey: {value: 'mockValue'}, mockKey2: {value: 'mockValue2'}, @@ -222,7 +222,7 @@ describe('Onyx', () => { cache.set('mockKey', {value: 'mockValue'}); cache.set('mockKey2', {other: 'otherMockValue', mock: 'mock', items: [3, 4, 5]}); - // WHEN merge is called with existing key value pairs + // When merge is called with existing key value pairs cache.merge({ mockKey: {mockItems: []}, mockKey2: {items: [1, 2], other: 'overwrittenMockValue'}, @@ -245,7 +245,7 @@ describe('Onyx', () => { // Given cache with existing object data cache.set('mockKey', {value: 'mockValue', otherValue: 'overwrite me'}); - // WHEN merge is called for a key with object value + // When merge is called for a key with object value cache.merge({ mockKey: {mockItems: [], otherValue: 'overwritten'}, }); @@ -262,7 +262,7 @@ describe('Onyx', () => { // Given cache with existing array data cache.set('mockKey', [{ID: 1}, {ID: 2}, {ID: 3}]); - // WHEN merge is called with an array + // When merge is called with an array cache.merge({ mockKey: [{ID: 3}, {added: 'field'}, {}, {ID: 1000}], }); @@ -277,7 +277,7 @@ describe('Onyx', () => { // Given cache with existing array data cache.set('mockKey', {ID: [1]}); - // WHEN merge is called with an array + // When merge is called with an array cache.merge({ mockKey: {ID: [2]}, }); @@ -290,31 +290,31 @@ describe('Onyx', () => { // Given cache with existing data cache.set('mockKey', {}); - // WHEN merge is called with bool + // When merge is called with bool cache.merge({mockKey: false}); // Then the object should be overwritten with a bool value expect(cache.getValue('mockKey')).toEqual(false); - // WHEN merge is called with number + // When merge is called with number cache.merge({mockKey: 0}); // Then the value should be overwritten expect(cache.getValue('mockKey')).toEqual(0); - // WHEN merge is called with string + // When merge is called with string cache.merge({mockKey: '123'}); // Then the value should be overwritten expect(cache.getValue('mockKey')).toEqual('123'); - // WHEN merge is called with string again + // When merge is called with string again cache.merge({mockKey: '123'}); // Then strings should not have been concatenated expect(cache.getValue('mockKey')).toEqual('123'); - // WHEN merge is called with an object + // When merge is called with an object cache.merge({mockKey: {value: 'myMockObject'}}); // Then the old primitive value should be overwritten with the object @@ -325,7 +325,7 @@ describe('Onyx', () => { // Given cache with existing data cache.set('mockKey', {ID: 5}); - // WHEN merge is called key value pair and the value is undefined + // When merge is called key value pair and the value is undefined cache.merge({mockKey: undefined}); // Then the key should still be in cache and the value unchanged @@ -338,7 +338,7 @@ describe('Onyx', () => { cache.set('mockKey', {value: 'mockValue'}); cache.set('mockKey2', {other: 'otherMockValue', mock: 'mock', items: [3, 4, 5]}); - // WHEN merge is called with existing key value pairs + // When merge is called with existing key value pairs cache.merge({ mockKey: {mockItems: []}, mockKey3: {ID: 3}, @@ -353,21 +353,21 @@ describe('Onyx', () => { describe('hasPendingTask', () => { it('Should return false when there is no started task', () => { // Given empty cache with no started tasks - // WHEN a task has not been started + // When a task has not been started // Then it should return false expect(cache.hasPendingTask('mockTask')).toBe(false); }); it('Should return true when a task is running', () => { // Given empty cache with no started tasks - // WHEN a unique task is started + // When a unique task is started const promise = Promise.resolve(); cache.captureTask('mockTask', promise); // Then `hasPendingTask` should return true expect(cache.hasPendingTask('mockTask')).toBe(true); - // WHEN the promise is completed + // When the promise is completed return waitForPromisesToResolve() .then(() => { // Then `hasPendingTask` should return false @@ -380,7 +380,7 @@ describe('Onyx', () => { it('Should return undefined when there is no stored value', () => { // Given empty cache with no started tasks - // WHEN a task is retrieved + // When a task is retrieved const task = cache.getTaskPromise('mockTask'); // Then it should be undefined @@ -389,11 +389,11 @@ describe('Onyx', () => { it('Should return captured task when it exists', () => { // Given empty cache with no started tasks - // WHEN a unique task is started + // When a unique task is started const promise = Promise.resolve({mockResult: true}); cache.captureTask('mockTask', promise); - // WHEN a task is retrieved + // When a task is retrieved const taskPromise = cache.getTaskPromise('mockTask'); // Then it should resolve with the same result as the captured task @@ -460,7 +460,7 @@ describe('Onyx', () => { AsyncStorageMock.getAllKeys.mockResolvedValue([ONYX_KEYS.TEST_KEY]); return initOnyx() .then(() => { - // WHEN multiple components are rendered + // When multiple components are rendered render( <> @@ -490,7 +490,7 @@ describe('Onyx', () => { AsyncStorageMock.getItem.mockResolvedValue('"mockValue"'); AsyncStorageMock.getAllKeys.mockResolvedValue([ONYX_KEYS.TEST_KEY]); - // WHEN multiple components are rendered + // When multiple components are rendered render( <> @@ -531,7 +531,7 @@ describe('Onyx', () => { }) .then(waitForPromisesToResolve) .then(() => { - // WHEN a new connection for a safe eviction key happens + // When a new connection for a safe eviction key happens Onyx.connect({key: `${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}9`, callback: jest.fn()}); }) .then(() => { @@ -563,12 +563,12 @@ describe('Onyx', () => { return initOnyx() .then(() => { - // WHEN a component is rendered + // When a component is rendered render(); }) .then(waitForPromisesToResolve) .then(() => { - // WHEN the key was removed from cache + // When the key was removed from cache cache.drop(ONYX_KEYS.TEST_KEY); }) @@ -601,7 +601,7 @@ describe('Onyx', () => { AsyncStorageMock.getAllKeys.mockResolvedValue([ONYX_KEYS.TEST_KEY, ONYX_KEYS.OTHER_TEST]); return initOnyx() .then(() => { - // WHEN the components are rendered multiple times + // When the components are rendered multiple times render(); render(); render(); @@ -629,7 +629,7 @@ describe('Onyx', () => { // Given Onyx with LRU size of 3 return initOnyx({maxCachedKeysCount: 3}) .then(() => { - // WHEN 4 connections for different keys happen + // When 4 connections for different keys happen Onyx.connect({key: 'key1', callback: jest.fn()}); Onyx.connect({key: 'key2', callback: jest.fn()}); Onyx.connect({key: 'key3', callback: jest.fn()}); @@ -643,7 +643,7 @@ describe('Onyx', () => { expect(cache.hasCacheForKey('key3')).toBe(true); expect(cache.hasCacheForKey('key4')).toBe(true); - // WHEN A connection for safe eviction key happens + // When A connection for safe eviction key happens Onyx.connect({key: ONYX_KEYS.COLLECTION.MOCK_COLLECTION, callback: jest.fn()}); }) .then(waitForPromisesToResolve) diff --git a/tests/unit/onyxClearNativeStorageTest.js b/tests/unit/onyxClearNativeStorageTest.js index 6412aeeaa..084658b47 100644 --- a/tests/unit/onyxClearNativeStorageTest.js +++ b/tests/unit/onyxClearNativeStorageTest.js @@ -45,15 +45,15 @@ describe('Set data while storage is clearing', () => { it('should persist the value of Onyx.merge when called between the cache and storage clearing', () => { expect.assertions(3); - // GIVEN that Onyx is completely clear - // WHEN Onyx.clear() is called + // Given that Onyx is completely clear + // When Onyx.clear() is called Onyx.clear(); - // WHEN merge is called between the cache and storage clearing, on a key with a default key state + // When merge is called between the cache and storage clearing, on a key with a default key state Onyx.merge(ONYX_KEYS.DEFAULT_KEY, MERGED_VALUE); return waitForPromisesToResolve() .then(() => { - // THEN the value in Onyx, the cache, and the storage is the merged value + // Then the value in Onyx, the cache, and the storage is the merged value expect(onyxValue).toBe(MERGED_VALUE); const cachedValue = cache.getValue(ONYX_KEYS.DEFAULT_KEY); expect(cachedValue).toBe(MERGED_VALUE); @@ -67,18 +67,18 @@ describe('Set data while storage is clearing', () => { it('should replace the value of Onyx.set with the default key state in the cache', () => { expect.assertions(3); - // GIVEN that Onyx is completely clear - // WHEN set then clear is called on a key with a default key state + // Given that Onyx is completely clear + // When set then clear is called on a key with a default key state Onyx.set(ONYX_KEYS.DEFAULT_KEY, SET_VALUE); Onyx.clear(); return waitForPromisesToResolve() .then(() => { - // THEN the value in Onyx and the cache is the default key state + // Then the value in Onyx and the cache is the default key state expect(onyxValue).toBe(DEFAULT_VALUE); const cachedValue = cache.getValue(ONYX_KEYS.DEFAULT_KEY); expect(cachedValue).toBe(DEFAULT_VALUE); - // THEN the value in Storage is null + // Then the value in Storage is null // The default key state is never stored during Onyx.clear const storedValue = Storage.getItem(ONYX_KEYS.DEFAULT_KEY); return expect(storedValue).resolves.toBeNull(); @@ -88,19 +88,19 @@ describe('Set data while storage is clearing', () => { it('should replace the value of Onyx.merge with the default key state in the cache', () => { expect.assertions(3); - // GIVEN that Onyx is completely clear - // WHEN merge then clear is called on a key with a default key state + // Given that Onyx is completely clear + // When merge then clear is called on a key with a default key state Onyx.merge(ONYX_KEYS.DEFAULT_KEY, MERGED_VALUE); Onyx.clear(); return waitForPromisesToResolve() .then(() => { - // THEN the value in Onyx and the cache is the default key state + // Then the value in Onyx and the cache is the default key state expect(onyxValue).toBe(DEFAULT_VALUE); const cachedValue = cache.getValue(ONYX_KEYS.DEFAULT_KEY); expect(cachedValue).toBe(DEFAULT_VALUE); const storedValue = Storage.getItem(ONYX_KEYS.DEFAULT_KEY); - // THEN the value in Storage is null + // Then the value in Storage is null // The default key state is never stored during Onyx.clear return expect(storedValue).resolves.toBeNull(); }); diff --git a/tests/unit/onyxClearWebStorageTest.js b/tests/unit/onyxClearWebStorageTest.js index a4c7511c4..3e8bb93b0 100644 --- a/tests/unit/onyxClearWebStorageTest.js +++ b/tests/unit/onyxClearWebStorageTest.js @@ -49,15 +49,15 @@ describe('Set data while storage is clearing', () => { it('should persist the value of Onyx.merge when called between the cache and storage clearing', () => { expect.assertions(3); - // GIVEN that Onyx is completely clear - // WHEN Onyx.clear() is called + // Given that Onyx is completely clear + // When Onyx.clear() is called Onyx.clear(); - // WHEN merge is called between the cache and storage clearing, on a key with a default key state + // When merge is called between the cache and storage clearing, on a key with a default key state Onyx.merge(ONYX_KEYS.DEFAULT_KEY, MERGED_VALUE); return waitForPromisesToResolve() .then(() => { - // THEN the value in Onyx, the cache, and the storage is the merged value + // Then the value in Onyx, the cache, and the storage is the merged value expect(onyxValue).toBe(MERGED_VALUE); const cachedValue = cache.getValue(ONYX_KEYS.DEFAULT_KEY); expect(cachedValue).toBe(MERGED_VALUE); @@ -69,18 +69,18 @@ describe('Set data while storage is clearing', () => { it('should replace the value of Onyx.set with the default key state in the cache', () => { expect.assertions(3); - // GIVEN that Onyx is completely clear - // WHEN set then clear is called on a key with a default key state + // Given that Onyx is completely clear + // When set then clear is called on a key with a default key state Onyx.set(ONYX_KEYS.DEFAULT_KEY, SET_VALUE); Onyx.clear(); return waitForPromisesToResolve() .then(() => { - // THEN the value in Onyx and the cache is the default key state + // Then the value in Onyx and the cache is the default key state expect(onyxValue).toBe(DEFAULT_VALUE); const cachedValue = cache.getValue(ONYX_KEYS.DEFAULT_KEY); expect(cachedValue).toBe(DEFAULT_VALUE); - // THEN the value in Storage is null + // Then the value in Storage is null // The default key state is never stored during Onyx.clear const storedValue = Storage.getItem(ONYX_KEYS.DEFAULT_KEY); return expect(storedValue).resolves.toBeUndefined(); @@ -90,19 +90,19 @@ describe('Set data while storage is clearing', () => { it('should replace the value of Onyx.merge with the default key state in the cache', () => { expect.assertions(3); - // GIVEN that Onyx is completely clear - // WHEN merge then clear is called on a key with a default key state + // Given that Onyx is completely clear + // When merge then clear is called on a key with a default key state Onyx.merge(ONYX_KEYS.DEFAULT_KEY, MERGED_VALUE); Onyx.clear(); return waitForPromisesToResolve() .then(() => { - // THEN the value in Onyx and the cache is the default key state + // Then the value in Onyx and the cache is the default key state expect(onyxValue).toBe(DEFAULT_VALUE); const cachedValue = cache.getValue(ONYX_KEYS.DEFAULT_KEY); expect(cachedValue).toBe(DEFAULT_VALUE); const storedValue = Storage.getItem(ONYX_KEYS.DEFAULT_KEY); - // THEN the value in Storage is null + // Then the value in Storage is null // The default key state is never stored during Onyx.clear return expect(storedValue).resolves.toBeUndefined(); }); diff --git a/tests/unit/onyxMetricsDecorationTest.js b/tests/unit/onyxMetricsDecorationTest.js index 54841f129..151835c08 100644 --- a/tests/unit/onyxMetricsDecorationTest.js +++ b/tests/unit/onyxMetricsDecorationTest.js @@ -16,59 +16,59 @@ describe('Onyx', () => { }); it('Should expose metrics methods when `captureMetrics` is true', () => { - // WHEN Onyx is initialized with `captureMetrics: true` + // When Onyx is initialized with `captureMetrics: true` Onyx.init({ keys: ONYX_KEYS, registerStorageEventListener: jest.fn(), captureMetrics: true, }); - // THEN Onyx should have statistic related methods + // Then Onyx should have statistic related methods expect(Onyx.getMetrics).toEqual(expect.any(Function)); expect(Onyx.printMetrics).toEqual(expect.any(Function)); expect(Onyx.resetMetrics).toEqual(expect.any(Function)); }); it('Should not expose metrics methods when `captureMetrics` is false or not set', () => { - // WHEN Onyx is initialized without setting `captureMetrics` + // When Onyx is initialized without setting `captureMetrics` Onyx.init({ keys: ONYX_KEYS, registerStorageEventListener: jest.fn(), }); - // THEN Onyx should not have statistic related methods + // Then Onyx should not have statistic related methods expect(Onyx.getMetrics).not.toBeDefined(); expect(Onyx.printMetrics).not.toBeDefined(); expect(Onyx.resetMetrics).not.toBeDefined(); - // WHEN Onyx is initialized with `captureMetrics: false` + // When Onyx is initialized with `captureMetrics: false` Onyx.init({ keys: ONYX_KEYS, registerStorageEventListener: jest.fn(), captureMetrics: false, }); - // THEN Onyx should not have statistic related methods + // Then Onyx should not have statistic related methods expect(Onyx.getMetrics).not.toBeDefined(); expect(Onyx.printMetrics).not.toBeDefined(); expect(Onyx.resetMetrics).not.toBeDefined(); }); it('Should decorate exposed methods', () => { - // GIVEN Onyx is initialized with `captureMetrics: true` + // Given Onyx is initialized with `captureMetrics: true` Onyx.init({ keys: ONYX_KEYS, registerStorageEventListener: jest.fn(), captureMetrics: true, }); - // WHEN calling decorated methods through Onyx[methodName] + // When calling decorated methods through Onyx[methodName] const methods = ['set', 'multiSet', 'clear', 'merge', 'mergeCollection']; methods.forEach(name => Onyx[name]('mockKey', {mockKey: {mockValue: 'mockValue'}})); return waitForPromisesToResolve() .then(() => { - // THEN metrics should have captured data for each method + // Then metrics should have captured data for each method const summaries = Onyx.getMetrics().summaries; methods.forEach((name) => { diff --git a/tests/unit/onyxTest.js b/tests/unit/onyxTest.js index d1ce3bab7..e55b9d952 100644 --- a/tests/unit/onyxTest.js +++ b/tests/unit/onyxTest.js @@ -358,7 +358,7 @@ describe('Onyx', () => { expect(testKeyValue).toBe(true); expect(otherTestKeyValue).toEqual({test1: 'test1'}); - // WHEN we pass a data object to Onyx.update + // When we pass a data object to Onyx.update Onyx.update([ { onyxMethod: 'set', @@ -406,7 +406,7 @@ describe('Onyx', () => { expect(mockCallback).toHaveBeenNthCalledWith(1, {existingData: 'test'}, 'test_1'); expect(mockCallback).toHaveBeenNthCalledWith(2, {existingData: 'test'}, 'test_2'); - // WHEN we pass a mergeCollection data object to Onyx.update + // When we pass a mergeCollection data object to Onyx.update Onyx.update([ { onyxMethod: 'mergecollection', @@ -463,7 +463,7 @@ describe('Onyx', () => { ]; try { - // WHEN we pass it to Onyx.update + // When we pass it to Onyx.update Onyx.update(data); } catch (error) { // Then we should expect the error message below @@ -474,7 +474,7 @@ describe('Onyx', () => { // Given the invalid data object with key=true data[1] = {onyxMethod: 'merge', key: true, value: {test2: 'test2'}}; - // WHEN we pass it to Onyx.update + // When we pass it to Onyx.update Onyx.update(data); } catch (error) { // Then we should expect the error message below @@ -504,7 +504,7 @@ describe('Onyx', () => { Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_CONNECT_COLLECTION, initialCollectionData); return waitForPromisesToResolve() .then(() => { - // WHEN we connect to that collection with waitForCollectionCallback = true + // When we connect to that collection with waitForCollectionCallback = true connectionID = Onyx.connect({ key: ONYX_KEYS.COLLECTION.TEST_CONNECT_COLLECTION, waitForCollectionCallback: true, @@ -534,7 +534,7 @@ describe('Onyx', () => { }); return waitForPromisesToResolve() - // WHEN mergeCollection is called with an updated collection + // When mergeCollection is called with an updated collection .then(() => Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_POLICY, collectionUpdate)) .then(() => { // Then we expect the callback to have called twice, once for the initial connect call + once for the collection update @@ -562,7 +562,7 @@ describe('Onyx', () => { }); return waitForPromisesToResolve() - // WHEN mergeCollection is called with an updated collection + // When mergeCollection is called with an updated collection .then(() => { Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_POLICY, collectionUpdate); return waitForPromisesToResolve(); @@ -593,7 +593,7 @@ describe('Onyx', () => { }); return waitForPromisesToResolve() - // WHEN mergeCollection is called with an updated collection + // When mergeCollection is called with an updated collection .then(() => Onyx.merge(`${ONYX_KEYS.COLLECTION.TEST_POLICY}${1}`, collectionUpdate.test_policy_1)) .then(() => { // Then we expect the callback to have called twice, once for the initial connect call + once for the collection update @@ -645,7 +645,7 @@ describe('Onyx', () => { expect(mockCallback).toHaveBeenCalledTimes(2); }) - // WHEN merge is called again with an object of equivalent value but not the same reference + // When merge is called again with an object of equivalent value but not the same reference .then(() => Onyx.merge(`${ONYX_KEYS.COLLECTION.TEST_POLICY}${1}`, _.clone(collectionUpdate.test_policy_1))) .then(() => { // Then we should not expect another invocation of the callback @@ -685,7 +685,7 @@ describe('Onyx', () => { expect(mockCallback).toHaveBeenCalledTimes(2); }) - // WHEN merge is called again with an object of equivalent value but not the same reference + // When merge is called again with an object of equivalent value but not the same reference .then(() => Onyx.merge(`${ONYX_KEYS.COLLECTION.TEST_POLICY}${1}`, _.clone(collectionUpdate.test_policy_1))) .then(() => { // Then we should expect another invocation of the callback because initWithStoredValues = false diff --git a/tests/unit/withOnyxTest.js b/tests/unit/withOnyxTest.js index bcc5b4b0b..7fa437d44 100644 --- a/tests/unit/withOnyxTest.js +++ b/tests/unit/withOnyxTest.js @@ -242,7 +242,7 @@ describe('withOnyx', () => { const onRender2 = jest.fn(); const onRender3 = jest.fn(); - // GIVEN there is a collection with three simple items in it + // Given there is a collection with three simple items in it Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, { test_1: {ID: 1}, test_2: {ID: 2}, @@ -251,7 +251,7 @@ describe('withOnyx', () => { return waitForPromisesToResolve() .then(() => { - // WHEN three components subscribe to each of the items in that collection + // When three components subscribe to each of the items in that collection const TestComponentWithOnyx1 = withOnyx({ testObject: { key: `${ONYX_KEYS.COLLECTION.TEST_KEY}1`, @@ -276,14 +276,14 @@ describe('withOnyx', () => { return waitForPromisesToResolve(); }) .then(() => { - // WHEN a single item in the collection is updated with mergeCollect() + // When a single item in the collection is updated with mergeCollect() Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, { test_1: {ID: 1, newProperty: 'yay'}, }); return waitForPromisesToResolve(); }) .then(() => { - // THEN the component subscribed to the modified item should have the new version of the item + // Then the component subscribed to the modified item should have the new version of the item // and all other components should be unchanged. // Note: each component is rendered twice. Once when it is initially rendered, and then again // when the collection is updated. That's why there are two checks here for each component. @@ -302,14 +302,14 @@ describe('withOnyx', () => { it('mergeCollection should merge previous props correctly to the new state', () => { const onRender = jest.fn(); - // GIVEN there is a collection with a simple item in it that has a `number` property set to 1 + // Given there is a collection with a simple item in it that has a `number` property set to 1 Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, { test_1: {ID: 1, number: 1}, }); return waitForPromisesToResolve() .then(() => { - // WHEN a component subscribes to the one item in that collection + // When a component subscribes to the one item in that collection const TestComponentWithOnyx = withOnyx({ testObject: { key: `${ONYX_KEYS.COLLECTION.TEST_KEY}1`, @@ -320,14 +320,14 @@ describe('withOnyx', () => { return waitForPromisesToResolve(); }) .then(() => { - // WHEN the `number` property is updated using mergeCollection to be 2 + // When the `number` property is updated using mergeCollection to be 2 Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, { test_1: {number: 2}, }); return waitForPromisesToResolve(); }) .then(() => { - // THEN the component subscribed to the modified item should be rendered twice. + // Then the component subscribed to the modified item should be rendered twice. // The first time it will render with number === 1 // The second time it will render with number === 2 expect(onRender).toHaveBeenCalledTimes(2); @@ -360,7 +360,7 @@ describe('withOnyx', () => { return waitForPromisesToResolve(); }) .then(() => { - // THEN the component subscribed to the modified item should only render once + // Then the component subscribed to the modified item should only render once expect(onRender).toHaveBeenCalledTimes(1); expect(onRender.mock.calls[0][0].simple).toBe('string'); @@ -369,7 +369,7 @@ describe('withOnyx', () => { return waitForPromisesToResolve(); }) .then(() => { - // THEN the component subscribed to the modified item should only render once + // Then the component subscribed to the modified item should only render once expect(onRender).toHaveBeenCalledTimes(2); expect(onRender.mock.calls[1][0].simple).toBe('long_string'); });