RFC: add new timing capabilities to modifier manager

[jelhan]

Rendered

Element modifiers enable developers to customize a HTML element. Currently modifiers are always installed after the browser painted the element. This could lead to a flash of unstyled content and negatively impact performance due to double paint. This RFC proposes adding a second timing option allowing modifiers to run before the browser painted the element.

[chriskrycho]

Seems good. 👍🏼 I think implementing support for this in ember-modifier can readily be done in a backwards-compatible way, likely just by migrating to the new timing equivalent to the existing installModifier as the default and introducing a second pair of imports (function- and class-based) which use the new timing semantics. All existing modifiers will Just Work™ that way, but can opt into using the installModifierOnLayout hook:

import { modifier, Modifier } from 'ember-modifier/name-TBD';

We need to think about how to name that, of course, but you can see how that approach works nicely from a design POV: it means the default is to use the existing semantics (which is what most users should use) while the more costly, but sometimes useful, semantics.

That said, I think it will be important to think about the relationship between the other possible timing semantics as well for the evolution of that library: before-insertion seems like it can just be another timing mode users will be able to opt into, but template-invocation-ordering is an orthogonal condition which seems likely to be cross-cutting, and it isn't obvious how to guarantee it given existing semantics about when and how we do re-renders—trying to enforce that guarantee isn't particularly natural in the context of Glimmer templates today.

[jelhan]

I think it will be important to think about the relationship between the other possible timing semantics as well for the evolution of that library: before-insertion seems like it can just be another timing mode users will be able to opt into

Yes. That's the idea. The most challenging part for that is to not complicate server-side rendering support even more. For server-side rendering a HTMLElement is not available. We only have js-dom. To not paint us into a corner, I try to address server-side rendering in the next RFC before coming to before insert semantics. But it paves the way for it.

template-invocation-ordering is an orthogonal condition which seems likely to be cross-cutting, and it isn't obvious how to guarantee it given existing semantics about when and how we do re-renders—trying to enforce that guarantee isn't particularly natural in the context of Glimmer templates today.

In the discussion I had with @wycats we tend to not support it. So far grabbing a reference to an element to be used later e.g. with {{in-element}} was named as only use case. We think that use case could be supported by other low-level primitives better. @wycats proposes adding a special syntax for grabbing a reference to an element to Glimmer templates itself in #652 (comment).

[jelhan]

I think implementing support for this in ember-modifier can readily be done in a backwards-compatible way, likely just by migrating to the new timing equivalent to the existing installModifier as the default and introducing a second pair of imports (function- and class-based) which use the new timing semantics.

While this might be a good interim solution, I don't see it as a great high-level API for long-term. It limits modifiers to schedule work for either of the timings. But does not expose the power of scheduling some work for on layout and some other work for on idle.

An event-driven API may fit better with added flexibility. It might look like this:

modifier.on('layout', (element) => {
  // do some work, which needs to be done before browser paints the element 
});

modifier.on('idle", (element) => {
  // do some other work, which could not cause a flash of unstyled content issue
});

modifier.on('idle', () => {
  // could split up the work in logically independent chunks to structure the code
});

modifier.on('destroy', () => {
  // teardown logic
});

But deciding on a high-level API is out of scope for this RFC. Thanks to low-level primitives provided by modifier manager, we can experiment with those and gather experience before committing to a specific high-level API.

[chriskrycho]

The text needs some clarification, then: as it stands the two capabilities/hooks appear to be installation-specific and that strongly implies that only one of them runs. (The text doesn’t explicitly say that but it’s what I inferred from the name!) If both hooks run, then I agree that we’d want to approach it differently; but we should update the hook names to be clear about that.

[NullVoxPopuli]

An event-driven API may fit better with added flexibility. It might look like this:

coming from Starbeam, and having worked with resources a little bit, I think I'd prefer a unified API for all things ™

for example (and with the caveat that this is very hand-wavy (except for the resource example, which comes from the docs)):

resources:

import { Cell, Resource } from "@starbeam/universal";

const Stopwatch = Resource(({ on }) => {
  const start = Cell(new Date());
 
  const interval = setInterval(() => {
    start.set(new Date());
  }, 1000);
 
  on.cleanup(() => {
    clearInterval(interval);
  });
 
  return start;
})

and an eventual modifier would be a resource that takes the element as an argument:

import { Modifier } from 'somewhere';

const Style = Modifier(({ element, on }) =>{
  // what's the timing in the body space here? pre-insert?

  on.layout(() => { /* set the style attribute */ });
  on.idle(() => { /* wiggle using the WebAnimation API */ });
  on.cleanup(() => { /* remove changes */ });
});

While this looks about the same, if you squint, I think it provides better ergonomics for editors and language servers.

But this is maybe getting off topic for what this RFC is intending to introduce.

I'm overall 💯 in favor of introducing pre-paint timings -- it would improve the perf of a lot of things.

[jelhan]

The text needs some clarification, then: as it stands the two capabilities/hooks appear to be installation-specific and that strongly implies that only one of them runs. (The text doesn’t explicitly say that but it’s what I inferred from the name!)

I agree. The current naming is misleading.

Having more than one hook, which is executed on first render does not fit well in the existing lifecycle model for modifiers in general. It currently has the following:

1. Create (one time)
2. Install (one time)
3. Update (zero to many times)
4. Destroy (one time)

Maybe we need to have something like this instead:

1. Create (one time)
2. Setup on layout (zero to one time)
3. Setup on idle (zero to one time)
4. Update (zero to many times)
5. Destroy (one time)

Maybe we want to provide a reference to the update lifecycle hook in that case as well. It would not fit well in the mental model if a modifier must implement one of the setup hooks just to grab a reference to the element.

[jelhan]

I renamed the hooks to setupModifierOnLayout and setupModifierOnIdle. Additionally I added a clarification that the same modifier manager can implement both hooks. @chriskrycho, I hope this addresses the confusion you raised.

[NullVoxPopuli]

how has exploration been going? This is highly needed! <3

[wagenet]

@NullVoxPopuli I think part of our challenge here is making sure that we're actually going to do the work. Is this something you'd be interested in helping with?

[NullVoxPopuli]

yes, It is -- partially to help with graphics and chart rendering performance, and partially to help out with ergonomics and performance in how ember-headless-table is wired

[machty]

Another nice use case for this: merging tailwind classes for component library use cases: https://twitter.com/buschtoens/status/1658483457000103938

[achambers]

@wycats Are there any capabilities in things like Starbeam that impact this RFC? Do you have any context here?