Skip to content
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

Tries to reuse a destroyed view when rerendering an outlet #9814

Closed
ef4 opened this issue Dec 4, 2014 · 3 comments · Fixed by #10372
Closed

Tries to reuse a destroyed view when rerendering an outlet #9814

ef4 opened this issue Dec 4, 2014 · 3 comments · Fixed by #10372

Comments

@ef4
Copy link
Contributor

ef4 commented Dec 4, 2014

  1. Put this in application.hbs:

    {{input type="checkbox" checked=showOutlet}}
    {{#if showOutlet }}
      {{outlet}}
    {{/if}}
  2. Put this in index.hbs:

    hello world

  3. Toggle the checkbox. The second time you toggle to true, Ember throws Assertion Failed: calling set on destroyed object. It looks to me like it's reusing a view that had already been destroyed on the previous render of the #if block.

This bug effects canary, both with and without HTMLBars. Under 1.8, the outlet fails to re-render, but it doesn't throw an exception.

@wagenet wagenet added the Bug label Dec 9, 2014
@oneeman
Copy link
Contributor

oneeman commented Dec 15, 2014

I'm looking into it. So far I've added a failing unit test for this (getting the assertion error as described). Hope to sort it out before too long, but if not I'll submit the test and punt fixing it to someone else.

@oneeman
Copy link
Contributor

oneeman commented Dec 16, 2014

For reference, here are a couple of old issues about this: #3187, #3929,

@oneeman
Copy link
Contributor

oneeman commented Dec 17, 2014

From looking at the old issues, it seems it's not clear how best to handle it and I'm certainly out of my depths. I took the failing test @ebryn had for the old issue and changed a line to get it to work off the current master branch.

@wagenet How about putting a discuss tag on this like the old one and pinging the relevant folks to weigh in again now that it's confirmed the same problem is still present with HTMLBars?

ef4 added a commit to ef4/ember.js that referenced this issue Feb 8, 2015
This closes emberjs#9814 and closes emberjs#10304, which are examples of a class of
problems caused by the way the router synchronously reaches into the
view hierarchy to set outlet state. It is a significant refactor of the
way the router communicates state to outlets.

What motivates this change?

 - Simple examples like `{{#if something}}{{outlet}}{{/if}}` are
   incorrect under the current model.

 - Richer examples like block-helpers to enable animation also
   suffer. In general, the router cannot know when and whether a
   particular outlet is going to exist, and it shouldn't need to know.

 - The router maintains a bunch of view-related state that is actually
   redundant with the view hierarchy itself, leading to unnecessary
   complexity.

 - This eliminates the longstanding weirdness & confusion caused by the
   fact that we used to create new `View` instances and then throw them
   away if they looked similar enough to ones that were already
   rendered. That functionality is now covered by state diffing in the
   `OutletView`.

 - We reduce the API surface area between router and view layer in a way
   that should make it easier to experiment with swapping in compatible
   implementations of either.

 - As a bonus, this changes makes outlets work in an observer-free way
   that will make them easy to integrate with upcoming planned view
   layer optimizations.

How does this work?

 - Rather than directly building and linking views, the router builds up
   an abstract summary of the render decisions that have been made by
   the current routes.

 - This state is cheap to recalculate as needed. It doesn't do any view
   creation. To avoid expensive observer creation & teardown, we just
   recreate the whole thing and use a `setState`-like mechanism to
   propagate the changes through the outlet hierarchy. This gives us
   optimal granularity of updates.

 - Actual view instantiation moves into the OutletView -- within the
   view layer where it belongs. Each outlet does a diff to see whether
   it should rerender itself or propagate inner changes down to its
   child outlets.

 - To bootstrap rendering, the router creates a single top-level outlet,
   after which all view creation is internal to the view layer.

Does this break any existing semantics?

 - No, as far as I can tell.

Could this get even better if we decided to deprecate some old
semantics?

 - Yes. It would be better if users` `renderTemplate` implementations on
   `Route`s were required to be idempotent. Then we could eliminate a
   bunch of the remaining state from them.

 - Also, when we deprecate the `render` helper we can eliminate the
   remaining use of `_activeViews` state tracking on the router. That is
   the only remaining use for it.
@ef4 ef4 closed this as completed in #10372 Feb 8, 2015
ef4 added a commit that referenced this issue Feb 8, 2015
This closes #9814 and closes #10304, which are examples of a class of
problems caused by the way the router synchronously reaches into the
view hierarchy to set outlet state. It is a significant refactor of the
way the router communicates state to outlets.

What motivates this change?

 - Simple examples like `{{#if something}}{{outlet}}{{/if}}` are
   incorrect under the current model.

 - Richer examples like block-helpers to enable animation also
   suffer. In general, the router cannot know when and whether a
   particular outlet is going to exist, and it shouldn't need to know.

 - The router maintains a bunch of view-related state that is actually
   redundant with the view hierarchy itself, leading to unnecessary
   complexity.

 - This eliminates the longstanding weirdness & confusion caused by the
   fact that we used to create new `View` instances and then throw them
   away if they looked similar enough to ones that were already
   rendered. That functionality is now covered by state diffing in the
   `OutletView`.

 - We reduce the API surface area between router and view layer in a way
   that should make it easier to experiment with swapping in compatible
   implementations of either.

 - As a bonus, this changes makes outlets work in an observer-free way
   that will make them easy to integrate with upcoming planned view
   layer optimizations.

How does this work?

 - Rather than directly building and linking views, the router builds up
   an abstract summary of the render decisions that have been made by
   the current routes.

 - This state is cheap to recalculate as needed. It doesn't do any view
   creation. To avoid expensive observer creation & teardown, we just
   recreate the whole thing and use a `setState`-like mechanism to
   propagate the changes through the outlet hierarchy. This gives us
   optimal granularity of updates.

 - Actual view instantiation moves into the OutletView -- within the
   view layer where it belongs. Each outlet does a diff to see whether
   it should rerender itself or propagate inner changes down to its
   child outlets.

 - To bootstrap rendering, the router creates a single top-level outlet,
   after which all view creation is internal to the view layer.

Does this break any existing semantics?

 - No, as far as I can tell.

Could this get even better if we decided to deprecate some old
semantics?

 - Yes. It would be better if users` `renderTemplate` implementations on
   `Route`s were required to be idempotent. Then we could eliminate a
   bunch of the remaining state from them.

 - Also, when we deprecate the `render` helper we can eliminate the
   remaining use of `_activeViews` state tracking on the router. That is
   the only remaining use for it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants