added react-hookstate and preact-hookstate frameworks

[avkonst]

Please merge these two frameworks. Hookstate is a competitor of Redux, Mobx. And it does really well in the performance benchmark. Target per single row updates are extremely fast with Hookstate in particular. Performance results from my experiments are discussed here: https://github.com/avkonst/hookstate/issues/14

[krausest]

Looks very interesting. I‘ll try to merge the next few days. Did you check whether your implementation is actually keyed (npm run isKeyed in webdriver-ts)? I’m asking because your link says swap rows is faster than vanillajs which would be cool, but often turns out to be a non-keyed update...

[avkonst]

I have cloned react-hooks from keyed one. Hookstate.useStateLink is a super-charged React.useState. I have done as close as possible handling of dispatch events as react-hooks and react-redux-hooks do. Hookstate does smart tracking of used/rendered states and rerenders only the affected areas. So, I do not know why react-hookstate would be categorised differently to react-hooks, for example. Here is what I see in the output of npm run isKeyed -- --framework hookstate:

react-hookstate-v16.8.6 + 1.5.2-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

Why does it fail keyed test for swap rows?

[avkonst]

I have found using the check tool that it expects 1TR to be added and 1TR to be removed. It is strange it does not accept in place updates. I have found a workaround for this, checking performance now.

[avkonst]

It passes the check when I disable state tracking in Hookstate. It forces the TR to be replaced everytime it's state is changed. Why do you expect for swap rows to replace the whole TR but not it's text content and danger class?

[avkonst]

OK. I have found the difference between keyed vs non-keyed in react frameworks. I will have a look if I can do both for Hookstate.

[avkonst]

Ok. fixed now check for isKeyed. It turned out I needed to add key property for tr tag. When id of the key property changes, React replaces this component, hence it satisfies the check. This slowed down some benchmarks, but still looking great for hookstate, in particular it is 10x times faster than redux on per row updates. I have also added non-keyed variant of the react-hookstate.

[avkonst]

Copy from the original discussion https://github.com/avkonst/hookstate/issues/14#issuecomment-585549234:

keyed/react-hookstate does effectively the following: 'delete TR at 1st and 998th position' and 'insert new TR at 1st and 998th position'.

non-keyed/react-hookstate does the following: 'set TD text for the label and id and TR class for 'selected' for the 1st row copying state from the 998th' and vice versa.

[avkonst]

swap rows confirmed incredibly fast performance of Hookstate on per field update originally demonstrated here: https://hookstate.netlify.com/performance-demo-large-table

[avkonst]

there was also a success story where Hookstate helped with very frequent 'per field' updates in the large array data set. Read more here: https://praisethemoon.org/hookstate-how-one-small-react-library-saved-moonpiano/ there is a video at the end demonstrating fast rerendering performance of React + Hookstate.

[avkonst]

Here is the latest results from swap rows benchmark:

[avkonst]

The difference between keyed/react-hookstate and keyed/react (or keyed/react-hooks) is because, the first rerenders only 2 rows (calling Row component only, which replaces the TR tag entirely). The second rerenders the whole table (calling Rows component), although taking 998 rows from memo/cache while looping.

[ryansolid]

That is interesting. @krausest @leeoniya @localvoid might find this of interest.

1. Your select row is fast because you are pushing selection state on to the items. While I don't believe it is strictly forbidden (a few libraries do it) it isn't equivalent to the other React implementations. They purposefully do it the other way since its what is being tested.. ie not to muddle the model state. The way this is currently implemented tests the same thing as the partial update test rather than something different. I'd consider changing this if you are interested in doing an equivalent comparison.

2. Swap Row is interesting. The other React libraries grab all Rows from cache. Technically they move the 2 DOM elements but Row Component is re-run 0 times since it is unchanged and memoized. By changing the list it just runs the diff and DOM reconciler. Your implementation re-creates the row without triggering the reconciler as the map function doesn't trigger. So it avoids React's naive reconciliation making it basically the cost of creating and attaching a few DOM nodes. It's a cool trick, but in no way intended. It basically bypasses the test. But points for creativity, I haven't seen anyone take this approach with React before.

[avkonst]

If I drop hookstate scoped state (point 1 and 2), it will make it the "equivalent ' to react hooks and will not justify why hookstate exists. I do not want another redux, I want high performance rerendering followed by state changes. If it is not allowed I do not see the point in benchmarking various libs as they are forced to hit react overhead.

[avkonst]

I would not restrict selected to be a part of an item. Redux and other libs could use the same approach but it would not improve anything for them as they will go through the map rows anyway

[avkonst]

2. creating and attaching a few DOM nodes. It's a cool trick, but in no way intended

Well vanilla js does exactly this, why can not I do the same in my framework?

[ryansolid]

React MobX and React Easy State could do the same thing and benefit. Basically any of the reactive React libs have this ability as well.

