Giter Site home page Giter Site logo

Comments (19)

sdhull avatar sdhull commented on July 26, 2024 2

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! 🙏❤️

from fastboot.

xg-wang avatar xg-wang commented on July 26, 2024 2

@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

  1. Fastboot set isBrowser to false
  2. hasDom = isBrowser
  3. register updateOperations if hasDom

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)

from fastboot.

mansona avatar mansona commented on July 26, 2024 2

@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 😂

from fastboot.

mansona avatar mansona commented on July 26, 2024 1

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 👍

from fastboot.

habdelra avatar habdelra commented on July 26, 2024 1

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());
    }
  },

from fastboot.

CvX avatar CvX commented on July 26, 2024

(Just for the reference, for those not familiar with this problem, here's the earlier issue in the ember repo: emberjs/ember.js#15662)

from fastboot.

xg-wang avatar xg-wang commented on July 26, 2024

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:
xg-wang/guides-app@f78ee16

from fastboot.

CvX avatar CvX commented on July 26, 2024

@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. 😉

from fastboot.

mansona avatar mansona commented on July 26, 2024

@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 😞

from fastboot.

xg-wang avatar xg-wang commented on July 26, 2024

@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.

from fastboot.

xg-wang avatar xg-wang commented on July 26, 2024

@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.

tl;dr

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.

Details

It's a bit complicated, What happened here go as follow:

  1. version route query store this.store.query('page', ...)
  2. version controller alias model.pages to pages
  3. page mixin registers computed property, return currentPage and currentSection once pages.[] (AKA model.pages) change.
  4. version template pass currentPage and currentSection to a component
  5. version/show component has another {{guides-article}} component, which inside it has {{chapter-links}} who deals with Fastboot correctly
  6. page service However, during the fastboot call in step5, there is a store query, which will trigger change in step1.
  7. Boom... step4 do the render without Fastboot guard.

from fastboot.

mansona avatar mansona commented on July 26, 2024

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?

from fastboot.

mansona avatar mansona commented on July 26, 2024

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

from fastboot.

xg-wang avatar xg-wang commented on July 26, 2024

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.

from fastboot.

sdhull avatar sdhull commented on July 26, 2024

@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?

from fastboot.

xg-wang avatar xg-wang commented on July 26, 2024

@sdhull

  1. It doesn't necessarily be the network request. Any async things outside fastboot promise will break, a XHR/fetch request is a common case for this.
  2. This exception happens inside fastboot, which is the node environment. When you set the breakpoint and check the network tab you're in the browser environment.

My recommendation would be:

  1. Do debug in node, something like: node --inspect-brk ./node_modules/.bin/ember s or go with vscode with this config
  2. Check any data on your template and their related async code

If the above still doesn't help, I can help more if you provide a reproduction.

from fastboot.

mansona avatar mansona commented on July 26, 2024

@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 😞

from fastboot.

sdhull avatar sdhull commented on July 26, 2024

@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".

from fastboot.

sdhull avatar sdhull commented on July 26, 2024

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?

from fastboot.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.