Add Typescript definitions

[nlochschmidt]

While trying to integrate promster with our service written in TypeScript I noticed that there don't seem to be typings for @promster/express. So needless to say, I'd love to see Typescript support for this.

[tdeekens]

I‘d love to see them too. Ideally we would rewrite the library to TS to avoid outdated types if forgotten to be updated.

Do you have any intention or see an ability of opening a PR for this yourself?

[nlochschmidt]

If the API doesn't change often, then a rewrite only to keep TS definitions in sync, is probably not the best use of anyone's time 😅, right?

[tdeekens]

In a way no. In another it's quite risky. I've had experience with thinking exactly that in the past in libraries. Then outdated types are frustrating.

[nlochschmidt]

TypeScript 3.7+ supports generating type definitions from JSDocs. I could try that out and open a PR if it works well.

Would that be useful?

[tdeekens]

That sounds like a good compromise. Thanks!

[nlochschmidt]

I did a proof of concept for generating the declarations based on https://voxpelli.com/2019/10/use-type-script-3-7-to-generate/

The result is in this branch: nlochschmidt/promster@master...nlochschmidt:typescript-definitions

To generate the type declarations (and run the typechecker) you need to run yarn build:declarations.

The declarations look a bit ugly though. TS creates a lot of *.d.ts files and merging them using the outFile option sadly doesn't let Typescript import the generated definitions from one module (metrics) to another (express) for some reason.

[nlochschmidt]

Funny enough there seems to be a problem with the defaultedOptions here:

promster/packages/express/modules/middleware/middleware.js

Line 28 in 575cca7

createRequestRecorder.defaultedOptions,

could it be that those should be defaultOptions according to:

promster/packages/metrics/modules/create-request-recorder/create-request-recorder.js

Line 125 in 575cca7

createRequestRecorder.defaultOptions = defaultOptions;

[tdeekens]

Funny enough there seems to be a problem with the defaultedOptions here:

This to me also looks wrong. Odd that it never surfaced as a bug. Will have a look and fix independently quickly if needed. Thanks.

TS creates a lot of *.d.ts

You mean one d.ts file per module we add comments to? That is relatively normal. Only if we'd write one large type definition across modules we'd have it in one file. If we look at the types of flopflip here (another library of mine) which is fully written in TypeScript, we can see that we also got a symetry between the modules written in TypeScript and the d.ts files.

[tdeekens]

I released a fix for the mentioned option defaulting across packages

Successfully published:
 - @promster/[email protected]
 - @promster/[email protected]
 - @promster/[email protected]
 - @promster/[email protected]
 - @promster/[email protected]
 - @promster/[email protected]

[tdeekens]

Here is a PR rewriting the library entirely to TypeScript. Little Saturday lockdown activity 😄. Not 100% perfect yet but pretty good as a start I think. THe hapi plugin is notoriously hard to type as it mitigates a breaking hapi version.

[nlochschmidt]

Thanks a lot for your work 👍