Gibt es etwas Falsches oder Schwieriges daran, das virtuelle DOM zu patchen, um dieses Problem zu beheben?
Ein PR ist willkommen, wir haben es versucht, aber es war einfach zu schwierig
2 „Gefällt mir“
Sehr interessant! Ein Fork von vdom ist nicht ideal, aber wir könnten ihn in Betracht ziehen, solange wir nachweisen können, dass es keine Regressionen gibt (und wir ihn in unsere eigene GitHub-Organisation verschieben).
Konnten Sie die vdom-Testsuite ausführen, um zu bestätigen, dass es keine Regressionen gibt? Wie einfach wäre es, einen neuen vdom-Test für dieses Prepend-Verhalten hinzuzufügen?
2 „Gefällt mir“
Ich werde zuerst die zweite Frage beantworten. Dieses Voranstellen von Elementen ist im Wesentlichen dasselbe, es eliminiert nur etwas zusätzliche Arbeit. Ich werde versuchen zu veranschaulichen, was vdom ursprünglich und nach dem Patch tut.
Angenommen, es gibt 20 Beiträge und 10 werden nach dem Scrollen oben geladen.
Ursprünglich führt vdom diese Transformationssequenz durch:
[20 alt] → [20 alt, 10 neu] // 10 neue Elemente werden am Ende angehängt
[20 alt, 10 neu] → // Alle 30 Elemente werden entfernt
→ [10 neu, 20 alt] // Alle 30 Elemente werden an ihrer Stelle eingefügt
Nach dem Patch wird die Sequenz zu:
[20 alt] → [20 alt, 10 neu] // 10 neue Elemente werden am Ende angehängt
[20 alt, 10 neu] → [20 alt] // 10 neue Elemente werden vom Ende entfernt
[20 alt] → [10 neu, 20 alt] // 10 neue Elemente werden vorangestellt
Wie Sie sehen können, gibt es immer noch zusätzliche Arbeit: Im Prinzip können Sie diese Elemente sofort voranstellen, aber auf diese Weise konnte ich einfach etwas Code an einer Stelle hinzufügen und mich nicht darum kümmern, das, was bereits vorhanden ist, neu zu schreiben.
Jede der Transformationen wird zu einer DOM-Operation, sodass YouTube-Videos zu spielen beginnen, da ursprünglich alte Elemente wieder eingefügt werden (was dazu führt, dass sie neu gerendert werden), und nach dem Patch bleiben sie an Ort und Stelle.
2 „Gefällt mir“
Hier ist ein Auszug aus der package.json von 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"
},
Von diesen scheinen ‘test’, ‘dot’, ‘cover’, ‘view-cover’, ‘browser’, ‘phantom’, ‘travis-test’ für Tests relevant zu sein.
‘browser’, ‘phantom’, ‘travis-test’ geben einen Parsing-Fehler aufgrund neuerer JavaScript-Konstrukte in meinem Code. Andere bestehen. Wenn ich diesen Code ändere
var prepend = simulate.every(item => item && item.key)
prepend &= aChildren.every((item, i) => item.key === bChildren[i + shift].key)
zu diesem 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
}
dann bestehen alle. Ich kann diese Änderung pushen, um JavaScript konsistent alt zu halten, wenn die Erfüllung all dieser Skripte wünschenswert ist.
2 „Gefällt mir“
Bitte tun Sie das – lassen Sie uns ihre bestehende Testsuite funktionsfähig halten.
Ich habe gerade virtual-dom in GitHub - discourse/virtual-dom: A Virtual DOM and diffing algorithm geforkt – können Sie bitte einen PR gegen dieses Repository erstellen?
3 „Gefällt mir“
3 „Gefällt mir“
Ich sollte auch anmerken, dass meine Änderung am virtuellen DOM sehr spezifisch ist. Sie zielt speziell auf Prependings ab und greift in allen anderen Fällen auf den ursprünglichen Algorithmus zurück. Und dieser ursprüngliche Algorithmus ist immer noch unvollkommen, wenn man das Problem allgemein betrachtet (es ist nicht schwer, Beispiele zu finden, bei denen unnötigerweise alte Elemente berührt werden). Andererseits behandelt er Appends, Removes und einzelne Einfügungen problemlos. Und damit muss man mit dem Post-Stream wirklich ausgefallen werden, um auszubrechen. Praktisch gesehen könnte die allgemeine Lösung des Problems ein wenig übertrieben sein, obwohl man natürlich besser schlafen kann, wenn man es tut.
Ich habe einige neue Tests hochgeladen. Planen Sie, den PR bald zu überprüfen?
1 „Gefällt mir“
Danke Aleksey – ich werde versuchen, es innerhalb der nächsten Woche überprüfen zu lassen
4 „Gefällt mir“
Ich habe den virtualdom-Fork eingerichtet und kann bestätigen, dass er das iframe-Problem bei manuellen Tests löst.
@Aleksey_Bogdanov Ich frage mich, ob du mir bei etwas helfen kannst. Ich habe versucht, einen Test innerhalb von Discourse hinzuzufügen. Im Wesentlichen:
-
Rendere
<span>ElementOne</span><span>ElementTwo</span> -
Entferne ElementTwo und füge
PrependedElementam Anfang ein -
Überprüfe, ob das Ergebnis
<span>PrependedElement</span><span>ElementOne</span>ist -
Überprüfe, ob das ursprüngliche ElementOne-Span dem endgültigen ElementOne-Span entspricht (d. h. überprüfe, ob es nicht neu gerendert wurde)
Leider schlägt die Überprüfung in (4) fehl, was bedeutet, dass ElementOne neu gerendert wurde. Hast du eine Idee, warum die neue Logik in diesem Fall nicht funktioniert? 
Ich habe die neue Fork-Konfiguration und den fehlschlagenden Test in den bestehenden PR gepusht.
3 „Gefällt mir“
Ja, ich habe vorher darüber geschrieben.
Ich würde nicht erwarten, dass dies allgemein funktioniert (und der von Ihnen beschriebene Test erfolgreich ist). Ursprünglich funktioniert es für Anhänge, Entfernungen, einzelne Einfügungen (vielleicht eine Kombination davon). Mit meiner Änderung funktioniert es auch für reine Voranstellen, was den YouTube-Bug beheben sollte. Ihr Test hat ‘Entfernung + Voranstellen’. Dies wird durch meine Änderung nicht abgedeckt.
Eine robustere Lösung würde zumindest eine signifikante Überarbeitung von ‘reorder’ und ‘diffChildren’ in virtual-dom erfordern. Das kann ich versuchen, aber es stellt sich die Frage, ob Sie langfristig eine eigene Fork von virtual-dom pflegen wollen. Wenn wir robust sein wollen, wäre es vielleicht eine bessere Idee und würde weniger Zeit und Mühe erfordern, zu einer vergleichbaren, aktiv gepflegten Bibliothek zu wechseln. Ich vermute, dass andere Bibliotheken dieses Problem inzwischen gelöst haben.
2 „Gefällt mir“
Ich verstehe – das ergibt Sinn. Aber selbst wenn ich es zu einem „reinen Voranstellen“ mache, erhalte ich denselben Fehler. Wenn ich einen Haltepunkt in der relevanten reorder()-Funktion setze, sehe ich, dass sie bei der Prüfung bFree.length === bChildren.length fehlschlägt:
Ich habe den aktualisierten Test auf den Branch gepusht.
Das ist definitiv nur eine kurzfristige Lösung. Wir haben bereits begonnen, unsere Verwendung von vdom durch Ember/Glimmer-Rendering zu ersetzen. Unser Ziel ist es, die Implementierung des Beitragsstroms innerhalb der nächsten 12 Monate zu ersetzen. Glimmer behandelt diese Art von „Voranstellung“ korrekt.
Daher bin ich absolut damit einverstanden, eine „unvollständige, aber bessere als zuvor“-Änderung zusammenzuführen. Aber wenn es nicht zu viel Arbeit ist, wäre es schön zu verstehen, warum dieser Test nicht funktioniert
3 „Gefällt mir“
Absolut. Ich bin mir noch nicht sicher, aber ich werde es untersuchen, sobald ich kann.
3 „Gefällt mir“
Aktualisierung.
Ich habe Schlüssel hinzugefügt, damit der Test erfolgreich ist. Schlüssel deuten auf virtual-dom hin, welche Elemente welchen entsprechen. Ohne sie geht virtual-dom davon aus, dass die Elemente die gleiche Reihenfolge wie zuvor haben und kann nicht erkennen, dass es sich um Prepend-Operationen handelt.
Dann habe ich einzelne Prepend-Operationen in mehrere Prepend-Operationen geändert, da virtual-dom bereits einzelne Einfügungen abgedeckt hat (und einzelne Prepend-Operationen ein Sonderfall sind), sodass sie keinen Unterschied machen sollten.
Die oben genannten Änderungen sind das, was ich bisher gepusht habe.
Aber jetzt machen mehrere Prepend-Operationen keinen Unterschied, was überraschend ist. Meine ursprüngliche Theorie, was falsch ist und warum mein Patch funktioniert, könnte fehlerhaft sein. Ich versuche also immer noch, es herauszufinden.
Ich stelle fest, David, dass du den Test im Browser debuggt hast. Ist er schwer einzurichten? Ich habe Schwierigkeiten, einen Debugger für Tests einzurichten und wäre dankbar, wenn du mir einige Hinweise geben könntest.
3 „Gefällt mir“
[quote=“Aleksey Bogdanov, post:59, topic:57692, username:Aleksey_Bogdanov”]
Ich habe Schlüssel hinzugefügt, damit die Tests bestanden werden. Schlüssel deuten auf virtual-dom hin, welche Elemente zu welchen gehören.
[/quote]Aha, ich verstehe – danke dafür!
Hast du eine funktionierende Discourse-Entwicklungsumgebung? Wenn ja, starte sie, besuche /tests im Browser und verwende dann die Filter-UI oben, um nach “avoids rerendering on prepend” zu suchen. Dann kannst du die Browser-Entwicklertools zum Debuggen verwenden. (z. B. gehe zum Tab “Quellen”, drücke Strg+P, um die Datei zu öffnen, suche nach vdom/diff und setze einen Haltepunkt)
3 „Gefällt mir“
Danke, ich werde es versuchen.
3 „Gefällt mir“
Also, dieser Code
assert.strictEqual(elementOneBefore, elementOneAfter);
macht keinen Unterschied, da elementOneBefore seine Identität auf jeden Fall durch alle diese Änderungen beibehält.
Hier ist eine kleine Veranschaulichung, wenn Sie es selbst sehen möchten:
<!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>
Was einen Unterschied macht, sind diese Aufrufe von ‘removeChild’, ‘insertBefore’, also habe ich eine Prüfung auf DOM-Mutationen hinzugefügt. Jetzt schlägt der Test für die vorherige Version fehl und besteht für die aktuelle, hoffentlich ist das also ausreichend.
6 „Gefällt mir“
Das ist großartig, vielen Dank für Ihre Arbeit hier @Aleksey_Bogdanov. Ich habe den PR gerade zusammengeführt – er wird in den nächsten Stunden auf Meta live sein.
6 „Gefällt mir“