mocha icon indicating copy to clipboard operation
mocha copied to clipboard

🚀 Feature: TypeScript .d.ts type declarations

Open boneskull opened this issue 4 years ago • 18 comments

AFAIK, it's recommended by both the TypeScript team and DefinitelyTyped (the people that publish everything under the @types scope on npm) that packages ship their own type declarations (.d.ts files). I think this is essentially for two reasons:

  1. It's tough to maintain types for a package you don't control
  2. The package owners are the best people to understand what the types are

I was on the fence about this for awhile, because I wasn't interested in trying to manually maintain declarations.

Recently, TypeScript introduced support for generation of type declarations from JavaScript sources.

I've used this feature pretty extensively (see report-toolkit) and have had good success with it. The way it works is this:

  1. Use JSDoc-style comment tags, where appropriate, which TS can understand.

    Mocha is already most of the way there, I think. Depending on the extent of the type inference, we may be able to restrict it to a subset the source code (initially, the user-facing, non-programmatic APIs), instead of needing to meticulously work through the entire codebase under lib/, but it's hard to say until we try it.

    Custom types (describing the shape of a plain object or the signature of a callback) are accomplished via @typedef. These are equivalent to TS' type keyword and subject to the same limitations.

    One place where we may get tripped up is that TS is unable to understand prototypal inheritance when not using the class keyword. So, though Test extends Runnable, we won't be able to illustrate that via the declaration--but if we focus on non-programmatic usage, this is moot.

    We need to make sure we're not breaking our API doc generation, though. I don't think this will be much of an issue, as that's mainly focused on the programmatic API. We can revisit the programmatic API at a later point.

  2. Use tsc to generate a .d.ts artifact from the sources we care about. Make it part of the build process; it doubles as a "type linter", because if something is incorrectly typed, the build will fail. Publish the resulting file(s) and keep them out of VCS.

Comments? Hate the idea?

boneskull avatar Jan 15 '20 17:01 boneskull

I'm super happy to see the enthusiasm here! I'd be a bit cautious though because depending on how your types differ, users may see some differences (at least initially) or have dependencies on how the types are declared. I think it's still worth trying to compare/contrast though!

I'd also heavily recommend using checkJs to validate that your types are correct as you do this too - not just using allowJs

DanielRosenwasser avatar Jan 16 '20 08:01 DanielRosenwasser

