add typescript definitions and replace jsdoc with typedoc

[rochdev]

This PR adds TypeScript definition files to officially support TypeScript out of the box instead of having to rely on @types/dd-trace. It also has the side effect to improve even JavaScript code completions in some IDEs such as Visual Studio Code. Now that we have typings, I've also replaced the current API documentation with an improved version based on typedoc.

To view a preview of the final docs, run:

yarn install
yarn typedoc

and then open ./out/index.html in a browser.

[brettlangdon]

Should we just have typedoc be a devDependency of the main package.json?

Or maybe add a script to the docs/package.json for running typedoc?

Right now if I clone and run yarn typedoc I get an error since typedoc is not available to yarn via the main package.json.

I can't figure out if that sentence makes sense.

[rochdev]

Should we just have typedoc be a devDependency of the main package.json?

The idea is to keep the dependency out so that we can better parallelize the build without unnecessary dependencies being pulled.

Right now if I clone and run yarn typedoc I get an error since typedoc is not available to yarn via the main package.json.

That is a bug that I'll submit a new commit for.

[perryh]

hi @rochdev,

I'm new to typescript. In index.d.ts, why aren't any of the type definitions exported? In my code, for example, I would like to specify that a variable is of type SpanOptions, but cannot because I don't have access to that interface.

Thanks!

[rochdev]

@perryh This is something I need to fix in the PR before we can merge it. In the meantime, I would recommend to take a look at @types/dd-trace.

[alloy]

Awesome! ✨

[alloy]

It is recommended (in the TS/DefinitelyTyped realm) to not use namespace. Moreover, the TS support in Babel 7 does not support it at all. When people really want namespaces, it's usually solved by utilizing multiple modules.

[rochdev]

I was not sure which one should be used, but decided to go with namespace as it seems to be the construct recommended for "internal modules" according to the docs.

Do you have any good reference on the difference between the two and when one should be used instead of the other?

[alloy]

It's hard to find a clear doc that states you should favour modules over namespaces and I'm sure there are still valid reasons to use namespaces. I personally follow the rule because it's what the DT repo uses through TSLint's recommended settings (which includes https://palantir.github.io/tslint/rules/no-namespace/).

When it comes to truly modelling a module, using module (or separate files to reflect modules) is a direct representation of reality. i.e. I could, if I wanted to, import one of the plugin modules. If I wanted TS to know about types of that module, using namespaces wouldn't make that happen. If you don't really want people to import modules, which seems to be the case here (?), then there's also no real point in modelling modules somewhat. Instead I'd just define an interface for each plugin that models the plugin config object you can pass to init, like we did in the DT types for this lib.

[alloy]

Perhaps interesting, yet still implicit, is this from the docs:

Starting with ECMAScript 2015, modules are native part of the language, and should be supported by all compliant engine implementations. Thus, for new projects modules would be the recommended code organization mechanism.

TS always wants to follow the standard but sometimes experiments with future possible additions to the spec. From reading this snippet and various 3rd party recounts it seems like this is what happened with namespaces and they are no longer encouraged.

[alloy]

cc @DanielRosenwasser do you have any input / know of a definitive document on this topic?

[alloy]

Yeah, the Babel part is my main concern too. However, I’m now realizing that as long as you don’t write the libs source in TS it shouldn’t really matter. Babel doesn’t do any type-checking, for that you’re supposed to use tsc, so in this case the d.ts file would simply be ignored and it should all be fine.

As for the example of exporting eg constants from the plugin modules, in that scenario I would actually be more inclined to use modules over namespaces, because it means users will be able to import those exports from the modules directly, instead of only through the main module of the package.

[rochdev]

I see. So basically for an import such as import "dd-trace/plugins/express" for example, it would work if there is a file named express.d.ts? I assumed this was only possible for actual .ts files only.

As a side note, I usually prefer to export everything from the main module to avoid the API being bound to the file structure, but I'm open to export the files directly if it's deemed useful.

[alloy]

Correct, or you could use the declare module “some/path” { } declaration in your index.d.ts file.

You can ofc still also export them from the main module, but I like letting the decision up to the user as more than once have I had an exotic reason to only import a subset of a package.

[DanielRosenwasser]

If the original code uses a namespace pattern for organization, namespaces are fine. New code probably shouldn't be authored using namespaces, but namespace is probably fine in .d.ts files.

[rochdev]

I will keep namespace for now and will update later if it causes any issues.

[rochdev]

@alloy Thanks for looking into this PR! I'm by no means a TypeScript expert so any help is appreciated!

[alloy]

Sure thing!

One thing that comes to mind is that, because you don't actually use TS, your type defs aren't being exercised, so it might be a good idea to add a ‘test’ like we have in the DT version: https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/dd-trace/dd-trace-tests.ts

[perryh]

@alloy hi :) should the type declarations from @types/dd-trace be exported? I'm having trouble referencing the types in my code.

only the trace variable is is exported: https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/dd-trace/index.d.ts
another library in my project that I can reference types from w/o issue: https://github.com/brightcove/hot-shots/blob/master/types.d.ts
thanks!

[rochdev]

One thing that comes to mind is that, because you don't actually use TS, your type defs aren't being exercised, so it might be a good idea to add a ‘test’ like we have in the DT version

@alloy I'll definitely add tests. So far I've been testing the types manually, but of course this doesn't scale at all.

[alloy]

@perryh Correct, currently really only the trace instance is exported. There’s this PR that exports more, but it needs some revision first DefinitelyTyped/DefinitelyTyped#32606

[rochdev]

I would like to specify that a variable is of type SpanOptions, but cannot because I don't have access to that interface.

@perryh In general simply creating an object of any type (including literal) with the correct properties should just work. Are you having any specific issue?

[perryh]

I would like to specify that a variable is of type SpanOptions, but cannot because I don't have access to that interface.

@perryh In general simply creating an object of any type (including literal) with the correct properties should just work. Are you having any specific issue?

Without exports, I'm not able to use them in declarations. For example, I have to declare this variable as any instead of Scope:

// tslint:disable-next-line:no-any
  const scope: any = tracer.scopeManager().active();

Thanks!

[brettlangdon]

> node --version
v10.15.0
> yarn type:test
yarn run v1.13.0
$ cd docs && yarn test
$ tsc --noEmit test
test.ts:119:17 - error TS2585: 'Promise' only refers to a type, but is being used as a value here. Do you need to change your target library? Try changing the `lib` compiler option to es2015 or later.

119 const promise = Promise.resolve();
                    ~~~~~~~

error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

[brettlangdon]

Should we add documentation to these methods?

[rochdev]

This is actually a bug in typedoc where it should use the documentation from the parent. I'll try to add our own doc for now and figure out why this is happening later.