From 9db16074c3bbcb26ed91c42a8d49f0814fb69671 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Wed, 10 Aug 2022 14:10:00 -1000 Subject: [PATCH 1/4] Fix waitForCollectionCallback --- README.md | 64 +++++++++++++++++++++++++++++++++++++++++++++++++++++ lib/Onyx.js | 15 +++++++++++++ 2 files changed, 79 insertions(+) diff --git a/README.md b/README.md index 2d1319f89..2edbece2f 100644 --- a/README.md +++ b/README.md @@ -137,6 +137,70 @@ export default withOnyx({ It is preferable to use the HOC over `Onyx.connect()` in React code as `withOnyx()` will delay the rendering of the wrapped component until all keys have been accessed and made available. +## Collections + +Collections allow keys with similar value types to be subscribed together by subscribing to the collection key. To define one, it must be included in the `ONYXKEYS.COLLECTION` object and it must be suffixed with an underscore. Member keys should use a unique identifier or index after the collection key prefix (e.g. `chat_42`). + +```javascript +const ONYXKEYS = { + COLLECTION: { + CHAT: 'chat_', + }, +}; +``` + +### Setting Collection Values + +To save a new collection key we can either do: + +```js +Onyx.merge(`${ONYXKEYS.COLLECTION.CHAT}${chat1.id}`, chat1); +``` + +or we can set many at once with `mergeCollection()`: + +```js +Onyx.mergeCollection(ONYXKEYS.COLLECTION.CHAT, { + [`${ONYXKEYS.COLLECTION.CHAT}${chat1.id}`]: chat1, + [`${ONYXKEYS.COLLECTION.CHAT}${chat2.id}`]: chat2, + [`${ONYXKEYS.COLLECTION.CHAT}${chat3.id}`]: chat3, +}); +``` + +### Subscribing to Collections + +There are several ways to subscribe to these keys: + +```javascript +withOnyx({ + allChats: {key: ONYXKEYS.COLLECTION.CHAT}, +})(MyComponent); +``` + +This will return the initial collection as an object of collection member key/values. Changes to the individual member keys will modify the entire object and new props will be passed with each individual key update. + +```js +Onyx.connect({key: ONYXKEYS.COLLECTION.CHAT}, callback: (memberValue, memberKey) => {...}}); +``` + +This will fire the callback `n` times depending on how many collection member keys have been stored. Changes to those keys after the initial callbacks fire will occur when each individual key is updated. + +```js +Onyx.connect({ + key: ONYXKEYS.COLLECTION.CHAT, + waitForCollectionCallback: true, + callback: (allChats) => {...}}, +}); +``` + +This final option forces `Onyx.connect()` to behave more like `withOnyx()` and only update the callback once with the entire collection initially and later with an updated version of the collection when individual keys update. + +### Performance Considerations When Using Collections + +Be cautious when using collections as things can get out of hand if you have a subscriber hooked up to a collection key that has large numbers of individual keys. If this is the case, it is critical to use `mergeCollection()` over `merge()`. + +Remember, `mergeCollection()` will notify a subscriber only *once* with the total collected values whereas each call to `Onyx.merge()` would re-render a connected component `n` times per key that you call it on. + ## Clean up To clear all data from `Onyx` we can use `Onyx.clear()`. diff --git a/lib/Onyx.js b/lib/Onyx.js index f5e105c0c..09f12a45a 100644 --- a/lib/Onyx.js +++ b/lib/Onyx.js @@ -253,8 +253,15 @@ function keysChanged(collectionKey, collection) { return; } + /** + * e.g. Onyx.connect({key: ONYXKEYS.COLLECTION.REPORT, callback: ...}); + */ const isSubscribedToCollectionKey = isKeyMatch(subscriber.key, collectionKey) && isCollectionKey(subscriber.key); + + /** + * e.g. Onyx.connect({key: `${ONYXKEYS.COLLECTION.REPORT}{reportID}`, callback: ...}); + */ const isSubscribedToCollectionMemberKey = subscriber.key.startsWith(collectionKey); if (isSubscribedToCollectionKey) { @@ -328,7 +335,15 @@ function keyChanged(key, data) { } if (_.isFunction(subscriber.callback)) { + if (subscriber.waitForCollectionCallback) { + const cachedCollection = getCachedCollection(subscriber.key); + cachedCollection[key] = data; + subscriber.callback(cachedCollection); + return; + } + subscriber.callback(data, key); + return; } if (!subscriber.withOnyxInstance) { From dfbe400f98c357c1cd1cfdcdd2b090715ebd4592 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Wed, 10 Aug 2022 14:12:31 -1000 Subject: [PATCH 2/4] Update docs --- API.md | 1 + lib/Onyx.js | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/API.md b/API.md index 1fd4e6213..0458fd9dc 100644 --- a/API.md +++ b/API.md @@ -76,6 +76,7 @@ Subscribes a react component's state directly to a store key | [mapping.withOnyxInstance] | Object | whose setState() method will be called with any changed data This is used by React components to connect to Onyx | | [mapping.callback] | function | a method that will be called with changed data This is used by any non-React code to connect to Onyx | | [mapping.initWithStoredValues] | Boolean | If set to false, then no data will be prefilled into the component | +| [mapping.waitForCollectionCallback] | Boolean | If set to true, it will return the entire collection to the callback as a single object | **Example** ```js diff --git a/lib/Onyx.js b/lib/Onyx.js index 09f12a45a..323152f0a 100644 --- a/lib/Onyx.js +++ b/lib/Onyx.js @@ -412,7 +412,7 @@ function sendDataToConnection(config, val, key) { * This is used by any non-React code to connect to Onyx * @param {Boolean} [mapping.initWithStoredValues] If set to false, then no data will be prefilled into the * component - * @param {Boolean} [mapping.waitForCollectionCallback] If set to true, it will trigger the callback once and return all data as a single object + * @param {Boolean} [mapping.waitForCollectionCallback] If set to true, it will return the entire collection to the callback as a single object * @returns {Number} an ID to use when calling disconnect */ function connect(mapping) { From 9cc99cd37e4a4b42c72cd661e50ce8365eaed41b Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Thu, 11 Aug 2022 08:40:01 -1000 Subject: [PATCH 3/4] improve language in docs and add example --- README.md | 36 +++++++++++++++++++++++------------- 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index 2edbece2f..d1e8700a6 100644 --- a/README.md +++ b/README.md @@ -139,12 +139,12 @@ It is preferable to use the HOC over `Onyx.connect()` in React code as `withOnyx ## Collections -Collections allow keys with similar value types to be subscribed together by subscribing to the collection key. To define one, it must be included in the `ONYXKEYS.COLLECTION` object and it must be suffixed with an underscore. Member keys should use a unique identifier or index after the collection key prefix (e.g. `chat_42`). +Collections allow keys with similar value types to be subscribed together by subscribing to the collection key. To define one, it must be included in the `ONYXKEYS.COLLECTION` object and it must be suffixed with an underscore. Member keys should use a unique identifier or index after the collection key prefix (e.g. `report_42`). ```javascript const ONYXKEYS = { COLLECTION: { - CHAT: 'chat_', + REPORT: 'report_', }, }; ``` @@ -154,16 +154,16 @@ const ONYXKEYS = { To save a new collection key we can either do: ```js -Onyx.merge(`${ONYXKEYS.COLLECTION.CHAT}${chat1.id}`, chat1); +Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${report1.reportID}`, report1); ``` or we can set many at once with `mergeCollection()`: ```js -Onyx.mergeCollection(ONYXKEYS.COLLECTION.CHAT, { - [`${ONYXKEYS.COLLECTION.CHAT}${chat1.id}`]: chat1, - [`${ONYXKEYS.COLLECTION.CHAT}${chat2.id}`]: chat2, - [`${ONYXKEYS.COLLECTION.CHAT}${chat3.id}`]: chat3, +Onyx.mergeCollection(ONYXKEYS.COLLECTION.REPORT, { + [`${ONYXKEYS.COLLECTION.REPORT}${report1.reportID}`]: report1, + [`${ONYXKEYS.COLLECTION.REPORT}${report2.reportID}`]: report2, + [`${ONYXKEYS.COLLECTION.REPORT}${report3.reportID}`]: report3, }); ``` @@ -173,23 +173,23 @@ There are several ways to subscribe to these keys: ```javascript withOnyx({ - allChats: {key: ONYXKEYS.COLLECTION.CHAT}, + allReports: {key: ONYXKEYS.COLLECTION.REPORT}, })(MyComponent); ``` This will return the initial collection as an object of collection member key/values. Changes to the individual member keys will modify the entire object and new props will be passed with each individual key update. ```js -Onyx.connect({key: ONYXKEYS.COLLECTION.CHAT}, callback: (memberValue, memberKey) => {...}}); +Onyx.connect({key: ONYXKEYS.COLLECTION.REPORT}, callback: (memberValue, memberKey) => {...}}); ``` -This will fire the callback `n` times depending on how many collection member keys have been stored. Changes to those keys after the initial callbacks fire will occur when each individual key is updated. +This will fire the callback once per member key depending on how many collection member keys are currently stored. Changes to those keys after the initial callbacks fire will occur when each individual key is updated. ```js Onyx.connect({ - key: ONYXKEYS.COLLECTION.CHAT, + key: ONYXKEYS.COLLECTION.REPORT, waitForCollectionCallback: true, - callback: (allChats) => {...}}, + callback: (allReports) => {...}}, }); ``` @@ -199,7 +199,17 @@ This final option forces `Onyx.connect()` to behave more like `withOnyx()` and o Be cautious when using collections as things can get out of hand if you have a subscriber hooked up to a collection key that has large numbers of individual keys. If this is the case, it is critical to use `mergeCollection()` over `merge()`. -Remember, `mergeCollection()` will notify a subscriber only *once* with the total collected values whereas each call to `Onyx.merge()` would re-render a connected component `n` times per key that you call it on. +Remember, `mergeCollection()` will notify a subscriber only *once* with the total collected values whereas each call to `merge()` would re-render a connected component *each time it is called*. Consider this example where `reports` is an array of reports that we want to index and save. + +```js +// Bad +_.each(reports, report => Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${report.reportID}`, report)); // -> Connected component will update on each iteration + +// Good +const values = {}; +_.each(reports, report => values[`${ONYXKEYS.COLLECTION.REPORT}${report.reportID}`] = report); +Onyx.mergeCollection(ONYXKEYS.COLLECTION.REPORT, values); // -> Connected component will update just once +``` ## Clean up From 7dcd8bebae08972d3c0ca98df47282f4616c11f9 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Fri, 12 Aug 2022 06:51:44 -1000 Subject: [PATCH 4/4] Update readme with suggested changes --- README.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index d1e8700a6..7c084bd32 100644 --- a/README.md +++ b/README.md @@ -157,7 +157,7 @@ To save a new collection key we can either do: Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${report1.reportID}`, report1); ``` -or we can set many at once with `mergeCollection()`: +or we can set many at once with `mergeCollection()` (see below for guidance on best practices): ```js Onyx.mergeCollection(ONYXKEYS.COLLECTION.REPORT, { @@ -177,7 +177,7 @@ withOnyx({ })(MyComponent); ``` -This will return the initial collection as an object of collection member key/values. Changes to the individual member keys will modify the entire object and new props will be passed with each individual key update. +This will add a prop to the component called `allReports` which is an object of collection member key/values. Changes to the individual member keys will modify the entire object and new props will be passed with each individual key update. The prop doesn't update on the initial rendering of the component until the entire collection has been read out of Onyx. ```js Onyx.connect({key: ONYXKEYS.COLLECTION.REPORT}, callback: (memberValue, memberKey) => {...}}); @@ -203,12 +203,12 @@ Remember, `mergeCollection()` will notify a subscriber only *once* with the tota ```js // Bad -_.each(reports, report => Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${report.reportID}`, report)); // -> Connected component will update on each iteration +_.each(reports, report => Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${report.reportID}`, report)); // -> A component using withOnyx() will have it's state updated with each iteration // Good const values = {}; _.each(reports, report => values[`${ONYXKEYS.COLLECTION.REPORT}${report.reportID}`] = report); -Onyx.mergeCollection(ONYXKEYS.COLLECTION.REPORT, values); // -> Connected component will update just once +Onyx.mergeCollection(ONYXKEYS.COLLECTION.REPORT, values); // -> A component using withOnyx() will only have it's state updated once ``` ## Clean up