canjs / can-view-nodelist

Node list helpers
https://canjs.com/doc/can-view-nodelist.html
MIT License
3 stars 0 forks source link

New NodeList API #37

Open justinbmeyer opened 6 years ago

justinbmeyer commented 6 years ago
test('unregisters child nodeLists', function () {
    expect(3);
    // two spans that might have been created by #each
    var spansFrag = fragment("<span>1</span><span>2</span>");
        var spansList = new NodeList( spansFrag );
        spansList.onUnbound(function(){
        ok(true,"unregistered spansList");
    });

    // A label that might have been created by #foo
    var labelFrag = fragment("<label>l</label>");
    var labelList = new NodeList(labelFrag);
        labelList.onUnbound( function(){
        ok(true,"unregistered labelList");
    });

    // the html inside #if}
    var ifList = new NodeList( frag(["~","","-",""]) );

    ifList.splice(1,1,spansFrag);

    // 4 because 2 elements are inserted, and ifChildNodes is live
    ifList.splice(4,1,labelFrag);
        ifList.onUnbound(function(){
        ok(true,"unregistered ifList");
    });

    deepEqual(ifList.nodes,[
        ifEls[0],
        spansList,
        ifEls[2],
        labelList
    ]);

        ifList.replace( [document.createTextNode("empty")] );

});
justinbmeyer commented 6 years ago

We could make these nodelists the default return value for can-stache and any other renderer.

Example:

var nodeList = stache("<div/><p/>")();

nodeList.disconnect() //-> allow us to tear down all live-bindings

document.body.appendChild(nodeList.fragment) //-> insert the nodeList's items into the dom

nodeList.onDisconnected( function(){ //-> alternate for onUnbound above

})

We could use mount for naming (like React) if that makes more sense.

@phillipskevin we should think about this hard for 5.0. How would this work with jsx?

@andrejewski has had some thoughts on this too.

justinbmeyer commented 6 years ago

NOTE: the following are unformed thoughts.

An alternative approach might be to add a "domMutated" queue to CanJS. The queues would be:

notify -> derive -> domUI ~> domMutated -> mutate

The ~> indicates that domMutated would be fired asynchronously. At the end of domUI, we'd want to run all domMutated tasks after all MutationEvents would have had time to fire.

This would make mutate also fire asynchronously, which would be a breaking change (although many users might not notice).

As we are introducing asynchronous dispatching, I would probably have domUI run as part of requestAnimationFrame:

notify -> derive ~> domUI ~> domMutated -> mutate

This will mean we modify the DOM only when the browser is doing a repaint anyway.

How this removes nodelists

This would allow us to avoid nodeLists because we would be able to remove a node, and be able to teardown any children bindings before "mutate" would be called.

There's one problem with this ... derive might still be called. I think that domMutated might need to be a special queue that must ALWAYS run and finish before we would dispatch any additional notify/derive queues.

The hope is that the removal would make sure any bindings are unbound.

The problem

The big problem nodeList solves is that deeply nested children should not be updated if a parent is trying to tear down the child. It currently works b/c every child adds itself to its parent nodelist.

CanJS also tries to make it so mutating the page outside CanJS can also teardown any bindings.

It feels odd that we are creating our own hierarchy outside of the DOM. The historical reason was that:

  1. The DOM was VERY slow.
  2. textNodes couldn't have expando properties in old browsers.
  3. jQuery didn't support events on textNodes
justinbmeyer commented 6 years ago

MutationObserver::takeRecords

I wonder if MutationObserver::takeRecords might help improve the design/use of nodeLists

One of the core uses for nodeLists is so that we can recursively teardown child can-view-live bindings before they might cause unnecessary re-renders.

The simplest change might be to make can-dom-mutate, upon removing any nodes, call takeRecords and process those records.

This would call the teardown function on all ( this needs fixing ) elements. They would unbind immediately.

This might be a breaking change as people expect inserted/removed to fire synchronously ... but there might be a way to "fake" this.

In the end, can-view-live would simply be listening for when these elements are removed, but be told synchronously. can-view-live.html would probably have to change which element it is listening to for removal, but that's not hard, and presumably not slow.

Queues alternate approach

An alternate approach would be for the queues to call .takeRecords() in between each queue. This is likely harder, more error prone, and links too much of CanJS together.

matthewp commented 6 years ago

@justinbmeyer I worry about performance with the takeRecords() idea. Wasn't the main reason why MutationEvents were deprecated because of perf? Using takeRecords() synchronously is equivalent to that, I think.

justinbmeyer commented 6 years ago

I think they were deprecated for performance, but I don’t know how much being synchronous was the reason. I think it was more about how heavy event dispatching is, and how global it was. I’m guessing though.

justinbmeyer commented 5 years ago

