search

canjs / can-stache

Live binding handlebars templates
https://canjs.com/doc/can-stache.html
MIT License
10 stars 13 forks source link

Use comment nodes in place of live-bound sections #680

Open phillipskevin opened 5 years ago

phillipskevin commented 5 years ago

In order to replace can-view-nodelist, we could insert comment nodes in place of live-bound sections. Some details of this will be explained below (and some are not yet figured out), but the high-level details are:

Put comment nodes in place of live-bound sections

This means that some stache like:

{{#if(items.length)}}
  {{#for(item of items)}}
    <p>{{item.name}}</p>
  {{/for}}
{{/if}}

...might return some DOM like:

<!-- if(items.length) -->
  <!-- for(item of items) -->
    <p>
      <!-- item.name -->Hello
    </p>
    <p>
      <!-- item.name -->World
    </p>

Use a WeakMap to store the "depth" of elements

Store the depth in the WeakMap when an element is inserted

When an element is inserted, recursively check its parents for an element with a depth, and set the depth of this element to its parent's depth plus 1.

The depth would only need to be stored for elements with bindings that will need to be torn down.

Use this depth to do the teardowns in the correct order

This depth can then be used to replace what can-view-nodelist does now.

From the can-view-nodelist docs:

<div>
{{#if items.length}}
    Items:
        {{#items}}
            <label></label>
        {{/items}}
    !
{{/if}}
</div>

When items.length value changes to 0, the inner content should be removed and the {{#items}} binding should be torn down.

How this would work exactly hasn't been totally figured out.

A few options will be discussed in a comment below.

phillipskevin commented 5 years ago

Options for handling teardowns:

1. Overwrite DOM methods

We could owerwrite the removeChild/appendChild/etc methods in order to know when to check an elements depth and remove its event handlers and the event handlers of its children in the correct order.

This isn't great because it breaks the goal of keeping this Isolated.

2. Make people use can-dom-mutate for DOM manipulation

This isn't great because it breaks the Invisible to users goal.

3. Use MutationObserver.takeRecords() at the end of the notify queue

One option is to use MutationObserver.takeRecords() at the end of the notify queue.

This would allow us to synchronously find all of the elements that have been removed, and tear down their bindings (and the bindings of their children) in the correct order based on their depth.

The downside of this is that it changes when mutation events are dispatched, which is likely a breaking change. We also aren't sure of the performance of this.

4. Use MutationObserver.takeRecords() with Zones

We could use Zones to figure out when to call takeRecords.

I'm not sure exactly how this would work, but it's another option we could consider.

It would most likely lead to a poor debugging experience, similar to the issues with debugging Angular apps.

phillipskevin commented 5 years ago

Potential issues with this solution:

1. This is likely to break apps

People using node.firstChild, node.children, etc would have to change their code since the DOM being rendered would contain these extra Comment Nodes.

2. You cannot "wrap" DOM

Something like this would cause problems since the "depth" would be changing:

{{#if(something)}}
<div>
{{/if}}

<p>{{otherThing}}</p>

{{#if(something)}}
</div>
{{/if}}

3. Performance

This will likely cause a performance degredation in stache. We'll need to test to see exactly what scenarios have worse performance and how bad it actually is.

phillipskevin commented 5 years ago

Added benfits of this solution:

1. Debugging

We would be able to use these Comment Nodes to show the {{#if(foo)}} helpers in CanJS devtools. This would be awesome.

justinbmeyer commented 5 years ago

There's a related problem to all of this ... it's that observation updates (in the derive queue) should be scheduled in relation to the element they are associated with. In stache, these observations are created with priority of their nodeList+1.

There's a test that sets this relationship up manually:

This tests this: https://github.com/canjs/can-view-live/blob/70d7de60628b1810b4d803beac5efb4421a6cd05/test/html-test.js#L107

If we are going recreate this, we need some way of making these computes evaluate late. We already have the ability to call [@can.setPriority](). Priorities could be an element?

Once an observable's priority is set, should this propagate to its children? For example, say I have a component like:

view: `{{this.fullName}}`,
ViewModel: {
  get fullName(){
    return this.user.first + this.user.last
  }
}

The observable created for {{this.fullName}} will get its priority set. Then should it similarly make fullName update "later", same with this.user.first and this.user.last if they are derived? This would be some form of "priority pressure". Once an Observation is associated with an element, all other "derivable" observables it reads from should be told to update late.

What if there are conflicts? Say this.user.first is derivable, but a parent component is reading it? Presumably, whichever is earlier wins. Also, Observation.updateChildrenAndSelf might solve this.

A related problem is that elements are not always in the page and then they get inserted later. This might be a problem if element-associated tasks are queued and then their element is inserted. We might have to re-arrange the queue.

Could we keep these separate and do a in-page check and re-insert?

Suggestion