¿Hay algo malo o difícil en parchear el virtual-dom para solucionar este problema?
Se agradece una PR, lo intentamos pero fue simplemente demasiado difícil
2 Me gusta
¡Muy interesante! Moverse a una bifurcación de vdom no es ideal, pero podríamos considerarlo siempre que podamos demostrar que no hay regresiones (y lo traslademos a nuestra propia organización de GitHub).
¿Pudiste ejecutar la suite de pruebas de vdom para confirmar que no hay regresiones? ¿Qué tan fácil sería agregar una nueva prueba de vdom para este comportamiento de prefijo?
2 Me gusta
Primero abordaré la segunda pregunta. Este comportamiento de prefijo es esencialmente el mismo, simplemente elimina algo de trabajo adicional. Intentaré ilustrar lo que hace vdom originalmente y después del parche.
Supongamos que hay 20 publicaciones y se están cargando 10 más arriba después de desplazarse.
Originalmente, vdom realiza esta secuencia de transformaciones:
[20 viejos] → [20 viejos, 10 nuevos] // Se agregan 10 elementos nuevos al final
[20 viejos, 10 nuevos] → // Se eliminan los 30 elementos
→ [10 nuevos, 20 viejos] // Se insertan los 30 elementos en su lugar
Después del parche, la secuencia se convierte en:
[20 viejos] → [20 viejos, 10 nuevos] // Se agregan 10 elementos nuevos al final
[20 viejos, 10 nuevos] → [20 viejos] // Se eliminan 10 elementos nuevos del final
[20 viejos] → [10 nuevos, 20 viejos] // Se anteponen 10 elementos nuevos
Como puede ver, todavía hay trabajo adicional: en principio, puede anteponer esos elementos de inmediato, pero de esta manera podría agregar algo de código en un lugar y no molestarme en reescribir lo que ya está allí.
Cada una de las transformaciones se convierte en una operación del DOM, por lo que los videos de YouTube comienzan a reproducirse porque originalmente se vuelven a insertar elementos viejos (lo que provoca que se vuelvan a renderizar), y después del parche permanecen en su lugar.
2 Me gusta
Aquí hay un extracto del package.json de 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"
},
De estos, ‘test’, ‘dot’, ‘cover’, ‘view-cover’, ‘browser’, ‘phantom’, ‘travis-test’ parecen relevantes para las pruebas.
‘browser’, ‘phantom’, ‘travis-test’ dan un error de análisis debido a construcciones de javascript más nuevas en mi código. Los otros pasan. Si cambio este código
var prepend = simulate.every(item => item && item.key)
prepend &= aChildren.every((item, i) => item.key === bChildren[i + shift].key)
a este código
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
}
entonces todos pasan. Puedo enviar este cambio para mantener javascript consistentemente antiguo si es deseable satisfacer todos estos scripts.
2 Me gusta
por favor hazlo - mantengamos funcionando su suite de pruebas existente
Acabo de hacer un fork de virtual-dom en GitHub - discourse/virtual-dom: A Virtual DOM and diffing algorithm - ¿puedes hacer un PR contra ese repositorio?
3 Me gusta
3 Me gusta
También debo señalar que mi cambio a virtual-dom es muy específico. Se dirige específicamente a las inserciones iniciales (prepends) y recurre al algoritmo original en todos los demás casos. Y ese algoritmo original todavía es imperfecto si se considera el problema en general (no es difícil encontrar ejemplos en los que tocaría elementos antiguos innecesariamente). Por otro lado, maneja las inserciones finales (appends), eliminaciones e inserciones únicas sin problemas. Y con esto, necesitas ser muy sofisticado con el flujo posterior para salirte de lo común. Así que, en la práctica, resolver el problema en general podría ser exagerado, aunque, por supuesto, puedes dormir mejor cuando lo haces.
He subido algunas pruebas nuevas. ¿Planeas revisar la PR pronto?
1 me gusta
Gracias Aleksey. Intentaré revisarlo la próxima semana.
4 Me gusta
Tengo configurado el fork de virtualdom y puedo confirmar que resuelve el problema del iframe en pruebas manuales.
@Aleksey_Bogdanov Me pregunto si puedes ayudarme con algo. He estado intentando agregar una prueba dentro de Discourse. Esencialmente:
-
Renderiza
<span>ElementOne</span><span>ElementTwo</span> -
Elimina ElementTwo y antepón
PrependedElement -
Comprueba que el resultado sea
<span>PrependedElement</span><span>ElementOne</span> -
Comprueba que el span original de ElementOne sea igual al span final de ElementOne (es decir, comprueba que no se haya vuelto a renderizar)
Desafortunadamente, la comprobación en (4) falla, lo que significa que ElementOne se volvió a renderizar. ¿Alguna idea de por qué la nueva lógica no funciona en este caso? 
He subido la nueva configuración del fork y la prueba fallida a la PR existente.
3 Me gusta
Sí, escribí sobre esto anteriormente.
No esperaría que esto funcionara en general (y que la prueba, tal como la describes, pasara). Originalmente, funciona para agregados, eliminaciones, inserciones únicas (quizás alguna combinación de ellas). Con mi cambio, también funciona para prepended puros, lo que debería cubrir el error de YouTube. Tu prueba tiene ‘eliminación + prepend’. Esto no está cubierto por mi cambio.
Una solución más robusta requeriría una reescritura significativa de ‘reorder’ y ‘diffChildren’ en virtual-dom al menos. Lo cual puedo intentar, pero surge la pregunta: ¿planeas mantener tu propio fork de virtual-dom a largo plazo? Si estamos tratando de ser robustos, podría ser una mejor idea y requerir menos tiempo y esfuerzo intentar cambiar a una biblioteca análoga mantenida activamente. Supongo que otras bibliotecas ya resolvieron este problema.
2 Me gusta
Ya veo, tiene sentido. Sin embargo, incluso si lo convierto en una ‘preposición pura’, obtengo el mismo error. Al colocar un punto de interrupción en la función reorder() relevante, se muestra que falla en la verificación bFree.length === bChildren.length:
He subido la prueba actualizada a la rama.
Esta es definitivamente solo una solución a corto plazo. Ya hemos comenzado a reemplazar nuestro uso de vdom con la renderización de Ember/Glimmer. Nuestro objetivo es reemplazar la implementación del flujo de publicaciones en los próximos 12 meses. Glimmer maneja este tipo de ‘preposición’ correctamente.
Por lo tanto, estoy totalmente dispuesto a fusionar un cambio ‘incompleto, pero mejor que antes’. Pero, si no es demasiado trabajo, sería bueno entender por qué esta prueba no funciona
3 Me gusta
Absolutamente. Aún no estoy seguro, pero lo investigaré tan pronto como pueda.
3 Me gusta
Actualización.
Añadí claves para que la prueba pase. Las claves indican qué elementos corresponden a cuáles en virtual-dom. Sin ellas, virtual-dom simplemente asume que los elementos siguen el mismo orden que antes y no puede darse cuenta de que está tratando con inserciones al principio.
Luego cambié la inserción única por inserciones múltiples porque virtual-dom ya cubría las inserciones únicas (y las inserciones únicas al principio son un caso especial), por lo que no deberían marcar la diferencia.
Los cambios anteriores son los que he subido hasta ahora.
Pero ahora las inserciones múltiples al principio no marcan la diferencia, lo cual es sorprendente. Mi teoría inicial de lo que está mal y, por extensión, por qué mi parche funciona, podría ser errónea. Así que todavía estoy intentando descifrarlo.
Observo, David, que depuraste la prueba en el navegador. ¿Es difícil de configurar? Me está costando configurar un depurador para las pruebas y agradecería si pudieras darme algunas indicaciones.
3 Me gusta
¡Ajá, ya veo! ¡Gracias por eso!
¿Tienes una configuración de desarrollo de Discourse que funcione? Si es así, iníciala, visita /tests en el navegador y luego usa la interfaz de filtro en la parte superior para buscar “avoids rerendering on prepend”. Luego puedes usar las herramientas de desarrollador del navegador para depurar. (por ejemplo, ve a la pestaña de fuentes, Ctrl+P para abrir el archivo, busca vdom/diff y establece un punto de interrupción)
3 Me gusta
Gracias, lo intentaré.
3 Me gusta
Entonces, este código
assert.strictEqual(elementOneBefore, elementOneAfter);
no hace ninguna diferencia porque elementOneBefore mantiene su identidad a través de todos estos cambios de cualquier manera.
Aquí tienes una pequeña ilustración, si quieres verlo por ti mismo:
<!DOCTYPE html>
<html>
<head>
<meta charset="utf-8">
<title>Youtube Bug Demo</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 = "Strange S.T.A.L.K.E.R. car glitch";
document.body.appendChild(iframe);
let button = document.createElement("button");
button.type = "button";
button.innerHTML = "Reinsert";
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>
Lo que sí marca la diferencia son esas llamadas a ‘removeChild’, ‘insertBefore’, así que añadí una comprobación de mutaciones del DOM. Ahora la prueba falla para la versión anterior y pasa para la actual, así que espero que esto sea suficiente.
6 Me gusta
Genial, muchas gracias por tu trabajo aquí @Aleksey_Bogdanov. Acabo de fusionar la PR; estará activa en Meta en la próxima hora.
6 Me gusta