Target Provides NodeList

Another idea I had ... have can-view-target callback with a NodeList instead of of a placeholder text node.

For example:

var target = viewTarget([
  { 
    tag: "div", 
    children: ["Hello", function placeHolderCallbacks(){
       this //-> NodeList[textNode]
       this.replaceWith( fragment( "<span/>" ) )
    }] 
  }
])

var result = target.hydrate();
result //-> NodeList[div]

.hydrate() will return a nodeList that contains references too all of the children NodeLists that are created for placeHolderCallbacks.

result would look something like:

result {
  0: NodeList{
     // 0: TextNode<"Hello">, ... I don't think this is needed ...
     1: NodeList: {0: <span/>}
  }
}

One possible performance issue is that we will create nodes for all paths that lead to callbacks. For example, stache like:

<div>
  FIRST
  <label>
    {{SOMETHING}}
  </label>
</div>
{{SOMETHING_ELSE}}

Might result in a nodeLists like:

{
  0: NodeList{ // this is the <div>
   1: { // this is the  <label>
     1: NodeList // this is for {{something}}
   }
  },
  2: NodeList // this is for  {{something_else}}
}

I'm not sure this will be a performance problem because we already walk all the elements that are parents of callbacks anyway. We would just be building this NodeList structure while we are doing this.

Ideally, mustacheCore is doing something like this:

var branchRenderer = function branchRenderer(scope, parentSectionNodeList, truthyRenderer, falseyRenderer){
  this //-> NodeList<text>
  this.expression = expression; // for debugging 
  // no need to register ... 
  var evaluator = makeEvaluator( this ) //-> pass the nodeList so `makeEvaluator` will use it to make sure `.fn` and such always pass it down.
  var obs = new Observation(evaluator);
  tempBind(obs);
  viewLive.html( obs, this );
}

viewLive.html will now expect NodeList. In there it would do something like:

nodeList.replaceWith( obs.value );

The positive for this change:

The negative:

justinbmeyer commented 5 years ago

can-view-live.html:

https://github.com/canjs/can-view-live/blob/d336c85024ead7004a867963e68a34fd7291b4fc/lib/html.js#L147

enqueues changes to happen in the future. This is to enable other, "higher in the dom tree" changes to happen which will prevent the DOM from actually changing.

All this is to say that I think that DOM updates need to be hierarchy aware. We already have this with nodeList depth. However, the domUI queue does not use a hierarchy.

justinbmeyer commented 5 years ago

So a problem with using domMutatation.onNodeRemoval backed with MutationObserver is that it does not fire when disconnected elements are removed from their parent.

Bar would never know to tear itself down with:

stache("{{#foo}} {{bar}} {{/foo}}")

Possible solutions

Force all stache templates to have a parentNode

If all stache templates had a parent node, could we listen on the parent for when our element is removed?

Problems:

onNodeRemoved with MutationObserver

Have domMutate include a onNodeRemoved callback fires when the element is removed in anyway. Ideally, we'd be able to allow people to mutate the dom without being aware of domMutate. Worst case, they would need to call domMutate.flushRecords() after mutating connected DOM.

The biggest concern is how to make this reasonably fast. Currently, in browsers that support MutationObserver, we alias domMutateNode.removeChild and other methods to their native equivalent.

We would now have to do a runtime isInDocument check on the element being removed to determine if it is in the page. If it is in the page, we can use the native removeChild, if it's not, we will need to add a special mutation record for the removal.

I'm not sure how fast isInDocument() is (it would use contains which I assume is a parentNode walk). This check only exists so we can use normal mutation observers. See onNodeRemoved withOUT MutationObserver.

Stache might be able to switch to different modes (using MO or adding special mutation records) by listening to a onNodeConnected(). We could even probably keep this inside domMutate by holding a WeakMap with elements to true if they are connected. We'd just look up this value to decide what to do. If the value wasn't present, we call isInDocument.

Other:

The existing onNodeRemoval should be renamed to onNodeDisconnected and onNodeInsertion to onNodeConnected to avoid confusion on the difference between onNodeRemoved and onNodeDisconnected.

Pros:

Cons:

onNodeRemoved withOUT MutationObserver

onNodeRemoved symbol

Instead of callbacks, we could support people adding can.onNodeRemoved on their elements and we'd make can-view-live go clean those references up.

Cons:

Force people to mount into the DOM

Somehow, we could expect people to always mount into the DOM. I'm not sure how to do this, but the following problem would need to be solved ...

Imagine the following template, where:

{{#this.foo}}
  {{abc}}
{{/this.foo}}

<child-comp foo:to="this.foo">

In this scenario, if the fragment was built, but not connected, when {{abc}} is removed, it's disconnected callback would not fire.

Cons: