Strengthen isKeyed test for swap rows

[krausest]

As discovered in #693 currently the isKeyed test for swap rows is too weak to detect implementations that remove the two rows and simply insert two new rows.
I really think it makes much more sense to require a keyed implementation to actually move the rows, i.e. add exactly the rows that were removed (otherwise any dom state would be lost when swapping rows).

If isKeyed checks whether the rows are actually moved for swap rows the following implementations fail:

• aurelia-v1.3.0-keyed
• binding.scala-v10.0.1-keyed
• crui-v0.1.0-alpha.13-keyed
• datum-v0.12.2-keyed
• dojo-v6.0.4-keyed
• fidan-v0.0.23-keyed
• maquette-v3.3.0-keyed
• mikado-v0.7.57-keyed
• ractive-v1.3.6-keyed
• react-hookstate-v16.8.6 + 1.5.2-keyed
• reflex-dom-v0.4-keyed
• san-v3.7.7-keyed

Can you please take a look at your implementations? Is it possible to fix it for your frameworks?
@Alexander-Taran @Atry @iazel @MartinRixham @agubler @ismail-codar @johan-gorter @ts-thomas @alexfmpe @errorrik

[Atry]

I think it’s better to create a third category other than keyed and unkeyed instead of changing the definition Stefan Krause <notifications@github.com>于2020年2月14日 周五下午12:20写道：

…

As discovered in #693 <#693> currently the isKeyed test for swap rows is too weak to detect implementations that remove the two rows and simply insert two new rows. I really think it makes much more sense to require a keyed implementation to actually move the rows, i.e. add exactly the rows that were removed (otherwise any dom state would be lost when swapping rows). If isKeyed checks whether the rows are actually moved for swap rows the following implementations fail: aurelia-v1.3.0-keyed binding.scala-v10.0.1-keyed crui-v0.1.0-alpha.13-keyed datum-v0.12.2-keyed dojo-v6.0.4-keyed fidan-v0.0.23-keyed maquette-v3.3.0-keyed mikado-v0.7.57-keyed ractive-v1.3.6-keyed react-hookstate-v16.8.6 + 1.5.2-keyed reflex-dom-v0.4-keyed san-v3.7.7-keyed Can you please take a look at your implementations? Is it possible to fix it for your frameworks? @Alexander-Taran <https://github.com/Alexander-Taran> @Atry <https://github.com/Atry> @iazel <https://github.com/Iazel> @MartinRixham <https://github.com/MartinRixham> @agubler <https://github.com/agubler> @ismail-codar <https://github.com/ismail-codar> @johan-gorter <https://github.com/johan-gorter> @ts-thomas <https://github.com/ts-thomas> @evs-chris <https://github.com/evs-chris> @alexfmpe <https://github.com/alexfmpe> @errorrik <https://github.com/errorrik> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#694?email_source=notifications&email_token=AAES3OSSZNJEDQGHZXGP6A3RC34JZA5CNFSM4KVPEJSKYY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4INVNPLQ>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAES3OQPU6I57ECUPVR2JRLRC34JZANCNFSM4KVPEJSA> .

[leeoniya]

the name of the metric has always been "swap", unambiguously. this simply improves the assertion of the metric.

[krausest]

@Atry I don't think we're changing the definition of keyed, but indeed we're changing the test for keyed that wasn't sufficient (and I'm not sure it's now watertight).

[johan-gorter]

Maquette does not support swapping nodes, you can exclude maquette from this test for this reason.

[krausest]

I decided to update the results table such that it show any open issues for the implementations. The information is kept in package.json (js-framework-benchmark.issues).

[ts-thomas]

Hello @krausest, the mikado keyed implementation was tagged with the issue "Keyed implementations must move the DOM nodes for swap rows", but that's not right. The implementation must assign the new values to the data array (like all others).

.route("swaprows", () => {
    const tmp = store[998];
    store[998] = store[1];
    store[1] = tmp;
})

Edit: Sorry but I misunderstood the issue, I thought that the description explains the issue not the desired behavior.

[krausest]

@ts-thomas Please try the following: Click on "Create 1,000 rows", open chrome's dev-tools and set the background color of the row with id 2 to red and click on "Swap rows". If you look now at the second to last row (which has now id 2) is it still red? If not it's not keyed.