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

Child bindings are called before the parent in Stache #1866

Closed
dylanrtt opened this issue Aug 27, 2015 · 5 comments
Closed

Child bindings are called before the parent in Stache #1866

dylanrtt opened this issue Aug 27, 2015 · 5 comments
Labels
Milestone

Comments

@dylanrtt
Copy link
Contributor

In the following example, if I start on foo/view and route to bar/edit, the template code corresponding to foo/edit is inserted temporarily... somehow after bar/edit is, even though I see editing: bar on the screen at the end...?

{{#eq page 'foo'}}
  {{#eq action 'view'}}
    viewing: foo
  {{/eq}}
  {{#eq action 'edit'}}
    editing: foo
  {{/eq}}
{{/eq}}

{{#eq page 'bar'}}
  {{#eq action 'view'}}
    viewing: bar
  {{/eq}}
  {{#eq action 'edit'}}
    editing: bar
  {{/eq}}
{{/eq}}

This can be a serious problem if a complex component is being inserted that does things you potentially do not want to happen. As an example, I was getting the bug from #1865 because a component in the foo/edit part of the page was exporting its view model, which was really confusing...

http://jsbin.com/woqumidivi/2/edit?html,js,console,output

@justinbmeyer justinbmeyer added this to the 2.3.0 milestone Sep 15, 2015
@justinbmeyer
Copy link
Contributor

This has nothing to do with routing. It has to do with child "sections" being notified before their parent of a change. I'm not sure how exactly to fix the problem.

The example is something like the following:

if( page === "foo" ) {
  if( action === "view") {
    console.log("VIEW foo");
  }
  if( action === "edit") {
    console.log("EDIT foo");
  }
}

if( page === "bar" && action === "edit") {
  console.log("EDIT bar");
} 

The problem is that the "child section" bindings, the ones inside page === "foo" like action === "edit" are being called back before page === "foo" updates. This means that "EDIT foo" is shown temporarily, before page === "foo" fires and removes that binding.

I'm not sure what the best fix is. Ideally, parents would always update before children. But for that to happen, children need to know about their parents or vice versa.

However, we've removed the parent / child nodeList relationship in most cases with can.stache to improve performance.

A "hack" fix would to to not nest your if statements within components. Make a helper that takes an page / action combo like:

{{#onPage 'bar' 'edit'}} <page-edit/> {{/}}

I'll think about how this can be fixed.

@justinbmeyer
Copy link
Contributor

Unfortunately, the order of binding doesn't help anything. This is because in this example the "action" event is fired before the "page" event. So even though the outer sections are binding before child sections, the "action" event is still fired first.

@dylanrtt
Copy link
Contributor Author

Ugh. I was hoping it was just a router bug and not such a deep problem. Here's a new jsbin that uses a can.Map instead of the router (no change as you explained).

http://jsbin.com/kaxebezapi/edit?html,js,console,output

To workaround it, I basically did what you suggested and made a virtual property that is computed from both.

@dylanrtt dylanrtt changed the title Changing router states has unexpected side effects Child bindings are called before the parent in Stache Sep 15, 2015
@justinbmeyer
Copy link
Contributor

One solution might be to have a special type of event binding that takes into account the ordering of the binding when dispatching.

Essentially, we could look at the target of everything we are about to dispatch batch events to ... and sort by when the binding took place. It would run in O( n log n ).

This way, regardless if you listened to page or action ... whichever compute listened first would get that event dispatched first.

This raises a question ... should the order of event.type matter in a batch? Probably not. It's all supposed to be one transaction.

@justinbmeyer
Copy link
Contributor

So, I think my solution for #2093 can probably be re-purposed for this. Essentially, it keeps track of updates to computes based on a "depth". Computes get updated in order of their depth. The same thing can be done for this issue. Essentially, each live-binding section compute includes the number of its parents as part of its depth value.

The good thing about my solution is that it doesn't require sorting.

The trick will be that this depth is different from the current compute depth (which is the number of computes a compute depends on). A compute like {{#eq page 'foo'}} could still depend on a lot of computes, but need to be re-evaluated before a child like: {{#eq action 'view'}}.

I'll probably release 2.3.3 w/o a fix for this issue, and then start looking how I can share the same code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants