dojo / widget-core

:rocket: Dojo 2 - widget authoring system.
http://dojo.io
Other
34 stars 39 forks source link

AnimationProperties interface needs changing to string enum #802

Closed tomdye closed 6 years ago

tomdye commented 6 years ago

Bug

current typing for AnimationProperties has fill and other fields defined as:

fill?: 'none' | 'forwards' | 'backwards' | 'both' | 'auto';

But when trying to use it with fill: 'forward' it is throwing a type error. There should all be changed to use string enums instead.

kitsonk commented 6 years ago

As discussed offline, this isn't really a bug, it has to do with how the properties are create and their type inferred.

So if anything it would be an enhancement.

agubler commented 6 years ago

I guess although this isn’t a bug in Typescript, could be argued it is a bug in the usage intended for the animations meta

tomdye commented 6 years ago

Agreed, I think this needs working on

tomdye commented 6 years ago

Looks like @types/web-animations-js uses type AnimationEffectTimingFillMode = "none" | "forwards" | "backwards" | "both" | "auto"; but it is not exported. When converting our AnimationTimingProperties object to use a string enum the following error is thrown:

src/meta/WebAnimation.ts(27,4): error TS2345: Argument of type 'AnimationTimingProperties' is not assignable to parameter of type 'number | AnimationEffectTiming'.
  Type 'AnimationTimingProperties' is not assignable to type 'AnimationEffectTiming'.
    Types of property 'direction' are incompatible.
      Type 'AnimationDirection | undefined' is not assignable to type '"normal" | "reverse" | "alternate" | "alternate-reverse" | undefined'.
        Type 'AnimationDirection' is not assignable to type '"normal" | "reverse" | "alternate" | "alternate-reverse" | undefined'.