typescript errors in 2.2.4

[timothyallan]

Getting a whack of TS errors now when using 2.2.4 and TypeScript 2.3

node_modules/react-draggable/index.d.ts
(49,1): error TS1046: A 'declare' modifier is required for a top level declaration in a .d.ts file.

node_modules/react-draggable/index.d.ts
(49,7): error TS2323: Cannot redeclare exported variable 'Draggable'.

/node_modules/react-draggable/index.d.ts
(49,66): error TS2304: Cannot find name 'DraggableState'.

node_modules/react-draggable/index.d.ts
(60,5): error TS2484: Export declaration conflicts with exported declaration of 'DraggableCore'.

[lostfictions]

Bit by this as well! Just to be clear, it totally breaks compilation for anyone using the package with TypeScript (and probably throws some weird stuff for anyone using the package with vanilla JS or Babel in VS Code or other IDEs that consume TypeScript defs.) The only way to get it compiling again is to delete the .d.ts file from node_modules, which obviously doesn't work for redistribution.

I'm only just trying react-draggable with TypeScript for the first time today, but looking at the definitions I'm not sure how they ever worked since they reference an identifier that doesn't exist.

Don't have time to submit a PR today, but here's a gist of a fixed .d.ts file.

I could also submit this as a PR to DefinitelyTyped instead and react-draggable could cut a new release with the definitions removed from this repo? I really appreciate when maintainers are willing to host definitions in the repo since it reduces friction for consumers of the package (and automagically improves completions in editors like VS Code even for vanilla JS!) That said, I think it might only makes sense to host definitions in-repo if maintainers are up for keeping them up-to-date and testing against TypeScript when the API changes.

[STRML]

Thanks @lostfictions. I've published v2.2.5 which fixes this issue.

I use Flow in our own projects, not TS. Perhaps your DT idea is better. But if I wanted to ensure that the TS definitions are correct in the future and maintain this definition, do you have suggestions on how to add that to the test suite?

[lostfictions]

Thanks a lot for the quick turnaround!

I'm not 100% sure there's an established best practice to test TypeScript defs against a regular test suite. (This is actually a broader problem: right now DefinitelyTyped requires tests for definition files, but last I checked their runner just checks that the test code passes the typechecker -- it doesn't actually run any of the tests to verify that definitions match up with real code!)

That said, a really nice feature in the latest version of TypeScript is the ability to typecheck vanilla JS files (just like Flow has always been able to do.) That means it should now be possible to quickly verify definition files against real code by including the test suite in a typechecking pass. Transitively, if the definitions pass and the tests pass (and they have decent coverage!) you have pretty good assurance that the definitions are correct.

In the case of react-draggable, this should be pretty simple: install TypeScript as a devDependency and add a line to the makefile under the invocation of Karma that runs the TS compiler in no-emit (typecheck-only) mode and includes the react-draggable definitions and the spec file.

It might take a bit of finessing to set up the tsc command (eg. it's probably simpler to have it ignore the types of all the other packages in the spec) but I could try to make a PR for this if that sounds good?