1 has come up a number of times before so its a known thing. To be fair select row is the most abused test in this benchmark so it's not surprising. In some cases, it's just how the library works. Sinuous's template API which is vital to its performance can't be done any other way but assigning to the item. And there are many other more questionable approaches there. I get how this is basically the whole point of the library to delegate changes downward so it can essentially forceUpdate in the listening components instead of doing some top-down immutable thing. So I can see the argument for this being fair game. At a certain point, many libraries might just go this way since for reactive libraries and even some not this is just the more performant way to go. I feel it goes against the spirit of that test, but it's a lesser evil than some of the other solutions here.

2 is abusing a trick. If you were writing the rendering library (like React) you wouldn't consider this approach as it is bypassing something you know your library has to do anyway. Like destroying and creating nodes every time you move something might not be acceptable. If you say had to sort a list alphabetically you might end up being slower because you needed to recreate the whole list. Or maybe you are using WebComponents or DOM nodes that contain state. It sort of abuses the Keyed mentality.

Well vanilla js does exactly this, why can not I do the same in my framework?

Vanilla doesn't destroy the nodes. It reuses them. Ie it moves the same nodes. A way to avoid reconciliation here while keeping node references is if the render was essentially a proxy (see Mikado). However, that has similar issues when the reordering is more complex than a swap having terrible scalability without the ability to batch and apply several transformations. Some libraries like Crui are looking into ways to do this in a more scalable way but they don't have 100% coverage and can only optimize for small set of known transformations currently.

Vanilla also holds a DOM reference to the selected TR to do selection but any data-driven implementor would recognize that basically breaks the mental model(ie.. it can not be re-rendered purely from state at any point). So the Vanilla does it why can't I is a slippery position. Don't get me wrong I think that ultimately people will take the fastest methods so all of these rules and posturings are pointless. However, there is value in how a library is presented for comparison beyond the performance.

[avkonst]

Sorry, I do not understand the point about abusing the trick. The test is to swap two rows in the state and expect correct rendering. If it was different requirement like sorting, it would trigger whole list rerender by hookstate correctly, but there was no a requirement to sort items.

[avkonst]

Moving selected marker out of the item state is doable, if you say it is not part of a state. I can just keep selected id in a separate state variable. I can do it without any performance impact. But do not see why the approach I taken is wrong.

[avkonst]

Btw, this benxhmark is missing very representative use case: editing of a single field, like a text box in a large form. Swap rows for hookstate is like editing 2 fields. Other state management libs also edit two rows, but are not smart enough to detect this hence whole list rerender.

[ryansolid]

Update every 10th row(Partial Update) is similar to that case where more optimal approaches won't trigger reconciliation. The way you do select rows also is this scenario.

Swap Rows is the only test in the benchmark that actually tests list reconciliation with changed positions (technically replace, remove row and append do too but nothing has moved). There were some attempts to add more interesting move tests but it proved difficult to make them fair(predictable cost) while not making them optimizable for since they were predictable. Just left things at sort.

Understanding how your library works I see the reason to leave select row as it is. That test was born out of an MVC time where you'd never store your selection state on the global models or store. But that isn't how things are always done today, with some putting all their state in global state containers. So even if it isn't equivalent it seems it could be idiomatic for your approach. Any change you made would still basically work the same I imagine.

Keyed Swap Rows destroying and recreating rows does not seem idiomatic. It doesn't scale on node size or number of nodes. It doesn't swap the rows in a keyed sense.. ie TR rows that correspond with data are moved in the DOM preserving DOM state. It just happens to be faster than React's naive list sorting on this particular scenario. I think we should change isKeyed test to detect this and fail.

[avkonst]

Update every 10th row(Partial Update) is similar to that case where more optimal approaches won't trigger reconciliation.

This is only partially in the same bucket. This test requires Hookstate to trigger rerender for 100 rows (but not only 1, like in the typical scenario when a user edits a form field). Updates for 100 rows individually are only 20-30% faster in react than updating the whole 100 rows table. If it was only 1 field update, the difference in performance between react-hookstate and react-hooks and others would be in similar magnitude as swap rows currently.

[avkonst]

Understanding how your library works I see the reason to leave select row as it is.

Thanks for understanding.

[avkonst]

It doesn't swap the rows in a keyed sense.. ie TR rows that correspond with data are moved in the DOM preserving DOM state.

Not every framework will be able to move the existing dom elements without destroying / recreating.

[avkonst]

destroying and recreating rows does not seem idiomatic. It doesn't scale on node size or number of nodes

I will be happy to see the new benchmark where more aggressive shuffling is done (i.e. sort by label?). This will be representative for scalability and can be verified. Sorry, swap rows does not reflect this "ideomatic" intent.

[avkonst]

I think that ultimately people will take the fastest methods so all of these rules and posturings are pointless. However, there is value in how a library is presented for comparison beyond the performance.

