Add uhydro keyed

[Krutsch]

Hello again 😁

Kind of feel unsettled submitting a second library to the benchmark suite, but here we are.
(I promise it is the last)

EDIT: Oops-a-daisy, the swap is not keyed, brb to fix ✅

[krausest]

Hi @Krutsch,
the isKeyed test thinks this implementation isn't keyed for swap rows:

npm run isKeyed -- --headless keyed/uhydro

> [email protected] isKeyed /build/webdriver-ts
> cross-env LANG="en_US.UTF-8" node dist/isKeyed.js "--headless" "keyed/uhydro"

args.framework undefined true
http://localhost:8080/frameworks/keyed/uhydro/package-lock.json
Frameworks that will be checked uhydro-v1.0.5-keyed
Keyed test for swap failed. Expected at least 1 added and 1 removed TR, but there were 0 added and 0 removed
uhydro-v1.0.5-keyed is keyed for 'run benchmark' and keyed for 'remove row benchmark' and non-keyed for 'swap rows benchmark' . It'll appear as non-keyed in the results
ERROR: Framework uhydro-v1.0.5-keyed is not correctly categorized

Can you please check uhydro's behaviour for swap rows?

[Krutsch]

@krausest It is definitely keyed. However, it is not moving the complete row, but only the elements inside, that have some state bound to them – which is the first td and the first a element. Removing (and other operations) will still work after swap because I am making use of the elements closure.

I can see that your test suite is expecting the row to move (which is not necessary in my case). If you would agree that this is fair, then I could update this test to check whether the elements inside moved?

[krausest]

I'm not sure whether others will share this view. From looking at the code the swap operation looks rather imperative (at least close to #772) by explicitly telling uhydro what data update to perform instead of relying on a comparison of the rows keys.

[Krutsch]

I don't quite follow. This is the written user code:

const d = getValue(data);
swap(d[1], d[998]); // lib call

What happens under the hood is that I check my own data structures, get all the nodes that map to the data and do the necessary operations for the elements. This is not tailored for this test 🤔.

EDIT: Pinging @ryansolid to get another opinion whether moving partial elements inside the tr is fair.

[ryansolid]

The implementation on the surface looks fine codewise. The most suspect piece is swap. Which I'm gathering is a reconciler of sorts. I haven't looked at the code yet but it feels might specific and this was a discussion we had with Mikado as well. Since I mean the Swap test is swapping rows but it's really in the spirit of any kind of move. Would uHydro have a specific method for different types of sorting? Most implementations swap row could be replaced with literally any type of sort and still work.

But maybe this is all beside the point. The tr is bound to state. There is a class on it that is derived from the item.id and the selection state. So I don't see how it could be keyed without moving the whole row. That being said the idea that there can be static nodes between the iterated row and the stateful parts is a novel idea and I think it would be interesting optimization that probably wouldn't break things. But I don't think that applies to this benchmark as each top level row element immediately has bound state.

[krausest]

Right: If you select the second row and then swap the rows, the selection background stays on the row in the second position, which is incorrect.

[Krutsch]

OK, This is now using a normal swap operation. Also, it will swap the tr instead of the sub elements only. Thanks for the
heads-up 👍🏻

[krausest]

WIth npm ci I'm getting an error. Builds fine with npm install though (I'll go with that):

npm ci && npm run build-prod
npm ERR! fsevents not accessible from chokidar

npm ERR! A complete log of this run can be found in:
npm ERR!     /root/.npm/_logs/2021-06-19T09_45_12_489Z-debug.log

[krausest]

Thanks! Results are updated.