update Mikado to latest + fix issues

[ts-thomas]

Since the full proxy mode isn't allowed anymore for some reason, I changed the keyed implementation to non-proxy.

Passed Issues

• 634 The HTML structure for the implementation is not fully correct.
• 694 Keyed implementations must move the DOM nodes for swap rows
• 772 Implementation uses manual DOM manipulations
• 796 Implementation uses explicit requestAnimationFrame calls
• 800 View state on the model
• 801 Implementation uses manual event delegation

[ts-thomas]

Why Mikado non-keyed isn't listed anymore within the results?

@ryansolid maybe you could help me please? Because when the acceptance criteria has changed I would like to fix the issues.

[ryansolid]

Yeah, I think Mikado being removed from Non-Keyed is probably a mistake. It looks like the issues are being applied there as well and so the filters. I never reviewed the Non-Keyed implementations so I can't speak to what if any rules they should follow. @krausest I think Thomas is right, and there is something not right with the Non-keyed filtering. Even if we hold non-keyed to the same standard around implementation details, the error #694 only applies to keyed.

---

To clarify it's not that proxies aren't allowed. MobX, Vue, and Solid State implementations uses proxies in a way that doesn't get marked. It's that we are marking implementations which put the selected state on the data model #800 . These ones aren't hidden as errors etc.. It's just acknowledging they aren't testing the same thing. Selection test is important since it is the only example of hoisted state change so it is good indicator of implementations that aren't approaching this test in a generalizable way. Ie with same data that could serve multiple views with different lifecycle.

As for this implementation, the same criticisms from before that lead you to using the proxies are still true now. While you aren't strictly using DOM manipulation out in the open you have a special helper method for each case that the implementor identifies and calls.

run => view.render
runlots => view.render
add => view.append
update => view.update
clear => view.clear
swap => view.reconcile
remove => view.remove
select => view.update

It takes a little digging to say #772 applies to this but that is where landed last time as the data doesn't determine the operation.

The change in this repo has mostly been instead trying to force all implementations into basically the lowest common denominator declarative implementation (sanity check: "Does this look like React, Vue, Svelte etc...?") we just categorize and let different implementations exist here. This is all about categorization of implementations trying to do the same things. It can be ignored. But now we have a way to group similar implementations.

So either way you go you can represent Mikado the way you want. The current proxy implementation with a fix for #694 (the real issue) basically falls under the same category as Sinuous marked with #800. And this PR which I think is the more idiomatic approach for Mikado falls under #772. Maybe there is a version of the proxy which doesn't fall under #800 ? Maybe it doesn't matter.

My libraries implementations fell under #772 until recently as well. I didn't want that so I changed them for a hit in performance (you can see all my non-Solid implementations vacate the top 10). I debated making the implementations #800 as that would gain most of it back but I ultimately decided I'd rather there be no question that these were comparable implementations.

[ts-thomas]

Thanks a lot for your detailed answer. I totally agree with #694, #772 and #800.
I fixed the select test #800, so multiple instances with the same data can handle its own select state now, because it is bind to the view instance. #772 does not apply, because all methods like "render", "update", "append", "remove" runs through the full render tree of the component (the template factory function which is created when parsing a template). Also #694 the swap rows test is now fixed in this version, the proxy implementation of Mikado had this behavior because the the proxy also applies to index access, which is something special (and btw. really awesome). So seems that all issues are fixed now compared to the previous proxy implementation of this benchmark.

Would be nice to know if I need to change anything to the non-keyed implementation or if this is just a mistake.

[krausest]

Sorry I added #694 to the non-keyed implementation, which was obviously a mistake. I updated the results such that it's back.
I'll review the PR this weekend and report the updated numbers.

[ts-thomas]

Thanks a lot @krausest

[ryansolid]

@ts-thomas

#772 does not apply anymore, it had exists in the proxy implementation but not in this version, because all methods like "render", "update", "append", "remove" runs through the full render tree of the component (the template factory function which is created when parsing a template)

