[BUGFIX beta] Ensure App.visit resolves when rendering completed.
#16087
Conversation
Prior to this change, rendering was not guaranteed to have been completed before the promise returned from the `visit` API had resolved. With these changes no additional delay is added, but we have still ensured that all rendering is completed.
| run.schedule('afterRender', null, resolve, this); | ||
| }); | ||
| // Ensure that the visit promise resolves when all rendering has completed | ||
| return renderSettled().then(() => this); |
There was a problem hiding this comment.
@rwjblue Sorry to bring up something old, but seeing some memory leaks in fastboot related to retaining the Container. Is it necessary to return this in the callback? I think it is, but wondering if it is possible this never resolves but the app still is rendered.
What I'm worries about is never reaching a settled state or resolving, thus every request that comes in builds a new instance (w/ shoebox data) and the eventually swamps the server's heap.
There was a problem hiding this comment.
some more info. I'm wondering if we need to "settle" the deferred promise when shouldRender is false....So not this block but the above block.
There was a problem hiding this comment.
I don't really follow. Can you file an issue with a reproduction?
There was a problem hiding this comment.
@rwjblue - We found it. Well @Serabe found it so I'll let him explain beyond this short answer! In an adapter, he found that headers as a computed was potentially leaking the container thus not allowing V8 to not gc anything. The problem wasn't necessarily ajax/najax/node-fetch but the way we configured our adapter.
// adapter
headers: computed(function() {to a getter fixed it.
// adapter
get headers() {We are on 3.5 and still investigating
Creates a new internal method (named
renderSettled) that returns a promise which will resolve when rendering has settled. Settled in this context is defined as when all of the tags in use are "current" (e.g.renderers.every(r => r._isValid())). When this is checked at the end of the run loop, this essentially guarantees that all rendering is completed.Prior to this change, rendering was not guaranteed to have been completed before the promise returned from the
visitAPI had resolved (yes, even thoughafterRenderqueue was being used). With these changes no additional delay is added, but we have still ensured that all rendering is completed...Fixes #16059