So, shall we approve the PR?

[leeoniya]

I will be happy to see the new benchmark where more aggressive shuffling is done

the labels are random, so the results would not be deterministic between runs :(

i think a lot of frameworks will do poorly enough on sorting 1000 elements, as to spell an untimely death for krausest's laptop.

[avkonst]

I will be happy to see the new benchmark where more aggressive shuffling is done

the labels are random, so the results would not be deterministic between runs :(

i think a lot of frameworks will do poorly enough on sorting 1000 elements, as to spell an untimely death for krausest's laptop.

sorting was only an example. other more aggressive shuffling could be 'reverse', '1,999,2,998,3,997, etc..' The point is not about sorting but about enforcing "idiomatic intent" by the test.

[leeoniya]

The point is not about sorting but about enforcing "idiomatic intent" by the test.

right, i'm 100% for having a test like this.

[avkonst]

It doesn't swap the rows in a keyed sense.. ie TR rows that correspond with data are moved in the DOM preserving DOM state. It just happens to be faster than React's naive list sorting on this particular scenario. I think we should change isKeyed test to detect this and fail.

Few question here: So, the expectation is 2 TR rows are swapped, but not destroyed/recreated? If my framework could do exactly this, would it pass your criteria, which you are proposing to verify in the improved isKeyed test?

[leeoniya]

So, the expectation is 2 TR rows are swapped, but not destroyed/recreated?

yes. the current test is a bit rudimentary in that it does not actually test that a swap happened. MutationObserver can report a remove/insert even when the elements are adjacent and detached/reinserted in one op:

https://jsfiddle.net/wfa5v71s/

this test should probably be improved and may reveal some existing inconsistencies in framework behavior. the original intent of the test was to quickly detect keyed vs non-keyed behavior.

[avkonst]

So, the expectation is 2 TR rows are swapped, but not destroyed/recreated?

yes. the current test is a bit rudimentary in that it does not actually test that a swap happened. MutationObserver can report a remove/insert even when the elements are adjacent and detached/reinserted in one op:

https://jsfiddle.net/wfa5v71s/

this test should probably be improved and may reveal some existing inconsistencies in framework behavior. the original intent of the test was to quickly detect keyed vs non-keyed behavior.

So, if yes, the expectation is that a framework does it in any way it can do it (assuming it is coded like it would be in a real app)? For example, Hookstate has got existing StateMemo routine which behaves like React.memo, but the input is a state and the memoised state is anything (can be DOM, why not? just never though about using StateMemo to memo DOM). It would swap rows without creating/destroying TRs (will verify it next week, not 100% sure currently). Will it pass the criteria?

[avkonst]

Hookstate has got existing StateMemo routine which behaves like React.memo, but the input is a state and the memoised state is anything (can be DOM, why not? just never though about using StateMemo to memo DOM). It would swap rows without creating/destroying TRs (will verify it next week, not 100% sure currently).

OK. I must admit that it is impossible to move two rows by Hookstate without asking React to do it, or without deleting/creating them. So, for me the only way to keep the current isKeyed check happy is to destroy/create rows.

If it is not acceptable, I will need to drop off scoped states and make the implementation really equal to react-hooks (this will make the performance equal to react-hooks - confirmed as I started with it). In this case the benchmark will not give much value for me, because it will not highlight the strongest benefit of Hookstate - fast in place per row updates (as I mentioned above there is no such a test to cover this scenario).

So, @krausest , please let me know your verdict. As far as the current isKeyed check concerned - it is passing.

[krausest]

In the true sense of a keyed update I think it‘s a requirement that the dom nodes are actually moved and not recreated (keyed means dom nodes and data list items are linked). I‘ll check if the isKeyed test can detect this and I‘m pretty excited what it‘ll report for the other frameworks. I hope to report back soon. (Please leave the PR open)

[krausest]

I opened #694 for the swap rows test.

[krausest]

FYI: I'm on holidays the next few days and will leave the PR open and decide how to proceed on when I'm back.

[krausest]

In the mean time I've flagged all keyed implementations that do not properly handle swap rows. I think at some point in time I'll move those implementations to the non-keyed section. Is there a chance to implement swap rows as a keyed operation for hookstate?

[avkonst]

Yes, there is a way to do it. It will not be much more different than plain react hooks, because of the constraint on swap rows mechanics you placed. I will probably do 2 implementations keyed and non-keyed. Apparently Hookstate will be the first framework for React to add non-keyed implementation. Overall, your benchmark needs new test: update of a single field (not 10% of rows, but just one). It is very frequent and common scenario - users edit one field at a time when deal with settings, forms, etc. This is the case where Hookstate performance is nearly equal to vanilla JS outperforming Redux and alike by magnitude.

[krausest]

I'm closing this PR due to a lack of updates.