CvX
commented
6 years ago (Just for the reference, for those not familiar with this problem, here's the earlier issue in the ember repo: https://github.com/emberjs/ember.js/issues/15662)
Closed mansona closed 6 years ago
CvX
commented
6 years ago (Just for the reference, for those not familiar with this problem, here's the earlier issue in the ember repo: https://github.com/emberjs/ember.js/issues/15662)
mansona
commented
6 years ago And as it turns out I was the last to comment on it 😂 Yes i did have this same issue when I had a loading page on 👍
xg-wang
commented
6 years ago You were using deferRendering the wrong way in the page service.
I cleaned up the service and it works fine, you can take a look at my changes:
https://github.com/xg-wang/guides-app/commit/f78ee16d36c1b65847dca2358b5fbe7edfe3078e
CvX
commented
6 years ago @xg-wang in your cleanup you removed set(this.get('headData'), 'title', `Ember.js - ${sectionTitle}: ${pageTitle}`);.
In fact the page service no longer uses the headData service anymore. So, no dynamic page titles, hence no errors. But that's hardly a solution. 😉
mansona
commented
6 years ago @xg-wang do you think you could do a more minimal change that demonstrates what you are trying to communicate? 🤔
I had a look at your change a few times over the last few days but because it is essentially a re-write I wasn't able to follow 😞
xg-wang
commented
6 years ago @CvX @mansona
Sorry I think I made it wrong. I thought it was caused by using deferRendering outside init with cp or use DS.PromiseObject, but it works well if I compose these in a small example.
However I can confirm it's not because of headData, removing that the error still exists.
I will probably have more time next week and would let you know if I can locate the root cause.
xg-wang
commented
6 years ago @CvX @mansona Sorry I ate my word, only got time tonight and did some debug.
So I think I find the scenario for this issue.
Mixin has property can be passed to child components, and contains store query; That store query is not guarded by Fastboot, and got triggered in during SSR but outside the Fastboot promise chain.
It's a bit complicated, What happened here go as follow:
this.store.query('page', ...)model.pages to pagescurrentPage and currentSection once pages.[] (AKA model.pages) change.currentPage and currentSection to a component{{guides-article}} component, which inside it has {{chapter-links}} who deals with Fastboot correctlymansona
commented
6 years ago Hey @xg-wang I'm sorry but I'm still not quite following 😞
Any chance you are free to hop on a call soon to discuss?
mansona
commented
6 years ago After a quick Slack chat with @xg-wang it seems like this is no longer an issue for the Ember Guides App 🎉
@xg-wang mentioned that there was probably something that could be done to improve the error message but I'll leave that to them to comment 👍
I'm closing this for now
xg-wang
commented
6 years ago The error message "TypeError: Cannot read property 'createComment' of undefined" from Glimmer is quite confusing, but is usually caused by out of model hook data loading without Fastboot guard. We should probably have a better error report.
sdhull
commented
6 years ago We started getting this recently and I have no idea why. Also there was a similar issue submitted to liquid-fire with no resolution.
For those of us coming here after googling Cannot read property 'createComment' of undefined could you give us a suggestion as to how we should go about debugging this error? Or a summary of "this happens when x + y" or something?
That store query is not guarded by Fastboot, and got triggered in during SSR but outside the Fastboot promise chain.
@xg-wang what does this mean? That deferRendering(promise) wasn't called?
@mansona I'm glad you got your issue resolved but the resolution apparently happened in a slack call that the rest of us weren't present for so I (and probably others!) would really appreciate it if you could give us a quick write-up of how this was solved for you and how the rest of us might solve the same thing! 🙏❤️
xg-wang
commented
6 years ago @sdhull The talk was a quick one, just confirming the master branch has already avoid the issue by removing the page Mixin, we didn't dig into the senario I mentioned above. I would say I spent quite some time to debug where the render was triggered without deferRendering guard.
@tomdale @kratiahuja Any comments? Seems this has very bad developer experience impact on fastboot.
Correct me if wrong: The issue is caused by Fastboot mode hasDom is false thus disable updateOperations in glimmer
The reason to do so is to have a promise to await all the information of operations to finalize the HTML, but the dark side is glimmer throwing wild error message if updateOperation is being made (almost always the case deferRendering is missing somewhere).
Is there a way to provide more meaning ful debug information for developers to locate the src for causing the issue in glimmer/fastboot?
Or is there a way to allow updateOperation? (maybe by updating the renderSettledDeferred promise internally)
sdhull
commented
6 years ago @xg-wang one strange thing is we have this on a page where I'm quite certain there are no requests being made after the model() hooks. To be sure, I set a debugger in setupController(), when it hit the debugger, checked the Network tab & all XHR requests had returned 200, I cleared the Network tab then let it run and there were no XHR requests after that.
Is there something else I can do to investigate further?
xg-wang
commented
6 years ago @sdhull
My recommendation would be:
node --inspect-brk ./node_modules/.bin/ember s or go with vscode with this configIf the above still doesn't help, I can help more if you provide a reproduction.
mansona
commented
6 years ago @sdhull I'm sorry but unfortunately the summary of what fixed it for the guides app is that we just restructured things a bit and the error went away. As @xg-wang is suggesting, there was probably something happening asynchronously that was causing some sort of re-render but after a refactor of that side of the app it seemed to just start working again.
I'm sorry it's not the "write up" that you were hoping for 😞
sdhull
commented
6 years ago @mansona alas, it is not. But I appreciate the reply nonetheless! 😬
OK I started the app in node debugger but I wasn't sure how to proceed. Should I add debuggers to my page? Should I pause on uncaught exceptions? I've tried both but even being able to step through the backtrace isn't very enlightening.
After looking more closely at the backtrace, I see najax and jquery-deferred/lib/jquery-callbacks early in the stacktrace, so I guess it is caused by a fetch.
I had started writing the above a few days ago. Today I finally tracked down what was causing this.
In my model hook, I'm essentially returning RSVP.hash({foo: foo, bar: bar}), where foo has_many bars & bar belongs_to foo.
But (foolishly) in the template I did {{foo.bar.baz}}. I believe the {{foo.bar}} portion returns a promise (because async relationships). And even though the promise should be resolved without another request (in theory), I guess it's a promise nonetheless and that promise is outside the "fastboot promise".
sdhull
commented
6 years ago FYI we ran into this again and would like to bring up another case where people might run into this.
We have a service with mutable state that affects rendering at the bottom of the page. A component in the middle of the page makes an ajax call to fetch a remote resource, and it properly use deferRendering with fastboot, however after that promise resolves, it mutates the state in that service which caused this other component to rerender, the rerender caused the dreaded createComment error.
We refactored but it seems like maybe fastboot should somehow automatically deal with re-renders after all deferRendering stuff is settled?
mansona
commented
6 years ago @sdhull that actually aligns with the use case that we were noticing it 👍we had a page service that received data in an afterModel and then made requests of its own and could end up causing a re-render.
We refactored to move all requests into the model() hook and for the page service to be more "functional" or "pure" or whatever you call it 😂
habdelra
commented
5 years ago So my experience is that you receive this error when you pass an argument to fastboot.deferRendering() that is not actually a promise (and perhaps more generally, not the promise(s) that is triggering the async leak in fastboot). In my particular scenario, I'm using ember concurrency, and I had this logic:
init() {
this._super(...arguments);
if (this.get('fastboot.isFastBoot')) {
this.get('fastboot').deferRendering(this.get('fetchSong').perform());
}
},Which resulted in this error. After more carefully examining the API docs for ember concurrency, the perform() actually returns a TaskInstance, not a promise. To get a promise from a TaskInstance you need to have a .then(). Changing my init() to the following (note the perform().then()) made this error go away:
init() {
this._super(...arguments);
if (this.get('fastboot.isFastBoot')) {
this.get('fastboot').deferRendering(this.get('fetchSong').perform().then());
}
},
Hi there 👋
I'm currently working with the Ember Learning team to turn the Ember Guides into an Ember App (yay), but I have come across something that I thought was intermittent but I have been able to consistently recreate it now for the first time.
To recreate just checkout https://github.com/ember-learn/guides-app/tree/bug/fastboot-createComment-error
npm iandnpm startand you should have the new guides-app running locally. Navigate to http://localhost:4200/v2.17.0/tutorial/routes-and-templates/ and you will see an error on the command line.Here is the stack trace:
My first thoughts are something to do with nesting
{{#each}}blocks because when you remove this line it all starts working again. I thought at first that it was the recursive component calls but the if you uncomment the following componentless implementation it still breaks.This is what leads me to think it was the nested
{{#each}}that is causing the problem.This issue does not happen in the browser, just in the fastboot environment.