Paused/finished YouTube videos automatically start playing when scrolling up far enough to load new posts

Is there something wrong or hard with patching virtual-dom to fix this problem?

A PR is welcome, we tried but it was simply too hard

1 Like

Very interesting! Moving to a fork of vdom isn’t ideal, but we might be able to consider it as long as we can prove there are no regressions (and we move it to our own GitHub organisation).

Were you able to run the vdom test suite to confirm there are no regressions? How easy would be it be to add a new vdom test for this prepend behaviour?

2 Likes

I’ll first address the second question. This prepend behavior is essentially the same, it just eliminates some extra work. I’ll try to illustrate what vdom does originally and after the patch.

Suppose there are 20 posts and 10 are being loaded on top after scroll.
Originally vdom makes this sequence of transformations:
[20 old] → [20 old, 10 new] // 10 new elements are appended to the end
[20 old, 10 new] → // all 30 elements are removed
→ [10 new, 20 old] // all 30 elements are inserted into their place
After the patch the sequence becomes:
[20 old] → [20 old, 10 new] // 10 new elements are appended to the end
[20 old, 10 new] → [20 old] // 10 new elements are removed from the end
[20 old] → [10 new, 20 old] // 10 new elements are prepended
As you can see there’s still extra work: in principle you can prepend those elements right away, but this way I could just add some code in one place and not bother about rewriting what’s already there.
Each of the transformations becomes a dom operation, so youtube videos start playing because originally old elements are reinserted (which triggers them being rerendered), and after the patch they stay in place.

2 Likes

Here is an excerpt from virtual-dom’s package.json

"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"
  },

Of these ‘test’, ‘dot’, ‘cover’, ‘view-cover’, ‘browser’, ‘phantom’, ‘travis-test’ seem relevant to testing.
‘browser’, ‘phantom’, ‘travis-test’ give a parsing error due to newer javascript constructs in my code. Others pass. If I change this code

        var prepend = simulate.every(item => item && item.key)
        prepend &&= aChildren.every((item, i) => item.key === bChildren[i + shift].key)

to this 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
        }

then all pass. I can push this change to keep javascript consistently old if satisfying all these scripts is desirable.

1 Like

:+1: please do - let’s keep their existing test suite working

I just forked virtual-dom into GitHub - discourse/virtual-dom: A Virtual DOM and diffing algorithm - please can you make a PR against that repo?

1 Like
2 Likes

I should also note that my change to virtual-dom is very narrow. It targets prepends specifically and falls back on the original algorithm in all other cases. And that original algorithm is still imperfect if you look at the problem generally (it’s not hard to come up with examples, when it would touch old elements unnecessarily). On the other hand it handles appends, removes, single inserts just fine. And with this you need to get real fancy with the post stream to break out. So practically speaking solving the problem generally might be a bit of an overkill, though of course you can sleep better when you do.