Open majido opened 6 years ago
The class registration style is consistent with other houdini APIs e.g., paint API.
As I pointed out in the other issue, the worklet global scope itself may be created/destroyed at will so all the code within its scope has to be idempotent/pure. AFAICT Javascript has no way to ensure a function is pure so this is best we can get and it also means that all three syntax are equivalent in terms of actual purity which is none. (/cc @bfgeek who is the expert here)
Also, note that in the proposed scheme, the UA may create a few (or just one) instance of the class and call the function with different inputs on the same object instance. Multiple animator instances does not means we will have 1:1 object instances in the worklet scope for them which is why the authors should write pure functions anyway.
The advantage of the class syntax is the ergonomics and its scoping.
Just to clarify, the way I read it your proposal was:
animate
function and the invoke it with this
bound to the worklet global scopeanimator
and call its animate
function.If you meant something different please let me know.
From @appsforartists on September 27, 2016 21:12
I hadn't given any thought to what this
would be bound to - I'm still learning how Worklets work. 😃
From @appsforartists on September 27, 2016 21:15
If the animators are already required to be pure, I think the first alternative (an anonymous function with certain values set on it) is the clearest representation. It's reasonable for an author to assume that a class instance has mutable state that persists across frames. Otherwise, it ought to just be a function.
If this is the structure of the Houdini APIs in general, it sounds like a bigger discussion. I'll pick @bfgeek's brain next time I see him.
From @appsforartists on September 27, 2016 21:16
(Regardless of their representation, if it's a requirement that animate
functions are pure, that should be noted in the documentation. It's presently unclear.)
From @appsforartists on September 27, 2016 22:31
Incidentally, one of the things React does really well is offer its component API in two flavors:
The natural thing for React users to do is author all components as simple functions until a case arises that needs local state, when they can upgrade that component to the more complex class API.
I understand the desire for consistency across Houdini APIs, but also thought it was worth documenting my inspiration for opening this issue.
I agree that the idempotency requirement needs to be documented particularly once we start writing a spec but perhaps it is worth a mention in explainer as well.
Thanks for pointing out the React conventions. It definitely helps to understand well established patterns when designing the API. In particular, if we support plain Javascript functions similar to React stateless functions we may be able to avoid having an entry for them in the instance map which is nice. This means we invoke the registered function with this bound to the worklet global scope.
Do you know how React detects if the component is statefull or stateless? I suppose it can do that by looking at the object prototype and seeing if it extends React.Component
(Is there anyway to create a statefull component without extending React.Component?). We may be able to do something similar if we require all animators to extend a base class.
From @appsforartists on September 28, 2016 16:48
It looks like they check for a render
method and a sentinel (isReactComponent
) that is set on the base class.
There is indeed a pre-ES2015 way to declare a component, React.createClass(dict)
. It appears they generate a class dynamically that descends from ReactComponent
, thus inheriting the sentinel and being easily distinguished from a pure function.
@vjeux, @gaearon, and @sebmarkbage understand the React internals better than I do.
From @sebmarkbage on September 28, 2016 20:55
We (React) check for a sentinel on fn.prototype.isReactComponent
of the function. FWIW, I'm not very happy with this design. Here's the alternative ideas we had and why we didn't choose them:
new
and then use duck typing on the return value since ECMAScript lets you return a different object. This doesn't work with non-object return values and unfortunately, you can't call new
on an arrow function.fn.prototype
object at all. This would mean that you could only use arrow functions or methods since they have a null
prototype. Unfortunately, this doesn't support plain functions nor transpilers that transpile arrow functions.fn.isReactComponent
since in ECMAScript classes static properties are inherited. Unfortunately, many other class systems that are used together with these environments doesn't support them (such as Scala.js).instanceof
but that relies on branding that doesn't work with multiple instances of the React libraries, polyfills or realms.Object.isConstructable(fn)
or Object.isClass(fn)
brand check or similar features. I like this the best but didn't want to go fully down that rabbit hole just yet.From @flackr on March 5, 2018 22:6
@majido the current API is such that we instantiate one instance per animation and consistently call animate with the same instance. However, I think the discussion in #87 has a lot of overlap with this goal, in that if we could recognize stateless animations we could run them speculatively and deduplicate multiple identical animations similar to paint worklet.
From @appsforartists on September 27, 2016 18:50
The animators in the README use the
class
keyword and static properties to construct an animation worklet. For animators that remember state across frames (#4), this seems reasonable. However, many animators will be pure transformations of input to output. For both readability/stylistic purposes and to avoid needlessly allocating memory/CPU cycles, it may be desirable to have an alternate representation for pure animators.Perhaps:
or
NOTE: The present API is also mutative - output is expressed by settings properties on the input, not by returning values. Some authors will prefer a truly pure API, but that's more a matter of style than of minimizing resource usage.
Copied from original issue: WICG/animation-worklet#14