[css-animationworklet] Avoid StatefulAnimatior/StatelessAnimator superclass and use existence of state() function

[majido]

Currently AnimationWorklet differentiate between Stateless/Stateful animators by requiring each specific animator class inherit from a specific superclass. AudioWorklet does something similar with AudioWorkletProcessor.

To specify this correctly we need to check the prototype chain for the registered animator class and see if any ancestor is actually the class we expect a.k.a. IsSubclassOf(class, expected_ancestor_class).

AFAICT, there is no such operation in ECMA262 to achieve this. So for now we have worded the spec to use "SameValue" operation and only check the immediate prototype.

Our options are:

• Ask ECMA262 to add such an operation.
• Implement this in our own spec e.g., by walking up the chain and using the [SameValue].
• Scrap using super-class in our API!

Note: I looked at AudioWorklet specification but while their working requires extending AudioWorkletProcessor but the actual registration algorithm does not actually check the prototype chain to verify this and also Chrome implementation does not check it. So AFAICT basically they have the same problem if they are to implement this correctly.

[hoch]

What's the side effect of "scraping super-class from the spec"?

[majido]

For AnimationWorklet this is not a big deal. We currently only use the super class a way to make distinction between Statefull and Stateless animators. I am not sure about AudioWorklet, do you depend on having a superclass so that you can have 'onmessage' method on?

[majido]

The decision to use two superclasses was made in F2F and discussion is here

To summarize the options we considered was:

1. Having a static attribute on the class. This was considered not to be that ergonomic
2. Looking at the shape/number of constructor argument. This is not well supported by JS/WEBIDL.
3. Use two super classes: This is the one we choose but even at the time I and others were concerned about its ergonomics [1].

I think we do have another option to do this that is both ergonomic and practical: Use existence of "state" getter to indicate that an animator is stateful. We do expect stateful animators to provide a state getter so let's make that the condition.

// Stateful
registerAnimator('a', class {
  constructor(options, state) {}
  animate(currentTime, effect) {}
  // The fact that there is a state getter indicates this is a stateful animator
  get state() {} 
});

// Stateless
registerAnimator('b', class {
  constructor(options) {}
  animate(currentTime, effect) {}
});

This is more ergonomics since it does not require a superclass, much easier to spec and implement, consistent with how we require and check existence of animate function, and requires less work on the web devs part since they are already writing state getters anyways for their stateful animators.

One argument in favor of using superclass was that in future we may use it to have other methods, attributes but so far we don't have anything like that. So having a better ergonomic API now is better that some potential future benefits.

[1] majidvp: Okay....you're using a heavy hammer for something that could be an attribute. Attributes are very common.

[majido]

To make the difference more clear. Here are the two choices:

Currently specified - Use two separate superclasses

// Extending StatefulAnimator indicates this is a stateful animator.
// It still has to implement state() function.
registerAnimator('a', class Foo extends StatefulAnimator {
  constructor(options, state) {}
  animate(currentTime, effect) {}
  state() { return 'some state'; } 
});

registerAnimator('b', class Bar extends StatelessAnimator {
  constructor(options) {}
  animate(currentTime, effect) {}
});

Cons:

• Two super classes (as opposed to mixin) makes its harder for authors to mix it with any existing class hierarchy they have. I suspect most will just have StatefulAnimator at the top of their class hierarchy which defeats the purpose.
• The author still has to implement state() function anyway.
• More difficult to spec and implement.

New proposal - Use state function existence to differentiate

registerAnimator('a', class Foo {
  constructor(options, state) {}
  animate(currentTime, effect) {}
  // The fact that there is a state function indicates this is a stateful animator.
  state() { return 'some state'; } 
});

registerAnimator('b', class Bar {
  constructor(options) {}
  animate(currentTime, effect) {}
});

Pros:

• No requirements on class hierarchy which allows a lot more flexibility.
• Implementation of state function(satisfying the contract) is enough!
• More consistent with other Houdini worklets which don't require super class.
• Simpler to spec and implement.

Cons:

• Having a superclass is nice e.g., we can add other methods that have meaningful default behavior (e.g., pause, play) but that only needs a single superclass Animator and also can be done later when it is needed.

[css-meeting-bot]

The Houdini Task Force just discussed Avoid StatefulAnimatior/StatelessAnimator superclass and use existence of state() function, and agreed to the following:

• RESOLVED: accept majid's proposal to use a state() function's existence to detect statefulness, and drop the superclass check

The full IRC log of that discussion

<Rossen_> Topic: Avoid StatefulAnimatior/StatelessAnimator superclass and use existence of state() function
<TabAtkins> ScribeNick: TabAtkins
<Rossen_> github: https://github.com//issues/850
<TabAtkins> majidvp: Last year, we made a resolution on how to differentiate between stateful and statless animator
<TabAtkins> majidvp: Idea was to use two seaprte classes
<TabAtkins> majidvp: You inherit from whichever you want
<TabAtkins> majidvp: We lookeda t some different options: looking at property on aniamtor, o rlength of arguments, etc
<TabAtkins> majidvp: While trying to implement this, I realized there were complications
<TabAtkins> majidvp: So new idea I thinka ddresses the shortcomings
<TabAtkins> majidvp: Rather than requiring a superclass to be extended, we look at the existence of the state() function
<TabAtkins> majidvp: So if your animator implements the state() function, they're a stateful animator. If not, they're stateless.
<TabAtkins> majidvp: I think that's simpler. THe interface matches the contract.
<TabAtkins> majidvp: Problem with the superclasses is that it clashes with bringing your own class hierarchy.
<TabAtkins> majidvp: It'll cause people to just extend StatefulAnimator at the top of their hierarchy, which defeats the point.
<TabAtkins> majidvp: This seems more consistent with other worklets too - PaintWorklet and LayoutWorklet don't require extending a class.
<heycam> q+
<TabAtkins> AmeliaBR: An issue I see with that is that from a JS perspective, you can add a getter function on an instance.
<TabAtkins> AmeliaBR: I'm assuming you judge it at the registration stage.
<TabAtkins> AmeliaBR: What if you put one on the instance?
<TabAtkins> majidvp: Verify it at registration stage, like paint functions works.
<TabAtkins> majidvp: If you add state() later, it'll be ignored.
<TabAtkins> majidvp: We want it to be a single value at registration for the whole life.
<TabAtkins> TabAtkins: So like we'll do a Get on the class object, seeing if "state" returns a function, and make the decision based on that. This is similar to, say, Promises and .then()
<TabAtkins> AmeliaBR: I see some different examples, a state getter and a state method. So it's the method you're discussing?
<Rossen_> ack heycam
<TabAtkins> majidvp: yes
<heycam> https://tc39.es/ecma262/#sec-ordinaryhasinstance
<TabAtkins> heycam: In the first issue comment, you say the current superclass model we have to ask tc39 to add an operation, but I think it already exists.
<TabAtkins> majidvp: I started looking at this becuase I wasn't sure how to spec it, and AudioWorklet already does this, but they haven't correctly specced it.
<TabAtkins> majidvp: During that my new idea came up, and I think regardless of speccing, this is a better API design.
<TabAtkins> proposed resolution: accept majid's proposal to use a state() function's existence to detect statefulness, and drop the superclass check
<TabAtkins> RESOLVED: accept majid's proposal to use a state() function's existence to detect statefulness, and drop the superclass check
<TabAtkins> <br type=lunch>