Add initialValue, delayUpdates to allow components to eagerly render. #329
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -131,6 +131,7 @@ Subscribes a react component's state directly to a store key | |
| | [mapping.initWithStoredValues] | <code>Boolean</code> | If set to false, then no data will be prefilled into the component | | ||
| | [mapping.waitForCollectionCallback] | <code>Boolean</code> | If set to true, it will return the entire collection to the callback as a single object | | ||
| | [mapping.selector] | <code>String</code> \| <code>function</code> | THIS PARAM IS ONLY USED WITH withOnyx(). If included, this will be used to subscribe to a subset of an Onyx key's data. If the selector is a string, the selector is passed to lodashGet on the sourceData. If the selector is a function, the sourceData and withOnyx state are passed to the selector and should return the simplified data. Using this setting on `withOnyx` can have very positive performance benefits because the component will only re-render when the subset of data changes. Otherwise, any change of data on any property would normally cause the component to re-render (and that can be expensive from a performance standpoint). | | ||
| | [mapping.initialValue] | <code>Any</code> | THIS PARAM IS ONLY USED WITH withOnyx(). If included, this will be passed as the initial hydrated value for the mapping. It will allow the component to render eagerly while data is being fetched from the DB. Note that it will not cause the component to have the `loading` prop set to true. | | ||
|
There was a problem hiding this comment.
What exactly does "hydrated" mean? 😅 Is there a better term we can use for this that has the same meaning? I can only think of someone drinking a lot of water. There was a problem hiding this comment. Standard React lingo. But open to suggestions. There was a problem hiding this comment. Heh, yeah, but what does it mean to you? Like, let's say you were explaining this to a 7-year-old. There was a problem hiding this comment. I was talking with another co-worker about this and ChatGPT for the win! 
So, I think it would be helpful to change this to say:
|
||
|
|
||
| **Example** | ||
| ```js | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,77 @@ | ||
| import * as _ from 'underscore'; | ||
|
There was a problem hiding this comment. Now that there is a There was a problem hiding this comment. Yeah, makes sense. |
||
|
|
||
| function areObjectsEmpty(a, b) { | ||
|
There was a problem hiding this comment. I think this implementation is too specific in that it requires two objects. I think there are already existing methods to check if a single object is empty, and then it's just as easy as doing There was a problem hiding this comment.
There was a problem hiding this comment. There are some arrays, but they are less frequently used. This one comes to mind: https://github.com/Expensify/App/blob/48437acb34dbc38b9b7a60dae4c272727b8ce3ab/src/libs/actions/ActiveClients.js#L9 |
||
| return ( | ||
| typeof a === 'object' | ||
| && typeof b === 'object' | ||
| && _.isEmpty(a) | ||
| && _.isEmpty(b) | ||
| ); | ||
| } | ||
|
|
||
| // Mostly copied from https://medium.com/@lubaka.a/how-to-remove-lodash-performance-improvement-b306669ad0e1 | ||
|
|
||
| /** | ||
| * @param {mixed} val | ||
| * @returns {boolean} | ||
| */ | ||
| function isMergeableObject(val) { | ||
| const nonNullObject = val != null ? typeof val === 'object' : false; | ||
| return (nonNullObject | ||
| && Object.prototype.toString.call(val) !== '[object RegExp]' | ||
| && Object.prototype.toString.call(val) !== '[object Date]'); | ||
| } | ||
|
|
||
| /** | ||
| * @param {Object} target | ||
| * @param {Object} source | ||
| * @returns {Object} | ||
| */ | ||
| function mergeObject(target, source) { | ||
| const destination = {}; | ||
| if (isMergeableObject(target)) { | ||
| // lodash adds a small overhead so we don't use it here | ||
| // eslint-disable-next-line rulesdir/prefer-underscore-method | ||
| const targetKeys = Object.keys(target); | ||
| for (let i = 0; i < targetKeys.length; ++i) { | ||
| const key = targetKeys[i]; | ||
| destination[key] = target[key]; | ||
| } | ||
| } | ||
|
|
||
| // lodash adds a small overhead so we don't use it here | ||
| // eslint-disable-next-line rulesdir/prefer-underscore-method | ||
| const sourceKeys = Object.keys(source); | ||
| for (let i = 0; i < sourceKeys.length; ++i) { | ||
| const key = sourceKeys[i]; | ||
| if (source[key] === undefined) { | ||
| // eslint-disable-next-line no-continue | ||
| continue; | ||
| } | ||
| if (!isMergeableObject(source[key]) || !target[key]) { | ||
| destination[key] = source[key]; | ||
| } else { | ||
| // eslint-disable-next-line no-use-before-define | ||
| destination[key] = fastMerge(target[key], source[key]); | ||
| } | ||
| } | ||
|
|
||
| return destination; | ||
| } | ||
|
|
||
| /** | ||
| * @param {Object|Array} target | ||
| * @param {Object|Array} source | ||
| * @returns {Object|Array} | ||
| */ | ||
| function fastMerge(target, source) { | ||
| // lodash adds a small overhead so we don't use it here | ||
| // eslint-disable-next-line rulesdir/prefer-underscore-method | ||
| const array = Array.isArray(source); | ||
| if (array) { | ||
| return source; | ||
| } | ||
| return mergeObject(target, source); | ||
| } | ||
|
|
||
| export default {areObjectsEmpty, fastMerge}; | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it expected that only
initialValueis added to this API doc? It seems likeshouldDelayUpdatesandmarkReadyForHydrationshould show up somewhere here. Maybe try re-running the generator?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, this document is generated? I modified manually
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I re-ran the generator. However
shouldDelayUpdatesandmarkReadyForHidrationare part of the HOC and not of Onyx, so not sure if I should put them here. That's why I also updated the READMEThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, OK... I guess that's fine if the generator didn't pick them up. I didn't think about it only being a part of the HOC. I think the stuff you added in the README is good enough for the HOC.