search

cometd / cometd-nodejs-client

CometD client for NodeJS
Apache License 2.0
17 stars 7 forks source link

feat: type definitions, es modules usage #25

Closed t-richards closed 3 years ago

t-richards commented 4 years ago

This PR:

  1. Adds a set of TypeScript type definitions (.d.ts) to the library.
  2. Adds example code to the readme for using the library via ES modules.

Testing

How I tested these type definitions locally:

  1. Convert all test/ files from .js -> .ts
  2. Add @types/node, @types/mocha, ts-node, typescript packages
  3. Convert all top-level variables in tests from const -> let
  4. mocha -r ts-node/register test/*.ts
  5. Expect tests to pass!
sbordet commented 4 years ago

@t-richards thanks for the PR, but I would like you to open an issue first to discuss what is the problem first and what's the best way to solve it.

I don't see what the problem is right now? We don't use TS and I don't see why we should? I am genuinely asking, thanks.

t-richards commented 4 years ago

Comment moved --> #26.

sbordet commented 4 years ago

@t-richards yes please open an issue with your comment and let's continue the discussion there.

sbordet commented 4 years ago

@t-richards, since you've done it, would you contribute also (maybe another PR) at least one test (converted or new) in TS?

Can tests be run by mocha in "mixed" mode, i.e. some of them in TS and some in JS?

t-richards commented 4 years ago

@sbordet I believe that mocha can run mixed TypeScript / JavaScript tests.

That said, if the test conversion process from JS -> TS is somewhat straightforward, I think it might make sense to convert them all in one go and not bother with the mixed mode. I will try this out in a separate PR.