Y a-t-il quelque chose de mal ou de difficile à patcher le virtual-dom pour résoudre ce problème ?
Une PR est la bienvenue, nous avons essayé mais c’était tout simplement trop difficile
2 « J'aime »
Très intéressant ! Passer à un fork de vdom n’est pas idéal, mais nous pourrions l’envisager à condition de prouver qu’il n’y a pas de régressions (et que nous le déplacions vers notre propre organisation GitHub).
Avez-vous pu exécuter la suite de tests vdom pour confirmer qu’il n’y a pas de régressions ? Serait-il facile d’ajouter un nouveau test vdom pour ce comportement de préfixe ?
2 « J'aime »
Je vais d’abord répondre à la deuxième question. Ce comportement de préfixation est essentiellement le même, il élimine simplement un travail supplémentaire. J’essaierai d’illustrer ce que fait le vdom à l’origine et après le correctif.
Supposons qu’il y ait 20 publications et que 10 soient chargées en haut après le défilement.
À l’origine, le vdom effectue cette séquence de transformations :
[20 anciens] → [20 anciens, 10 nouveaux] // 10 nouveaux éléments sont ajoutés à la fin
[20 anciens, 10 nouveaux] → // les 30 éléments sont supprimés
→ [10 nouveaux, 20 anciens] // les 30 éléments sont insérés à leur place
Après le correctif, la séquence devient :
[20 anciens] → [20 anciens, 10 nouveaux] // 10 nouveaux éléments sont ajoutés à la fin
[20 anciens, 10 nouveaux] → [20 anciens] // 10 nouveaux éléments sont supprimés de la fin
[20 anciens] → [10 nouveaux, 20 anciens] // 10 nouveaux éléments sont préfixés
Comme vous pouvez le constater, il y a toujours un travail supplémentaire : en principe, vous pouvez préfixer ces éléments immédiatement, mais de cette façon, je peux simplement ajouter du code à un endroit et ne pas avoir à réécrire ce qui est déjà là.
Chacune des transformations devient une opération DOM, de sorte que les vidéos YouTube commencent à être lues car à l’origine les anciens éléments sont réinsérés (ce qui déclenche leur rendu), et après le correctif, ils restent en place.
2 « J'aime »
Voici un extrait du fichier 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"
},
Parmi ceux-ci, ‘test’, ‘dot’, ‘cover’, ‘view-cover’, ‘browser’, ‘phantom’, ‘travis-test’ semblent pertinents pour les tests.
‘browser’, ‘phantom’, ‘travis-test’ génèrent une erreur d’analyse en raison de constructions JavaScript plus récentes dans mon code. Les autres réussissent. Si je change ce code
var prepend = simulate.every(item => item && item.key)
prepend &= aChildren.every((item, i) => item.key === bChildren[i + shift].key)
en ce code
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
}
alors tout passe. Je peux pousser ce changement pour maintenir la cohérence du JavaScript ancien si la satisfaction de tous ces scripts est souhaitable.
2 « J'aime »
s’il vous plaît faites-le - gardons leur suite de tests existante fonctionnelle
Je viens de forker virtual-dom dans GitHub - discourse/virtual-dom: A Virtual DOM and diffing algorithm - pourriez-vous s’il vous plaît faire une PR contre ce dépôt ?
3 « J'aime »
3 « J'aime »
Je dois également noter que ma modification du virtual-dom est très ciblée. Elle concerne spécifiquement les ajouts en début de liste et utilise l’algorithme d’origine dans tous les autres cas. Et cet algorithme d’origine reste imparfait si l’on considère le problème de manière générale (il n’est pas difficile de trouver des exemples où il toucherait inutilement d’anciens éléments). D’un autre côté, il gère correctement les ajouts en fin de liste, les suppressions et les insertions uniques. Et avec cela, vous devez être très sophistiqué avec le post-traitement pour vous démarquer. Donc, en pratique, résoudre le problème de manière générale pourrait être un peu excessif, bien que bien sûr vous puissiez mieux dormir lorsque vous le faites.
J’ai soumis de nouveaux tests. Prévoyez-vous de réviser la PR bientôt ?
1 « J'aime »
Merci Aleksey - J’essaierai de le faire examiner la semaine prochaine.
4 « J'aime »
J’ai configuré le fork de virtualdom et je peux confirmer qu’il résout le problème de l’iframe lors des tests manuels.
@Aleksey_Bogdanov Je me demandais si vous pouviez m’aider avec quelque chose cependant. J’ai essayé d’ajouter un test dans Discourse. Essentiellement :
-
Rendre
<span>ElementOne</span><span>ElementTwo</span> -
Supprimer ElementTwo et préfixer avec
PrependedElement -
Vérifier que le résultat est
<span>PrependedElement</span><span>ElementOne</span> -
Vérifier que le span original ElementOne est égal au span final ElementOne (c’est-à-dire vérifier qu’il n’a pas été re-rendu)
Malheureusement, la vérification en (4) échoue, ce qui signifie qu’ElementOne a été re-rendu. Avez-vous une idée pourquoi la nouvelle logique ne fonctionne pas dans ce cas ? 
J’ai poussé la nouvelle configuration du fork et le test échoué sur la PR existante.
3 « J'aime »
Oui, j’ai écrit à ce sujet plus tôt.
Je ne m’attendrais pas à ce que cela fonctionne en général (et que le test tel que vous le décrivez réussisse). À l’origine, cela fonctionne pour les ajouts, les suppressions, les insertions uniques (peut-être une combinaison de ceux-ci). Avec mon changement, cela fonctionne également pour les préfixes purs, ce qui devrait couvrir le bug de YouTube. Votre test a une « suppression + préfixe ». Ceci n’est pas couvert par mon changement.
Une solution plus robuste nécessiterait une réécriture significative de « reorder » et « diffChildren » dans virtual-dom au moins. Ce que je peux essayer, mais cela pose la question, allez-vous maintenir votre propre fork de virtual-dom à long terme ? Si nous essayons d’être robustes, il serait peut-être préférable et nécessiterait moins de temps et d’efforts d’essayer de passer à une bibliothèque analogue activement maintenue. Je suppose que d’autres bibliothèques ont déjà résolu ce problème à l’heure actuelle.
2 « J'aime »
Je vois - c’est logique. Cependant, même si j’en fais un « prepend pur », j’obtiens le même échec. Placer un point d’arrêt dans la fonction reorder() pertinente montre qu’elle bugue sur la vérification bFree.length === bChildren.length :
J’ai poussé le test mis à jour sur la branche.
C’est définitivement une solution à court terme. Nous avons déjà commencé à remplacer notre utilisation de vdom par le rendu Ember/Glimmer. Notre objectif est de remplacer l’implémentation du flux de messages dans les 12 prochains mois. Glimmer gère correctement ce type de « prepend ».
Donc, je suis tout à fait d’accord pour fusionner un changement « incomplet, mais meilleur qu’avant ». Mais, si ce n’est pas trop de travail, ce serait bien de comprendre pourquoi ce test ne fonctionne pas
3 « J'aime »
Absolument. Je ne suis pas encore sûr, mais je vais me pencher dessus dès que je pourrai.
3 « J'aime »
Mise à jour.
J’ai ajouté des clés pour que le test passe. Les clés indiquent au virtual-dom quels éléments correspondent à quels autres. Sans elles, le virtual-dom suppose simplement que les éléments suivent le même ordre qu’auparavant et ne peut pas comprendre qu’il s’agit d’ajouts en début de liste.
Ensuite, j’ai remplacé un ajout unique en début de liste par plusieurs ajouts en début de liste, car le virtual-dom gérait déjà les insertions uniques (et les ajouts uniques en début de liste sont un cas particulier), donc cela ne devrait pas faire de différence.
Les changements ci-dessus sont ce que j’ai poussé jusqu’à présent.
Mais maintenant, les ajouts multiples en début de liste ne font pas de différence, ce qui est surprenant. Ma théorie initiale sur ce qui ne va pas et, par extension, pourquoi mon correctif fonctionne, pourrait être erronée. J’essaie donc toujours de comprendre.
Je note, David, que vous avez débogué le test dans le navigateur. Est-il difficile à configurer ? J’ai du mal à configurer un débogueur pour les tests et j’apprécierais que vous puissiez me donner quelques conseils.
3 « J'aime »
Aha, je vois - merci pour ça !
Avez-vous une configuration de développement Discourse fonctionnelle ? Si oui, démarrez-la, visitez /tests dans le navigateur, puis utilisez l’interface de filtre en haut pour rechercher « avoids rerendering on prepend ». Vous pouvez ensuite utiliser les outils de développement du navigateur pour déboguer. (par exemple, allez dans l’onglet sources, ctrl+p pour ouvrir le fichier, recherchez vdom/diff, et définissez un point d’arrêt)
3 « J'aime »
Merci, je vais essayer.
3 « J'aime »
Donc, ce code
assert.strictEqual(elementOneBefore, elementOneAfter);
ne fait aucune différence car elementOneBefore conserve son identité dans tous ces changements, quelle que soit la méthode.
Voici une petite illustration, si vous souhaitez vérifier par vous-même :
<!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>
Ce qui fait une différence, ce sont ces appels removeChild, insertBefore, j’ai donc ajouté une vérification des mutations du DOM. Maintenant, le test échoue pour la version précédente et réussit pour la version actuelle, donc j’espère que cela suffira.
6 « J'aime »
C’est super, merci beaucoup pour votre travail ici @Aleksey_Bogdanov. Je viens de fusionner la PR - elle sera en ligne sur Meta dans l’heure qui suit.
6 « J'aime »