RE: [web-animations] Readonly interfaces and inheritance: seeking advice

Hi Brian,

Sorry for the delay in getting around to this. I admit this is a large API with lots to get my head around, so apologies if I fail to understand what's going on in the first few exchanges. But let me try:

> What do you think of this hierarchy? Is there a better way of keeping AnimationTiming and ComputedAnimationTiming in sync without introducing AnimationTimingReadOnly?

I can't really see the benefits of the hierarchy as-is. In particular, I don't see any concrete uses of the AnimationTimingReadOnly interface in the API: no properties expose it, for example. It doesn't even seem that helpful as a spec device, since AnimationTiming has to duplicate all the members of AnimationTimingReadOnly anyway, so you have to synchronize those.

At the very least, I would make AnimationTimingReadOnly into e.g. `[NoInterfaceObject] SharedAnimationTimingProperties` and use `implements` instead of `:`, so that it becomes a spec-device mixin instead of an exposed-to-JavaScript inheritance hierarchy. But there may be better options.

> What we are *trying* to do here is expose both the specified timing (which is input/output) and calculated timing (which is purely an output). Many of the fields are the same but sometimes the range of values differs. For example, the |duration| member of a returned ComputedAnimationTiming object will always be a double, not a string. Likewise, the |fill| value will never be "auto".

Honestly, if you are willing to deal with the maintenance burden as a spec writer, I think making this distinction explicit, by creating separate classes that are not at all related, would be best. `SpecifiedAnimationTiming` and `ComputedAnimationTiming` seem like a great division. That way any small differences like the ones you mention do not need to be awkwardly shoehorned as overrides to base-class behavior, but instead can be given their own space to breathe. But I recognize that keeping two classes that are close-to-the-same in sync might not be something you're excited about, so that's where a mixin strategy like above might be better, despite potential awkwardness around the small divergences.

> We've also added some attributes onto ComputedAnimationTiming that aren't on AnimationTiming(ReadOnly) like timeFraction etc. Perhaps they don't belong there since they're more about *current* state rather than the result of resolving timing parameters but maybe it's ok?

I'm not sure I have the domain knowledge to answer this question very well :). How is ComputedAnimationTiming meant to be used by user code? Maybe it should be named CurrentAnimationTiming instead, and then this wouldn't be a problem?

> * Or should computedTiming just return a snapshot object with different object identity every time it changes? Perhaps a method that returns a dictionary object?

This is kind of a tough one. Is the intent that, now and forever, ComputedAnimationTiming (or CurrentAnimationTiming) is just supposed to be a record containing information? Or could it ever acquire behavior? If the former, then a dictionary snapshot sounds very nice.

Regarding method vs. property, I think the distinction there should be based simply on whether or not the operation is expensive. If gathering the computed information is expensive, it should be a method, no matter what it returns. Similarly if it is simply returning pre-determined information, it should be a property.

> I guess AnimationNode.timing should not be readonly? I think making it [Replaceable] might be problematic from a performance point of view but if we define setting as doing a member-wise copy is that idiomatic?

Hmm, I think I am missing something---what is wrong with it being readonly?

> For example, is this better?

I think it is better than the hierarchy version, but I am curious what you think after digesting the above points.

> We talked about making AnimationNodes generated from CSS be read-only so you'd have to clone it if you wanted to modify it rather than ending up in a situation where the object is partly live due to being partially overridden by script. That's a separate discussion but your suggestions to the above will probably help guide that.

In general, I (unlike some of my fellow TAG members) agree that having things be read-only is a useful signal that they are controlled by the browser and not by you. I make an exception for pure-record snapshots, with no behavior or class-ness at all. Animation nodes seem very much class instances, and not records, so without knowing the specifics very well, read-onlyness seems valuable in these cases.

Received on Wednesday, 9 July 2014 20:36:50 UTC