-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
{{component}} helper race condition with associated model #10739
Comments
if possible, can you provide an repro that demo's this, maybe an ember-cli app that has this issue? |
@trentmwillis I've had issues with this as well, but I think the core issue is the |
Related to #10302. Unsure how to move that PR forward. |
Got a reproduction working here: https://github.com/trentmwillis/ember-component-helper-issue So it seems my initial assumption was wrong, our actual usage is more like: {{component card.model.type ...}} And so the issue appears to be with having nested model properties. I reproduced in the above repo with both fixture and http-mock data. |
Update: applying the solution given in #10302 didn't fix anything. I did track down that it is failing out here in the Since I don't quite understand streams and what read is hoping to accomplish I'm not sure I can offer a fix for this. I did note that changing line 33 to be: var componentClass = this.componentClassStream.cache || read(this.componentClassStream); Fixes the problem, but I don't think is a correct solution. |
So after digging into this more, I can't seem to reproduce without using Ember Data Models. So I'm thinking I should probably open a there instead. |
Can confirm, this only happens in the process of At that point the component tries to re-create itself ( Possible solutions:
|
@lukemelia @stefanpenner Do you have a preference as to which way we solve this? It's blocking a few things and I'd like to go ahead and get a PR in. |
@nathanhammond thanks for looking into this! For what it's worth, I think enforcing ordering in the destroy makes the most sense. Mainly because having errors thrown in the case of undefined may still be useful in other situations. |
@trentmwillis The caveat is that most things in Handlebars render nothing when passed a falsey value, implying that I'm not sure which approach is the solution with the "least surprise." |
@lukemelia @nathanhammond @trentmwillis so a couple of things here:
|
More information (notes for me): willDestroy is called after the stream subscribed to here is updated–meaning that |
A thought, since isDestroying is not set on the stream, would it be possible to check for it on the container? I feel like that would be a simple solution until glimmer lands |
I encountered something similar when doing {{#if newContentModal}}
{{component newContentModal
lesson=lesson
dismiss='dismissModal'}}
{{/if}} When I set |
@samselikoff Did you feel dirty while doing it? ;) It seems like we should probably make the component helper not attempt to render a component with falsey values. Also, hard failures for missed lookups often seen in @trentmwillis and I might take a swing at a safer component helper today... paging @lukemelia for thoughts. |
@nathanhammond That seems like a reasonable first step to me. The solution that @krisselden suggested is more holistically correct but also much more involved. |
Should be fixed as the test from #10302 is passing (as of Glimmer restructuring). Happy to reopen if this is still an issue... |
The
{{component}}
helper seems to have a race condition that is causing problems to surface in tests that I am writing.Usage is like so:
Then when tests are run, a failure occurs during the
afterEach
whendestroy
is run on theapplication
instance.If I change the above usage to use a static string rather than a model property like so:
The tests and teardown are fine. This leads me to believe it's being caused by the model being destroyed before the component, resulting in the component having a type of
undefined
.Potentially related PR, #10302
The text was updated successfully, but these errors were encountered: