| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
| * @param {object} [context] - If defined and the current value is a function, the value | ||
| * is called with `context` as first argument and the result becomes the new input. | ||
| * @param {number} [index] - If defined and the current value is an array, the value | ||
| * @param {object} [info] - object to return information about resolution in |
There was a problem hiding this comment.
I think I'd initialize info.cacheable to true in this method so that if you call resolve outside of _resolveDataElementOptions then info contains a usable value. Then in _resolveDataElementOptions you can check if (info.cacheable && !custom) instead of if (info.cacheable)
Sorry, something went wrong.
There was a problem hiding this comment.
It would have to be something like
if (info && info.cacheable === undefined) {
info.cacheable = true;
}because resolve is called in a loop. But I think its not needed. You'd need to provide the info object anyway, so not a biggie to provide initial value too.
Sorry, something went wrong.
There was a problem hiding this comment.
In other words, I like to keep the initialization (and any related if's) out of loop
Sorry, something went wrong.
There was a problem hiding this comment.
Maybe just document it then?
* @param {boolean} [info.cacheable] - Should be initialized to true. Will be updated by this method.
Sorry, something went wrong.
There was a problem hiding this comment.
This looks good! Definitely less code than my version!
I had opened my PR previously and you left a comment that I didn't understand. I was going to follow up with you to clarify, but now I understand :-) Thanks so much!
Sorry, something went wrong.
|
Was about to merge but I thought of this case:
Or am I missing something here? |
Sorry, something went wrong.
|
DatasetController does me._cachedDataOpts = null; in _update. Won't that invalidate the cache? |
Sorry, something went wrong.
|
Right, just wanted to make sure that case was tested |
Sorry, something went wrong.
* [perf] cache resolved data element options * Address review comments * Move uninitialized variables, update comments
| Back | FazBrowse Home | New Git URL |
Alternative approach that came to mind while taking a look at #6574
Closes #6382