New rule: Prevent direct DOM access

[krausest]

Please vote "thumbs up" for the formulation of the new rule that should be added to the benchmark or propose a better formulation:

[krausest]

Proposal 1: The implementation must use only the idiomatic style of its library (e.g. direct dom access is not permitted).

[krausest]

Proposal 2: The initial rendering must be able to render the selection state. (Question: Is that sufficient to prevent reaching into the dom with the initial state?)

[krausest]

Proposal 3: Don't use shortcuts in your implementation. Using explicit event delegation, manipulating the dom directly or using direct request animation frame calls are considered bad smells and may lead to rejection.

[WebReflection]

@krausest mind expanding on the explicit event delegation? Both lit/lighterhtml use the row to intercept once internal events, while hyperHTML, as example, places listeners right on the node that should trigger the action. Wich implementation is correct, if not both?

[WebReflection]

P.S. I like all proposals but I'm not sure I get the last one

[krausest]

@WebReflection The idea with event delegation came from #430, but maybe it's not really well put. I'd consider it an illegal shortcut if e.g. a react implementation used userland code to add the select row event listener to the table.

Maybe this way:

Proposal 4: Use the idiomatic style of the library and avoid userland shortcuts in your implementation like userland dom manipulations, userland request animation frame calls or userland event delegation. These are considered bad smells and may lead to rejection.

[leeoniya]

yeah, there are a bunch of libs that delegate and dom-climb (in the impls):

https://github.com/krausest/js-framework-benchmark/search?q=parentNode&unscoped_q=parentNode

[WebReflection]

@krausest I like proposal 4 the most 👍

[leeoniya]

(e.g. direct dom access is not permitted).

this part is gonna be the most controversial. i feel like outright disallowing it is a bit heavy-handed, but policing the details gets messy in a hurry. with the right abstraction, it could probably be acceptable as long as it's generalizable for other applications and is uniform for create and update. for example, if the framework had a way to notify of the granular necessary textContent changes but left the actual mechanics down to a subscriber/driver. maybe if there is a framework that allows you to bring your own rendering driver. like if core React lib was the submitted lib and react-dom (the "driver") was just part of the impl.

[adamhaile]

This conversation has gotten so far into the weeds.

The point of DOM libraries is not to abstract all access to the DOM. It's to help people build useful apps.

There are lots of ways to do that. All have their respective pros and cons. Some people -- yeah, I'm calling out @localvoid here -- have gotten so far into the VDOM approach that they have tunnel vision. They demand that all other libraries adopt the same cons VDOM faces or they're "cheating."

I ask a library to do what it does well, and for the rest, to get out of my way -- which is to say, to interop with non-library code in predictable ways with minimal ceremony. The fact that Surplus (and lit-html) plays well with the DOM is a feature, not a bug.