(also we're looking for feedback!)

DanielRosenwasser avatar Jan 16 '20 08:01 DanielRosenwasser

You'd probably wanna try to make sure your API that's generated is compatible with the one exposed by @types/mocha, at least, since once you ship one, it'll override that one; so to make the transition easy, making the type names and structure the same as what's currently documented on DT would be useful.

weswigham avatar Jan 16 '20 20:01 weswigham

@weswigham Meaning that if someone has @types/mocha installed, and we start shipping declarations, TS will just ignore the @types/mocha ones?

boneskull avatar Jan 16 '20 21:01 boneskull

Yep. A design decision made a long time ago was that a package's own declarations take priority over declarations from the @types namespace - it's more of a fallback than an override.

weswigham avatar Jan 16 '20 22:01 weswigham

I had a look at DefinitelyTyped - Mocha, it supports Mocha v5.2 and takes about 3000 lines. To keep this file up to date seems to be a huge task.

We should start improving our JSDoc for that purpose, but not ship yet any mocha.d.ts file with our package or override @types/mocha. The past few months it has been difficult enough to publish a release or just to get a review for a PR. So I'm afraid of paralyzing ourself with any additional task we are trying to fulfill.

If we start shipping @types, the quality has to be excellent, otherwise we get flooded by issues we are not able to handle.

@cspotcode was involved in @types/mocha. His point of view would be interesting.

juergba avatar Jan 17 '20 15:01 juergba

Custom types (describing the shape of a plain object or the signature of a callback) are accomplished via @typedef.

We should also be able to sprinkle type-only .ts files into the code for convenience. We can declare types in these files using TS interface / type syntax, then import them elsewhere.

We can use the DefinitelyTyped tests to confirm compatibility. https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/mocha/mocha-tests.ts

Can mocha start shipping its own experimental .d.ts in an opt-in, non-automatic fashion? Consumers would choose to use mocha's bundled declaration if they want, but it would not override @types/mocha. This would be a good way to get feedback and squash bugs.

otherwise we get flooded by issues we are not able to handle.

The Mocha team should have a list of people interested in bugfixing the declarations. Whenever declaration bugs are filed, those people can be pinged and asked to write a PR. I can be on that list to start.

Is mocha's release process automated? If it's straightforward to publish patch releases, this'll be much easier.

cspotcode avatar Jan 17 '20 15:01 cspotcode

How would you ship “opt-in” types?

boneskull avatar Jan 17 '20 16:01 boneskull

@juergba

To keep this file up to date seems to be a huge task.

I don’t want to manually maintain a declaration file either.

That’s the idea: it’s generation would be automated from our jsdoc docstrings, of which we have quite a bit already.

but it’s also why this is labeled nice-to-have

boneskull avatar Jan 17 '20 16:01 boneskull

How would you ship “opt-in” types?

I haven't verified that this works, but I have an idea. Maybe the TypeScript team knows if it's do-able.

We could ship a .d.ts that does declare module "mocha" {... but is not referenced in our package.json's "types" field.

If a mocha user wants to opt-in to our bundled types, they have to manually refer to that d.ts file via their tsconfig or a /// <reference path="" /> pragma. If the bundled types are broken, they can remove the pragma and install @types/mocha. Hopefully they do this after filing a helpful bug report. :)

EDIT: to clarify, during this experimental phase, I imagine the types would only be used by adventurous users who know how to configure opt-in and how to help us diagnose any bugs. The goal is to improve the types enough that they can eventually be automatic, and the DefinitelyTyped declarations can be deprecated.

cspotcode avatar Jan 17 '20 16:01 cspotcode

I switched on checkJs and it's already finding bugs / code smells. It's cool to see this pay off in other ways.

For example, https://github.com/mochajs/mocha/blob/master/lib/cli/run-helpers.js#L85 doesn't match https://nodejs.org/api/fs.html#fs_fs_existssync_path

cspotcode avatar Jan 17 '20 18:01 cspotcode

I'm at a stopping point with #4156 for the moment. It builds the declarations and tests them against mocha-tests.ts from DefinitelyTyped. They fail, so there's still work to do. It's a matter of tweaking the JSDoc, fixing typos in mocha's codebase, and identifying bugs like https://github.com/microsoft/TypeScript/issues/36270 if/when they exist.

cspotcode avatar Jan 17 '20 22:01 cspotcode

Has anyone had a chance to look at this? Do you have any questions?

cspotcode avatar Feb 05 '20 17:02 cspotcode

Would you consider accepting a PR converting mocha's own code to (strict) native typescript?

A PR that would typecheck mocha's own code, generate .d.ts from sources, and ensure compatibility with @types/mocha.

AviVahl avatar Nov 24 '20 23:11 AviVahl

Coming back to this a few years later: the general TypeScript community consensus is now that package should only ship their own types if they're able to generate them automatically. As much as I love TypeScript, per #5027 we're not looking to do any major changes like switching the source code language any time soon.

This issue is blocked on us:

  1. Getting much more familiar with Mocha
  2. Filing -> discussing -> approving a separate issue about converting to TypeScript (which is not guaranteed to happen)
  3. Creating & merging a TypeScript PR

JoshuaKGoldberg avatar Dec 27 '23 23:12 JoshuaKGoldberg

Note that global declarations are not auto-included for non @types/ packages by default.

As such, if Mocha shipped its own types and you as a consumer wanted all its global declarations included in your project, you'd have to now add something like

{
  "compilerOptions": {
    "types": ["mocha"]
  }
}

That might be considered a best practice, but it's going to break a lot of existing projects relying on automatic types inclusion.

DanielRosenwasser avatar Dec 28 '23 10:12 DanielRosenwasser

I personally had the integration of the types of mocha into mocha itself on my agenda, as we do it in fastify. I am not aware of a consensus, that only projects, which generate the typescript types by itself should ship them with the project. @JoshuaKGoldberg Do you have some links regarding this.

@DanielRosenwasser Thank you for making me aware of this potential issue. Do you have some more links which document this behaviour?

Uzlopak avatar Dec 28 '23 10:12 Uzlopak

a consensus, that only projects, which generate the typescript types by itself should ship them with the project.

From https://www.typescriptlang.org/docs/handbook/declaration-files/publishing.html:

If your types are generated by your source code, publish the types with your source code. Both TypeScript and JavaScript projects can generate types via declaration.

Otherwise, we recommend submitting the types to DefinitelyTyped, which will publish them to the @types organization on npm.

JoshuaKGoldberg avatar Dec 28 '23 16:12 JoshuaKGoldberg