I'm not sure I follow. What is the similarity/difference between these methods? Like render vs append vs reconcile? Is the implementor not hand picking what sort of DOM update that will be performed? Like I'm gathering render doesn't reconcile and blows away nodes already there. That append doesn't check the current list and only adds to the end. Does update reconcile? Does reconcile handle all the other cases as well? Like if I used reconcile on all the tests would it just be a less optimized version but work for everything? Maybe I'm misunderstanding.

To me this seems like a new paradigm for this benchmark as the abstraction while not the DOM itself is a DOM abstraction rather than a data abstraction. The view object being passed around isn't the actual DOM but it's a proxy (not literally an ES6 Proxy) for the representation of the instantiated template. This leaves the data model up to the end user as it is their responsibility to manually synchronize between their data model and view model. That is usually the criteria to be under #772 but I have to admit this is probably the smoothest I've ever seen this pulled off.

[ts-thomas]

@ryansolid It sound like reconciling is a must have, as well as using always the same function to render for all tasks. I couldn't find those condition anywhere. I think it's not good how the keyed test becomes more and more a data-driven-only test which is tailored to a specific group of libs. That misses the goal of a comparison between different keyed concepts massively.

I can switch back to the proxy implementation, I was already fine with it, but it was removed from the result table. It is hard to find out what is actually wanted and what is not. The keyed test becomes more and more political.

Edit: Contrary to your assumption, the render function automatically calls reconcile in keyed mode under the hood.

[ts-thomas]

@ryansolid I much like your solid implementation. But this line:

d[index].setLabel(d[index].label() + ' !!!');

Why it is not marked as #772 accordingly to your argumentation? Because this should also a manual DOM manipulation then. Why you did not use setData here?

[ryansolid]

That's fair question and if you look at Solid State implementation you won't find that even though they are identical under the hood. It's part of the data structure very much like a proxy (which state uses). It's the same as d[index].label += "!!!". Basically it's a matter of scope. It's nested state. When the update model isn't tied to template or component this is how you break it up. Think of it as submodels. Modelling the data in a reactive system can be as important as the view. The function is still a setter and behaves the same as the parent one.

But you are correct to think that with any proxy it is difficult to tell if it is direct or decoupled. That's basically what #800 tests though. As if it's direct DOM manipulation it is difficult to do avoid #800. If it passes that the proxy isn't just a wrapper over the DOM node. Really #772 and #800 can be the opposite side of the same coin.

I do see though the distinction between setting data, and proxying view updates have never been smaller. And the API for Mikado is deliberate and re-usable. It's almost impossible for someone on the surface to know how these are different and you can't devise a check for that. So given how subtle and not enforceable it is I don't know what to do with it.

[ts-thomas]

I found out what the issue was. I am very surprised that no one of you came to this hint:

The last option is disabled by default, but it definitively should be enabled by default. The issues are marked in the results visibly, but when hidden no one can see this at all. I'm also pretty sure that most ppl will did not notice/remember this option. We are 3 of them ^^

[krausest]

@ts-thomas This was intentional. Implementations with "hard" errors should not be visible in the comparison by default.

[krausest]

Hmm - what's the reason for closing this PR?

[ts-thomas]

@ryansolid @krausest

I thought there was a fundamental problem with the proxy implementation because it was no longer listed. Generally I would prefer this implementation from this PR, but it might looks too similar to the non-keyed implementation. Due to the higher diversity, I would prefer to fall back to the proxy implementation.

I have fixed all issues of the proxy variant, I will create a clean PR for this. Something good also turned out, I came up with the idea of ​​providing a transactional proxy in a future release. I'm very excited.

[ryansolid]

@ts-thomas Nice. I'd like to see it. This is one of those things where we have it but it always is a bit awkward. Like in Solid or MobX transactions are opt in with using setState or batch function but it doesn't always feel as natural. With some libraries they just queue up a microtask and defer execution there like Vue. I think the most interesting transactional with lists is if we are better off running a series of operations in succession or just saying .. stuff happened reconcile it. For simple operations it is definitely beneficial to be more granular but for more complicated things I think a single reconciliation pass is preferable. It's easier to default to the latter, but it would be interesting if based on the type and amount of work queued the library could be smart enough to decide.

[krausest]

(Results update didn't belong here ;-)