(Aside: Web Components are built on DOM nodes as an API. If you put this rule in place, you're saying Web Components are wrong. Frankly, I agree that Web Components are a bad design, but not for this reason.)

None of us are doing original research here. All these approaches were tried in the last millennium, most in the 70s. If there was a clear winning strategy it would have won 30 years ago. Surplus is based on research done by INRIA in the 80s and 90s into declarative, predictable effects. It is idiomatic to work with the DOM API in Surplus, because Surplus intentionally makes it possible, and S computations let you do it in predictable and declarative ways.

/rant

I would propose a different rule:

Before submitting to this benchmark, please demonstrate that your library has been used to create realistic apps by providing a link to an implementation of either the realworld demo or an app of roughly equivalent complexity. Note: TodoMVC is not sufficient.

(Put-up-or-shut-up: https://github.com/adamhaile/surplus-realworld )

I think this rule would filter out a lot of the libraries that were built just for this benchmark. I support people exploring ways to achieve awesome performance, but they shouldn't be in the benchmark until they've proven they can build useful apps.

[WebReflection]

@adamhaile a repo called real-world doesn't mean every library is used in the real world. hyperHTML ships with ABP to million users and it's been used by W3C Respec for long time and I've never had the time, or the interest, to PR real world cause I've done all the others already plus the lib literally ships to the real world so your proposal is also kinda irrelevant. Also worth saying that I agree many libraries are there to show potentials, even if experimental, but then again that's the beauty of this project, imo. I do agree though, that there are too many entries so that maybe those about libs that haven't been updated in a long time should probably be removed as irrelevant/outdated, but even here I don't have strong feelings about it.

[WebReflection]

@leeoniya I think if the way of the library is to manipulate directly the Dom then let it be? This new rule purpose is more about avoid cheating for the sake of scoring.

[adamhaile]

@WebReflection Yep, the things you cite are exactly the kind of proof I was referring to. They show that hyperHTML is a mature and useful lib. I cited realworld as an example of a minimum requirement -- hyperHTML is obviously way over that. My comments were directed at some of the libraries that don't seem to have any use beyond submitting entries to this benchmark. Those libs should absolutely be implementing this benchmark for their testing, but I would argue that they shouldn't submit until they've developed the lib to the point that it can build useful apps.

[Freak613]

@adamhaile , correct me if I'm wrong, But surplus-realworld has zero usage of manual event delegation or computation lifting, and can be considered as idiomatic implementation. You didn't used it there, but used it in the benchmark, for the reason.

It's not in the demo, because optimization requires additional effort to find tight places and apply fixes. You can say that in React, when we put sCU we also need an effort for that. But in React, sCU shows feature of the library itself, how it can address dataflow optimization. And as a bonus, we can port it to other platforms, and still utilize its benefits. S.js is the same, cause of data-centric approach.

Most of the libraries, if not all, have exit gates to the DOM. And if not Ref-s, access to event.target is possible literally everywhere. And easiness of porting this solution to other libraries demonstrates that this is not new/unique feature. It's not the feature of the library, it the feature of the DOM spec. It can be okay if you go down to the DOM to apply additional logic to the element itself. But it looks "hacky" when you use it to build additional bridges to get around the view logic.

However, I agree about matureness requirement, other way the benchmark will soon be full of one-day libraries. And my libraries in this bench can be considered as that (if don't look at the numbers of npm downloads), although they're not using techniques described here, working in the designed way. That's what stops me from pushing any new raw libraries here, until I test it enough to decide that I reached sufficient functionality. It's not clear how to define this matureness though, but good point to have it in mind.

I sure you, if we're thinking that manual delegation and lifting is best what we can, that's not true. It's limiting us. There should be ways to generalize such approaches, and instead of arguing about what level of workarounds is acceptable in this or other benchmark, it's better to face that there is no perfect library, there always be weak points and edge cases, and start thinking about how to fix that. Library developers should carry over as much complexity as possible, there was no VDom 10 years ago, but now it's here and it's helpful no matter how bad it is at some points.

[ryansolid]

I'm all for proposal 2, but that should be no surprise. After all the discussion in #430 only the stipulations I made in the first post were never contended. I feel any of those rules would be fine to set. The rest of the discussion goes off about exposing DOM, categorization, and degree of severity of different bad things one can do. But it was never my intention. I wanted to focus on an agreeable baseline rather than argue about how we are all different.

To me matureness is pretty hard to enforce. We can stipulate again some very very basic constraints but they'd be too loose for those that care. My personal opinion is a library is only misrepresenting itself when it doesn't have means of control flow abstracted into the library or it hasn't addressed it's disposal logic. If your library can handle mounting and unmounting conditionally and rearranging lists, without creating memory leaks, it's enough. We definitely have had libraries that don't have the primitives to make real apps, and there is definitely some libraries that are memory leaking right now. Most obvious indicator is when test 24 and 25 have relatively the same high memory usage. None of the tests, test conditionals. I'm gathering there was a show/hide at one point that did. Largely redundant outside of it ensures the library to do conditionals.

EDIT: Thinking about it TodoMVC. If your library can do the official TodoMVC. Realworld app I've looked at and I've wanted to do but I imagine it would take me days if not weeks given a small windows of time to work on it. TodoMVC isn't as full featured but it contains all the core pieces for the library to be functional.

---

But while we are talking about how we are all different. I know why the Top Down render libraries see just updating it outside of the render cycle so taboo, it feels hacky to just go in there after the fact and mess with the DOM even if it was masked from the implementor. Even if you compile the view into separate creation and update paths it is something fixed you don't touch that runs systematically from top to bottom.

In fine grained, all rendering works like the hack. There is no render cycle. Everything is after the fact (after the initial render). Any part of the DOM updates independently. You find yourself writing directives to encapsulate DOM manipulations as that is the same foundation the whole rendering is built upon. Whether I'm binding to update an attribute or defining some complicated reusable behavior it is more of the same. It's an event system more than a rendering engine. It's the same reason I can take Knockout and use it completely differently with ko-jsx. With a few primitives you can change how any part of it works.

But it looks "hacky" when you use it to build additional bridges to get around the view logic.

That is exactly what directives are. It's been idiomatic to do that in fine grain libraries forever. I'm glad @localvoid has illustrated how in general every library can do this though because it brings to light the fact the ability to do so isn't necessarily a feature. However it doesn't mean it isn't more idiomatic for some libraries to approach things this way. That being said I'd love a future where end user directives were not a thing as it definitely is more cognitive overhead for developers.

On the other hand event delegation is one that I've been struggling with a lot since generally explicit event delegation is an optimization technique you only use when desired. It doesn't usually feel free. When I'm using webcomponents I use it a lot more. When I'm not less so, since I'm generally not exposing custom events. Being able to define the handler methods on the children rather than the parent make a lot of sense. but is really difficult for single pass libraries that have to consider the ShadowDOM. If you haven't attached the node to the parent how do you know which shadowroot to append the delegate to. The only reasonable option is explicitly add the event listeners for the node handling delegation but that requires more verbose bindings across multiple elements to pull off.

I always link the page in Knockout docs where they tell you how explicit event delegation is what you want to do in these scenarios in their fine grained library. The truth is you won't usually use it unless it's called for to optimize, at which point you will do so automatically and assuredly. You think no worse for doing it. Is a benchmark a place where you'd optimize? I think so. Is it idiomatic? Sort of, it is the recommended solution for the problem. It's what anyone using the library would do in that scenario. Any library could do it, but would they?

So what are we benchmarking? Is this a comparison of different render techniques or a showcase of how someone would use the library if presented with the problem? I realize we lose if everything just digresses to vanillajs, but many libraries that don't have implicit event delegation will almost certainly use explicit when the right scenario comes along. It's part of the toolbox.

[alexfmpe]

So what are we benchmarking? Is this a comparison of different render techniques or a showcase of how someone would use the library if presented with the problem?

I think it also matters for whom we are benchmarking.

Implementers leverage this benchmark to have a feel for how their optimizations affect several measurements in simple scenarios one can reason about. In this case, one can just run it locally without worrying about how it performs against the other frameworks. We've done this a fair bit in Reflex-DOM to check for significant performance regressions in some large changes.

As for consumers of the lib, I imagine they're mostly concerned with

• is the idiomatic use of this lib fast enough for using in the bulk of my webapp?
• when I actually need great performance, will I be able to have it, even if via less idiomatic, not-as-nice code? or do I need to switch to something else all together?

I can't help but feel we ought to be measuring both extremes: allow frameworks to have a version where all the abstractions that make sense are used, and an every-trick-in-the-book version. That way folks can judge whether the 'elegant' one is too slow or the 'fast' one is too difficult to work with for their purposes.

Right now we force each framework to pick one point in this spectrum and then have a hard time of getting everyone to agree on which point should be chosen, weakening the value of the comparisons.

[Freak613]

Probably, one more option is to mark in the result table whether or not library used access to DOM from the userspace and let the people do whatever they want.

People who referring to benchmark results will immediately get information about how good library handle bottlenecks and do they need to write manual updates. @localvoid did something similar in uibench, with short briefings for every library. For this benchmark, some icon at the head will be enough.

[leeoniya]

possible candidates for indicators:

• userland event delegation & .closest()
• dom readback (getting id from already-rendered content rather than bound data/ev handler or addl attribute)
• imperative dom mutation (adding classes, updating textcontent)

until the details are hashed out, can we all agree on Proposal 2 (the initial rendering must be able to render the selection state.) and maybe perhaps that selected has to be out-of-band and not pollute each data object?

are there any thoughts on whether caching previousSelected for the purposes of fast deselecting should be a allowed? i'm in the "no" camp on this. but maybe this already falls under the rule of impertive mutation (even if via some lib-exposed proxy).

[Freak613]

Having marks, need for any new rule becomes not so critical. From real world standpoint it's unlikely you have selected element for not rendered list, and the benchmark doesn't test this case, therefore making this requirement restrictive just to make a restriction.

The short recipe for unrestricted environment will be:

const next = e.target.closest('tr')
next.className = "selected"
prev.className = null
prev = next

And the whole place will burn. And it makes storing selected in the state meaningless, we don't have requirements for store organization initially, delegated it to developer hands.

[Freak613]

I would put just one mark, is the implementation fully Data-Driven or not.

UPDATE: okay, not suitable to filter out DOM access. Then any term/description about restrictions from this and similar tickets.

[ryansolid]

What I'm getting at is that the state at any point can be rendered from the data not stored in the DOM is the only definition that universally sticks for what being "data-driven" means. @Freak613 when you say fully data-driven what do you mean?

I don't believe precludes the potential of DOM manipulations and caching as libraries do that under the hood anyway, it's a matter of chosen exposure/abstraction. Being data-driven doesn't exclude imperative manipulation, just says that the state belongs in the input not in the output of:

const view = fn(state);

fn could be a lot of things and still be data-driven.

[adamhaile]

it's better to face that there is no perfect library, there always be weak points and edge cases, and start thinking about how to fix that.

@Freak613 just wanted to highlight this point to agree 1000%. That's the spirit that I wish prevailed here.

The Surplus implementation here hasn't changed for several months. That's not because I think it's perfect. I often pursued a "ratcheting" strategy: make it fast-but-manual to put a line in the sand, then make it fast-but-clean while achieving the same performance.

Frankly, the level of trolling got high enough here that I took my time elsewhere. All approaches have their pros and cons, but some here think that their approach's cons are more important than others, and if other libraries don't adopt the cons they have to face, then they're cheating.

Meanwhile, we're not even talking about implementations that:

• use globals to avoid the cost of creating closures and prop objects
• can only choose between immediate or rAF rendering on a global basis, even though that's a benchmark-only feature that is useless in non-trivial apps (sidenote: it can be predictable if you pursue a massive scheduling endevor like React's Suspense, but none of the apps using it here have anything close to that)
• use object identity as a covert side-channel to tell the rendering engine whether a row is affected by a selected() change

But DOM access -- even though your library is based on an entire area of research (SRP) oriented toward making predictable, declarative side effects -- that's beyond the pale.

[krausest]

The voting wasn't exactly helpful to select a new rule 😬 . I added a best-of to the readme (last rule in how to contribute) - hope that helps.