React-sketchapp: Typescript typings support

[ricokahler]

I am currently writing typings for react-sketchapp. I plan on publishing these typings to @types/react-sketchapp but an alternative to that is having the typings included in this repo itself. This is done by including a typings field in the package.json that references a typings file (e.g. react-sketchapp.d.ts).

Including the typings first-party enables better typescript support as well as better javascript support (without a typescript dependency) for editors that use the typescript language service.

Proposal:

If you give me your blessing, I'd like to fully type this library and include those typings in a pull, else it's going to @types/react-sketchapp.

[ljharb]

A DefinitelyTyped lib is the proper place to put it; typescript types don’t belong in a non-TypeScript repo, especially since there’s no way to prevent drift.

[ricokahler]

@ljharb would you be open to a full typescript port? I'd be willing to do it.

[ljharb]

No, Airbnb does not use TypeScript at this time, so none of our repos should be using it either.

[ricokahler]

Alright I'll publish the typings to @types sometime today and close this then I might add a pull for documentation changes to let people know it exists but that's all.

Thanks.

[ljharb]

That sounds fine, thanks!

[ricokahler]

@ljharb Here is the pull for the typings if you'd like to review them for correctness. https://github.com/DefinitelyTyped/DefinitelyTyped/pull/21637

I'll add the documentation pull after this pull gets accepted.

[jongold]

Sweet - agreed that we should keep this repo flow typed, but super appreciative of your work making this better for the typescript community :)

[ricokahler]

So I finished the typings and they got merged into DefinitelyTyped but I think I made a small error.

In the flow types.js SketchDocument.currentPage is typed without a getter but I see it used like a getter (e.g. () => SketchPage) from here. Is this type incorrect? I'm not familiar with flow unfortunately.

Anyway, the typescript typings I wrote mirror the flow types and I don't think it's working correctly.

Is the correct type for currentPage () => SketchPage?

[ljharb]

That indeed seems like the way it's used in the codebase, which suggests that this line has a bug.