grantcarthew / node-rethinkdb-job-queue

A persistent job or task queue backed by RethinkDB.
https://github.com/grantcarthew/node-rethinkdb-job-queue/wiki
MIT License
157 stars 16 forks source link

Typescript type definitions. #66

Closed viridia closed 7 years ago

viridia commented 7 years ago

i can do some experiments to see if there's a way to avoid both of those. Again, some of this may be a result of my inexperience.

Modeling Jobs as a type has to deal with the fact that Jobs contain a mix of properties from both the application and the library. The obvious way to deal with this is through inheritance by using Job as a base type, which the user would extend. The question then is whether to make Job a class or an interface.

Making Job an interface seems like the right solution on first glance, since interfaces are purely compile-time entities that are overlayed on existing JavaScript objects. However, the downside of using an interface is that the user's actual concrete class would have to re-declare all of the properties in Job, because the compiler wants to check that the class implements all of the properties in the interface. So you basically have to duplicate the entire definition of Job, except putting 'public' in front of each member.

The alternative is to make Job an abstract class (in fact I probably should have used the 'abstract' keyword). However, in order to inherit from a class, it has to be a real value of some kind, i.e. Queue.Job has to exist as an actual thing.

One might ask why not move Job to the global namespace? Well, the problem there (and again this might be my lack of experience) is that index.js exports queue using commonjs syntax rather than export default, which has you know has slightly different semantics; And because of that, the index.d.ts file uses "export = Queue" rather than "export default Queue" in order to tell the typescript compiler to use commonjs semantics, which in turn means that you needs to use "import as Queue" rather than "import Queue" in user code. And it's hard to mix importing with importing other things.

That being said, it seems to work sometimes without me having to do this, and I'm not yet sure why. That is, for most of the time I didn't have Queue.Job = Job, and then there was a brief period where it stopped working, and then later started working again. Further investigation is warranted.

I'll go ahead and remove the package.json dependency, it's only there to make my text editor's linter happy.

grantcarthew commented 7 years ago

Ok, thanks @viridia

I was under the impression we just needed the .ts and config files. Not keen on changing functional code to support a superset language. The package.json still has the @types/node dependency.

viridia commented 7 years ago

Sorry, since I switched over to using yarn I sometimes forget how to use npm :)

grantcarthew commented 7 years ago

Good stuff. Thanks @viridia