Há algo errado ou difícil em aplicar um patch no virtual-dom para corrigir este problema?
Uma PR é bem-vinda, tentamos, mas foi simplesmente muito difícil
2 curtidas
Muito interessante! Mover para um fork do vdom não é o ideal, mas podemos considerar isso, desde que possamos provar que não há regressões (e o movamos para nossa própria organização GitHub).
Você conseguiu executar a suíte de testes do vdom para confirmar que não há regressões? Quão fácil seria adicionar um novo teste de vdom para esse comportamento de prepend?
2 curtidas
Responderei primeiro à segunda pergunta. Esse comportamento de prepend é essencialmente o mesmo, apenas elimina um trabalho extra. Tentarei ilustrar o que o vdom faz originalmente e após o patch.
Suponha que haja 20 posts e 10 estão sendo carregados no topo após a rolagem.
Originalmente, o vdom faz esta sequência de transformações:
[20 antigos] → [20 antigos, 10 novos] // 10 novos elementos são anexados ao final
[20 antigos, 10 novos] → // todos os 30 elementos são removidos
→ [10 novos, 20 antigos] // todos os 30 elementos são inseridos em seu lugar
Após o patch, a sequência se torna:
[20 antigos] → [20 antigos, 10 novos] // 10 novos elementos são anexados ao final
[20 antigos, 10 novos] → [20 antigos] // 10 novos elementos são removidos do final
[20 antigos] → [10 novos, 20 antigos] // 10 novos elementos são prepended
Como você pode ver, ainda há trabalho extra: em princípio, você pode prepend esses elementos imediatamente, mas desta forma eu poderia apenas adicionar algum código em um lugar e não me preocupar em reescrever o que já está lá.
Cada uma das transformações se torna uma operação no DOM, então os vídeos do YouTube começam a tocar porque originalmente os elementos antigos são reinseridos (o que os faz serem renderizados novamente), e após o patch eles permanecem no lugar.
2 curtidas
Aqui está um trecho do package.json do 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"
},
Destes, ‘test’, ‘dot’, ‘cover’, ‘view-cover’, ‘browser’, ‘phantom’, ‘travis-test’ parecem relevantes para testes.
‘browser’, ‘phantom’, ‘travis-test’ dão um erro de análise devido a construções mais recentes de javascript no meu código. Outros passam. Se eu mudar este código
var prepend = simulate.every(item => item && item.key)
prepend &= aChildren.every((item, i) => item.key === bChildren[i + shift].key)
para 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
}
então todos passam. Posso enviar essa alteração para manter o javascript consistentemente antigo se satisfazer todos esses scripts for desejável.
2 curtidas
por favor, faça - vamos manter a suíte de testes existente deles funcionando
Eu acabei de fazer um fork do virtual-dom para GitHub - discourse/virtual-dom: A Virtual DOM and diffing algorithm - você poderia, por favor, fazer um PR contra esse repositório?
3 curtidas
3 curtidas
Também devo observar que minha alteração no virtual-dom é muito específica. Ela visa especificamente as inserções iniciais (prepends) e recorre ao algoritmo original em todos os outros casos. E esse algoritmo original ainda é imperfeito se você olhar para o problema de forma geral (não é difícil criar exemplos em que ele tocaria em elementos antigos desnecessariamente). Por outro lado, ele lida bem com inserções finais (appends), remoções e inserções únicas. E com isso, você precisa ser muito engenhoso com o fluxo posterior (post stream) para se destacar. Portanto, na prática, resolver o problema de forma geral pode ser um exagero, embora, é claro, você possa dormir melhor quando o faz.
Enviei alguns novos testes. Você pretende revisar o PR em breve?
1 curtida
Obrigado Aleksey - tentarei fazer a revisão na próxima semana
4 curtidas
Tenho o fork do virtualdom configurado e posso confirmar que ele resolve o problema do iframe em testes manuais.
@Aleksey_Bogdanov Gostaria de saber se você pode me ajudar com algo. Tenho tentado adicionar um teste dentro do Discourse. Essencialmente:
-
Renderize
<span>ElementOne</span><span>ElementTwo</span> -
Remova ElementTwo e adicione
PrependedElementno início -
Verifique se o resultado é
<span>PrependedElement</span><span>ElementOne</span> -
Verifique se o span original ElementOne é igual ao span final ElementOne (ou seja, verifique se ele não foi renderizado novamente)
Infelizmente, a verificação em (4) falha, o que significa que ElementOne foi renderizado novamente. Alguma ideia de por que a nova lógica não está funcionando neste caso? 
Enviei a nova configuração do fork e o teste que falha para o PR existente.
3 curtidas
Sim, escrevi sobre isso anteriormente.
Eu não esperaria que isso funcionasse em geral (e o teste como você descreve para passar). Originalmente, funciona para anexos, remoções, inserções únicas (talvez alguma combinação delas). Com minha alteração, também funciona para prepend puro, o que deve cobrir o bug do youtube. Seu teste tem ‘remoção + prepend’. Isso não é coberto pela minha alteração.
Uma solução mais robusta exigiria uma reescrita significativa de ‘reorder’ e ‘diffChildren’ em virtual-dom, pelo menos. O que eu posso tentar, mas isso levanta a questão, você planeja manter seu próprio fork de virtual-dom a longo prazo? Se estamos tentando ser robustos, pode ser uma ideia melhor e exigir menos tempo e esforço tentar mudar para uma biblioteca análoga ativamente mantida. Minha suposição é que outras bibliotecas já resolveram esse problema até agora.
2 curtidas
Entendo - isso faz sentido. No entanto, mesmo que eu o torne um ‘prepend puro’, obtenho a mesma falha. Colocar um ponto de interrupção na função reorder() relevante mostra que ela falha na verificação bFree.length === bChildren.length:
Eu enviei o teste atualizado para o branch.
Esta é definitivamente apenas uma solução de curto prazo. Já começamos a substituir nosso uso de vdom pela renderização Ember/Glimmer. Nosso objetivo é substituir a implementação do stream de posts nos próximos 12 meses. Glimmer lida com esse tipo de ‘prepend’ corretamente.
Portanto, estou totalmente feliz em mesclar uma alteração ‘incompleta, mas melhor do que antes’. Mas, se não for muito trabalho, seria bom entender por que este teste não está funcionando
3 curtidas
Com certeza. Ainda não tenho certeza, mas vou investigar assim que puder.
3 curtidas
Atualização.
Adicionei chaves para que o teste passe. As chaves indicam quais elementos do virtual-dom correspondem a quais. Sem elas, o virtual-dom apenas assume que os elementos seguem a mesma ordem de antes e não consegue entender que está lidando com prepends.
Em seguida, alterei um único prepend para múltiplos prepends, pois o virtual-dom já cobria inserções únicas (e prepends únicos são um caso especial), então eles não deveriam fazer diferença.
As mudanças acima são o que eu enviei até agora.
Mas agora múltiplos prepends não fazem diferença, o que é surpreendente. Minha teoria inicial sobre o que está errado e, por extensão, por que meu patch funciona, pode estar falha. Então, ainda estou tentando descobrir.
Observo, David, que você depurou o teste no navegador. É difícil de configurar? Estou tendo dificuldades para configurar um depurador para testes e agradeceria se você pudesse me dar algumas dicas.
3 curtidas
Aha, entendi - obrigado por isso!
Você tem uma configuração de desenvolvimento do Discourse funcionando? Se sim, inicie-a, visite /tests no navegador e, em seguida, use a interface de filtro na parte superior para pesquisar por “avoids rerendering on prepend”. Em seguida, você pode usar as ferramentas de desenvolvedor do navegador para depurar. (por exemplo, vá para a aba de fontes, ctrl+p para abrir o arquivo, pesquise por vdom/diff e defina um ponto de interrupção)
3 curtidas
Obrigado, vou tentar.
3 curtidas
Portanto, este código
assert.strictEqual(elementOneBefore, elementOneAfter);
não faz diferença porque elementOneBefore mantém sua identidade em todas essas alterações de qualquer maneira.
Aqui está uma pequena ilustração, se você quiser ver por si mesmo:
<!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>
O que faz diferença são essas chamadas de ‘removeChild’, ‘insertBefore’, então adicionei uma verificação para mutações de DOM. Agora o teste falha para a versão anterior e passa para a atual, então espero que isso seja suficiente.
6 curtidas
Ótimo, muito obrigado pelo seu trabalho aqui @Aleksey_Bogdanov. Acabei de mesclar o PR - ele estará ativo no Meta dentro da próxima hora.
6 curtidas