yandex-ui / noscript

Noscript: JavaScript MVC Framework for building SPA
http://yandex-ui.github.io/noscript/
MIT License
34 stars 22 forks source link

ns.ViewCollection баг обновления ноды вида #590

Open chestozo opened 8 years ago

chestozo commented 8 years ago

Вот тут: https://github.com/yandex-ui/noscript/blob/master/src/ns.viewCollection.js#L671 ns.replaceNode(view._extractNode(newNode), view.node);

но если посмотреть на ns.replaceNode https://github.com/yandex-ui/noscript/blob/master/src/ns.dom.js#L9:

ns.replaceNode = function(oldNode, newNode) {
    // такая вот защита от лишних действий
    if (oldNode === newNode) {
        return true;
    }

    // если oldNode детачена из DOM, то у нее нет родителя
    if (oldNode.parentNode) {
        oldNode.parentNode.replaceChild(newNode, oldNode);
        return true;
    }

    return false;
};

Ну т.е. по ощущениям должно быть наоборот: ns.replaceNode(view.node, view._extractNode(newNode));

Я прав / не прав?

chestozo commented 8 years ago

После фикса тесты не упали, плохо покрыли :/

eljusto commented 8 years ago

Судя по коду, да https://github.com/yandex-ui/noscript/search?utf8=✓&q=ns.replaceNode В других местах порядок аргументов правильный.

chestozo commented 8 years ago

Подебажил на тестовом проекте - вроде бы всё корректно работает в текущей реализации https://github.com/chestozo/noscript-demo. У нас по логам есть кейс, когда view._extractNode(newNode) ничего не вернул. Как такое произошло пока не ясно.

fresk-nc commented 8 years ago

Почитал внимательно комменты _updateHTML, кажется, что это запланированное поведение

chestozo commented 8 years ago

У нас был как-то nullreference тут. Предлагаю пока оставить.

chestozo commented 7 years ago

Возможно, тоже связано #579