node.parentNode is null

[rsileoni]

I stumbled upon the same issue described in #726, maybe the bug is still present

http://jsfiddle.net/wDNQR/

Thanks

[rsileoni]

Here is a failing test http://jsfiddle.net/64RSZ/ Oddly enough the test fail on Fiferox but pass on Chrome :(

[jonvuri]

Here is the line where the error occurs: https://github.com/ractivejs/ractive/blob/dev/src/virtualdom/items/Triple/prototype/update.js#L14

I haven't been able to isolate the root issue, but I do have a few observations: First, should node.parentNode be checked to exist before calling a method on it? Or should it definitely exist at this point, even if a decorator has done something such as in this fiddle, where it sets the new html directly (possibly orphaning the nodes in this loop)? Second, why when you put a debug point on the above line and run the fiddle in Chrome does the text node have value 'qux', but in Firefox it still has the old value 'baz'? It's this second inconsistency that makes me hesitate to just add a node.parentNode null check, since it might point to an earlier problem.

[Rich-Harris]

Am (finally) looking into this. I'm not sure whether to consider this a bug or not - basically, the decorator is modifying the part of the DOM that the triple-stache is responsible for, so when the triple comes to update itself, it finds that its nodes no longer exist in the document.

We could add a check for node.parentNode to see if a given node is still in the document, but that's a kludge that's specific to this case, and it doesn't really solve the problem - the triple will create fresh DOM (because task.name has changed) immediately after you've created DOM of your own with this line:

// this has the effect of chucking out the DOM that the triple is responsible for
// and creating a new text node that Ractive has no knowledge of or control over
update: function (txt) {
  node.textContent = txt;
}

I tracked down the discrepancy between Chrome and Firefox, and it's quite interesting - turns out that Chrome will compare the value of the HTML before and after, and if it's just replacing one text node with an identical one, it short-circuits. Firefox isn't that smart; it creates a new text node whether it's needed or not. Try running this in both consoles:

body = document.body;

body.innerHTML = 'foo';
n1 = body.firstChild;

body.innerHTML = 'foo';
n2 = body.firstChild;

console.log( n1 === n2 ); // true in Chrome, false in FF

[MartinKolarik]

I don't think this is a bug. Static mustaches would be a good solution here or maybe even something like this (though I don't like the setTimeout hack).

[martypdx]

@robertosileoni You're making this way too complicated :smile:

Reactive programming means use declarations over imperative procedures (see http://jsfiddle.net/Ct7DM/1/):

    <li 
        style="font-weight:{{#highlight}}bolder{{/highlight}}"
        on-focus-input="highlight:true" 
        on-blur="highlight:false" 
        contenteditable="true" > 
        {{name}}
    </li>

    init: function () {
        this.on('highlight', function(e, val){
            this.set('highlight', val)
        }.bind(this))
    }

But, with css you don't even need that:

li:active, li:focus {
    font-weight: bolder;
}

[martypdx]

@MartinKolarik, @Rich-Harris: We could do checks and throw errors to look for modified nodes, but it seems that it would be overly pervasive to always check if a node had been mucked with.

What might make more sense, as an enhancement, is to set a MutationObserver when debug===true on the nodes that get inserted and throw if they get removed. That way we could streamline the effect on the code base to a few node insertion points, and limit this to a development time activity (when MutationObservers are feasibly supported in the browser)

[Rich-Harris]

@martypdx That's an interesting idea. I'm not totally sure what we'd do with the information from a mutation observer, but it seems like it could be useful in certain situations. I don't think it should be in the core library though - we could discuss it separately, I'm going to close this issue as there isn't really a bug here that needs addressing.

[rsileoni]

@Rich-Harris Thanks for the explanation, and sorry for the late reply. I agree with you that this can not be considered a bug, but rather a bad practice.