svgdotjs / svg.js

The lightweight library for manipulating and animating SVG
https://svgjs.dev
Other
11.14k stars 1.08k forks source link

#1106

Closed pragdave closed 4 years ago

pragdave commented 4 years ago

It was difficult to use the zero-parameter form of Timeline.schedule() with TypeScript. The svg.js.d.ts file didn't include the return type for a list of runner infos, and adding it still made it harder than it should be to iterate over it type safely.

In this commit I:

  1. Added the type information for ScheduledRunnerInfo, and updated the Timeline types to include it as a return type for schedule

  2. Added a new function to Timeline, getSchedule, that simply returns the runner list. This seems to be cleaner than having the original schedule that can return two wildly different things.

I didn't remove the old functionality, so schedule() is still compatible. I suggest we might want to deprecate that old form at some point.

coveralls commented 4 years ago

Coverage Status

Coverage remained the same at 99.924% when pulling 66094c88a8c6452cbc0943af282ba9757b5f5421 on pragdave:some-type-based-tidying into c8cb22863bf8c3ac157f6098be9154908aea9ec2 on svgdotjs:master.

Fuzzyma commented 4 years ago

Hm, it is quite common in svg.js to get and set a value with the same function. However, you dont write this | Number as return type but instead you have an overload. Otherwise you get back a union type and typescript doesnt know if its the one or other type. So writing it seperately would actually solve your issue:

        schedule(runner: Runner, delay?: number, when?: string): this
        schedule(): ScheduledRunnerInfo[]

I also made the runner non-optional (which is correct). It is hard to keep the ts file always up to date but I try my best :).

As for the PR: I would rather not introduce a new method here. I would accept the PR without the new method and with the suggested changes I made in this post :)

pragdave commented 4 years ago

re the union: Oh, of course

pushed

Fuzzyma commented 4 years ago

Perfect! Thank you!