- Notifications
You must be signed in to change notification settings - Fork59
Cache retrieved model from getter in embedded#158
base:master
Are you sure you want to change the base?
Cache retrieved model from getter in embedded#158
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| if(!getModel.get(embedCheckField)){ | ||
| getModel.fetch({ | ||
| success:function(model){ | ||
| varupdatedEmbeddeds=_.clone(parentModel.get('_embedded')||{}); |
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.
@westonruter can you explain why you are caching the parent embed here? does this prevent duplicate lookup calls? can you explain how to reproduce and i can work on a unit test to demonstrate what this fixes? thanks
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.
Yes, exactly. If we cache the fetchedembedded model inside of the parent's_embedded then subsequent calls will avoid doing external calls, and will instead return what was previously fetched.
The scenario where I encountered this problem was when I introduced arelated post REST field with agetRelatedPost model getter, and I wanted togetFeaturedMedia of that related post:https://github.com/xwp/wp-customize-featured-content-demo/blob/6243ce237ab2974d5d17b1d4a8895169bff3e67b/js/frontend.js#L8-L51
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.
Great, thanks, that should help figure out a unit test. I started working on a core merge changeset, I need to fix a few docs fixes that went into core I don't want to overwrite.https://github.com/adamsilverstein/develop.wordpress/tree/pr157. I verified the existing client unit tests all pass and we'll get a run on that branch in travis now as well - the travis:js run includes the wp-api client tests -https://travis-ci.org/adamsilverstein/develop.wordpress
Builds on#157. The one new commit is07fea24.
Needs unit tests.