Есть ли какие-то проблемы или сложности с исправлением этой проблемы путём обновления virtual-dom?
Мы приветствуем pull request, мы пытались, но это оказалось слишком сложно
2 лайка
Очень интересно! Переход на форк vdom не является идеальным решением, но мы могли бы рассмотреть его при условии, что сможем доказать отсутствие регрессий (и перенесём его в нашу собственную организацию на GitHub).Вам удалось запустить набор тестов vdom, чтобы подтвердить отсутствие регрессий? Насколько сложно было бы добавить новый тест для vdom, проверяющий это поведение с добавлением в начало?
2 лайка
Сначала отвечу на второй вопрос. Такое поведение с добавлением в начало по сути такое же, просто оно устраняет лишнюю работу. Я постараюсь проиллюстрировать, что делает vdom изначально и после патча.
Предположим, есть 20 постов, а после прокрутки сверху загружается ещё 10.
Изначально vdom выполняет следующую последовательность преобразований:
[20 старых] → [20 старых, 10 новых] // 10 новых элементов добавляются в конец
[20 старых, 10 новых] → // все 30 элементов удаляются
→ [10 новых, 20 старых] // все 30 элементов вставляются на свои места
После патча последовательность становится такой:
[20 старых] → [20 старых, 10 новых] // 10 новых элементов добавляются в конец
[20 старых, 10 новых] → [20 старых] // 10 новых элементов удаляются из конца
[20 старых] → [10 новых, 20 старых] // 10 новых элементов добавляются в начало
Как видите, лишняя работа всё ещё есть: в принципе, эти элементы можно было бы добавить в начало сразу, но таким образом я мог просто добавить немного кода в одном месте и не беспокоиться о переписывании уже существующего.
Каждое из преобразований становится операцией DOM, поэтому видео на YouTube начинают воспроизводиться, потому что изначально старые элементы вставляются заново (что вызывает их повторный рендеринг), а после патча они остаются на месте.
2 лайка
Вот фрагмент из файла package.json пакета virtual-dom
"scripts": {
"test": "node ./test/index.js | tap-spec",
"dot": "node ./test/index.js | tap-dot",
"start": "node ./index.js",
"cover": "istanbul cover --report html --print detail ./test/index.js",
"view-cover": "istanbul report html && opn ./coverage/index.html",
"browser": "run-browser test/index.js",
"phantom": "run-browser test/index.js -b | tap-spec",
"dist": "browserify --standalone virtual-dom index.js > dist/virtual-dom.js",
"travis-test": "npm run phantom && npm run cover && istanbul report lcov && ((cat coverage/lcov.info | coveralls) || exit 0)",
"release": "npm run release-patch",
"release-patch": "git checkout master && npm version patch && git push origin master --tags && npm publish",
"release-minor": "git checkout master && npm version minor && git push origin master --tags && npm publish",
"release-major": "git checkout master && npm version major && git push origin master --tags && npm publish"
},
Из них ‘test’, ‘dot’, ‘cover’, ‘view-cover’, ‘browser’, ‘phantom’, ‘travis-test’ кажутся связанными с тестированием.
Скрипты ‘browser’, ‘phantom’, ‘travis-test’ вызывают ошибку парсинга из-за использования более новых конструкций JavaScript в моём коде. Остальные проходят успешно. Если я изменю этот код
var prepend = simulate.every(item => item && item.key)
prepend &&= aChildren.every((item, i) => item.key === bChildren[i + shift].key)
на этот код
var prepend = true
for (var i = 0; prepend && i < simulate.length; i++) {
prepend = simulate[i] && simulate[i].key
}
for (var i = 0; prepend && i < aChildren.length; i++) {
prepend = aChildren[i].key === bChildren[i + shift].key
}
то все тесты пройдут. Я могу внести это изменение, чтобы поддерживать совместимость со старыми версиями JavaScript, если это необходимо для выполнения всех указанных скриптов.
2 лайка
пожалуйста, сделай это — давайте сохраним работоспособность их существующего набора тестов
Я только что форкнул virtual-dom в GitHub - discourse/virtual-dom: A Virtual DOM and diffing algorithm · GitHub — не могли бы вы создать PR против этого репозитория?
3 лайка
3 лайка
Также стоит отметить, что мое изменение в virtual-dom очень узконаправленное. Оно касается именно вставок в начало, а во всех остальных случаях используется исходный алгоритм. И этот исходный алгоритм всё ещё несовершенен, если рассматривать проблему в целом (не так уж сложно придумать примеры, когда он будет ненужно затрагивать старые элементы). С другой стороны, он отлично справляется с вставками в конец, удалением и одиночными вставками. А чтобы обойти его, нужно действительно изощрённо работать с потоком постов. Поэтому, с практической точки зрения, решение проблемы в общем виде может быть излишним, хотя, конечно, спать спокойнее, когда это сделано.
Я добавил несколько новых тестов. Планируешь ли ты скоро проверить PR?
1 лайк
Спасибо, Алексей — я постараюсь организовать проверку в течение следующей недели.
4 лайка
Я настроил форк virtualdom и могу подтвердить, что он решает проблему с iframe при ручном тестировании.
@Aleksey_Bogdanov, не мог бы ты, пожалуйста, помочь мне с одним вопросом? Я пытаюсь добавить тест в Discourse. Суть следующая:
-
Отрендерить
<span>ElementOne</span><span>ElementTwo</span> -
Удалить ElementTwo и добавить
PrependedElementв начало -
Убедиться, что результат —
<span>PrependedElement</span><span>ElementOne</span> -
Проверить, что исходный span с ElementOne равен финальному span с ElementOne (то есть убедиться, что он не был перерисован).
К сожалению, проверка в пункте (4) не проходит, что означает, что ElementOne был перерисован. Есть ли у тебя идеи, почему новая логика не работает в этом случае? 
Я запушил новую конфигурацию форка и падающий тест в существующий PR.
3 лайка
Да, я уже писал об этом ранее.
Я не ожидал, что это будет работать в общем случае (и что описанный вами тест пройдёт). Изначально это работало для добавлений в конец, удалений и одиночных вставок (возможно, некоторых их комбинаций). С моим изменением это также работает для чистых добавлений в начало, что должно устранить ошибку с YouTube. Ваш тест включает «удаление + добавление в начало». Это не покрывается моим изменением.
Более надёжное решение потребовало бы значительной переработки функций ‘reorder’ и ‘diffChildren’ в библиотеке virtual-dom, по крайней мере. Я могу попробовать это сделать, но возникает вопрос: планируете ли вы в долгосрочной перспективе поддерживать свой собственный форк virtual-dom? Если мы стремимся к надёжности, возможно, будет лучше и потребует меньше времени и усилий попробовать перейти на аналогичную активно поддерживаемую библиотеку. Думаю, другие библиотеки уже решили эту проблему.
2 лайка
Понятно — это имеет смысл. Однако, даже если я сделаю это «чистым добавлением в начало», я получаю ту же ошибку. Установка точки останова в соответствующей функции reorder() показывает, что она сбоит на проверке bFree.length === bChildren.length:
Я обновил тест и отправил его в ветку.
Это определённо лишь краткосрочное решение. Мы уже начали заменять использование vdom на рендеринг Ember/Glimmer. Наша цель — заменить реализацию потока сообщений в течение следующих 12 месяцев. Glimmer корректно обрабатывает такие случаи «добавления в начало».
Поэтому я совершенно не против слить «неполное, но лучшее, чем раньше» изменение. Но, если это не слишком много работы, было бы здорово понять, почему этот тест не работает
3 лайка
Конечно. Пока я точно не знаю, но как только смогу, обязательно разберусь.
3 лайка
Обновление.
Я добавил ключи, чтобы тест проходил. Ключи подсказывают virtual-dom, каким элементам соответствуют какие. Без них virtual-dom просто предполагает, что элементы следуют в том же порядке, что и раньше, и не может понять, что речь идёт о добавлении в начало.
Затем я изменил одно добавление в начало на несколько, так как virtual-dom уже обрабатывает одиночные вставки (а одиночное добавление в начало — это частный случай), поэтому они не должны вносить различий.
Вышеизложенные изменения — это то, что я пока загрузил.
Но теперь несколько добавлений в начало не вносят различий, что удивительно. Моя первоначальная теория о том, в чём проблема, и, следовательно, почему мой патч работает, может быть ошибочной. Поэтому я всё ещё пытаюсь разобраться.
Дэвид, я заметил, что вы отлаживали тест в браузере. Это сложно настроить? Мне трудно настроить отладчик для тестов, и я был бы признателен, если бы вы могли дать мне несколько советов.
3 лайка
Ага, понял — спасибо!
У тебя есть рабочая среда разработки Discourse? Если да, запусти её, перейди в браузере по адресу /tests, а затем используй интерфейс фильтра вверху, чтобы найти «avoids rerendering on prepend». После этого ты можешь использовать инструменты разработчика браузера для отладки. (Например, перейди на вкладку Sources, нажми Ctrl+P, чтобы открыть файл, найди vdom/diff и поставь точку останова).
3 лайка
Спасибо, я попробую.
3 лайка
Итак, этот код
assert.strictEqual(elementOneBefore, elementOneAfter);
не имеет значения, потому что elementOneBefore сохраняет свою идентичность при всех этих изменениях в любом случае.
Вот небольшая иллюстрация, если вы хотите увидеть это своими глазами:
<!DOCTYPE html>
<html>
<head>
<meta charset="utf-8">
<title>Демонстрация бага YouTube</title>
</head>
<body>
<script>
let iframe = document.createElement("iframe");
iframe.width = 690;
iframe.height = 388;
iframe.src = "https://www.youtube.com/embed/Xc5rB-0ZBcI";
iframe.title = "Странный глюк с машиной в S.T.A.L.K.E.R.";
document.body.appendChild(iframe);
let button = document.createElement("button");
button.type = "button";
button.innerHTML = "Вставить заново";
button.onclick = function(){
let iframeStart = document.querySelector("iframe");
document.body.removeChild(iframeStart);
document.body.insertBefore(iframeStart, button);
let iframeEnd = document.querySelector("iframe");
alert(iframeStart === iframeEnd);
};
document.body.appendChild(button);
</script>
</body>
</html>
Что действительно имеет значение, так это вызовы ‘removeChild’ и ‘insertBefore’, поэтому я добавил проверку мутаций DOM. Теперь тест падает для предыдущей версии и проходит для текущей, так что, надеюсь, этого достаточно.
6 лайков
Это отлично, большое спасибо за вашу работу здесь @Aleksey_Bogdanov. Я только что объединил PR — он станет доступен на Meta в течение следующего часа.
6 лайков