EFI: Publish and maintain types

[sheplu]

Motivation

Types are widely used in the ecosystem, and requested as shown in the Next-10 survey last year. By providing them directly (or helping the community to provide them) would be a good help for the community. But this can be time consuming and not be perfect with versionning (based on typescript)

Expectation

Evaluate if we want to provide more type documentation using TS or JSDoc (or other solution)

Implementation

Status

Part: Technical

Draft

Types of the Express ecosystem are “at best” available and working. But they can be greatly improved. By automatically building and publishing types when we release a new version it would greatly help users. The community at scale rely more and more on typing (or the equivalent using JSDoc) and we should work to publish them automatically.
Add types inside package
Publish dedicated package

[UlisesGascon]

As far I know currently many people use https://www.npmjs.com/package/@types/express. Do we plan to contribute there or just integrate this into our code base?

IMO, anything that we can do to help our community to use Express is a good move, but I don't see a big value as maintainers to rewrite to TS as we already have JSDocs in place (for us and for the community). Also I am not very use to work with TS, so maybe all the plan is smoother than I imagine.

[timodempwolf]

Is there any advantage of using TS instead of JSDocs?
As far as I am aware tsserver works as well with JSDocs as it does with Typescript types, but I'm no expert on the subject.

[wesleytodd]

Do we plan to contribute there or just integrate this into our code base?

We are in a bit of a rock/hard place here. Because of their stance on semver I don't think we can really do either well. Since we are strongly committed to semver support, I almost think it is ideal that we take a hands off approach. That said, if we had folks who were really passionate about it and could make a clear case one way or another I would love it. Mainly because I agree that "anything that we can do to help our community to use Express is a good move".

how to actually deal with semver as Typescript still on the regular makes new releases that break existing types

Yep, this is fundamentally the problem. If we take ownership of them it become our problem. And while I agree we should do what we can to help folks using Express, I think a better way would be to JSDoc things and then let the folks on the TS/DT side generate their types from our JSDoc than to actually ship or build the types our self and deal with their bad decisions around semver.

[wesleytodd]

As I am thinking about this, maybe the way we could do this "best" is the following:

1. Add robust JSDoc type annotations to all our packages
2. Generate a .d.ts file and run a subset of tests against it (tests to specifically validate the types, not the behavior), but don't ship that for users
3. When those get updated, have some automation which pings someone from DT (need them to agree to that obviously) so they can update

[wesleytodd]

I was going through cleaning up other threads in here and found this one: #7

[ljharb]

The major advantages I see of sticking to DT instead of inline types are:

1. no semver conflation - the JS package doesn't get bumps only for type changes and vice versa, and breaking changes in types don't have to be major bumps in the packge
2. the TS team helps manage TS compatibility - it's complex to define what TS versions are supported and support multiples, and the DT repo/TS team just handles this

[fdrobidoux]

2. the TS team helps manage TS compatibility - it's complex to define what TS versions are supported and support multiples, and the DT repo/TS team just handles this

This shouldn't be an issue for legacy software. Active development would use a local version of TS instead (e.g. bundled with VSCode) even with legacy express apps.

[wesleytodd]

@fdrobidoux I don't follow. Can you explain what you mean? The issue @ljharb is talking about is that if we publish types along with our packages (instead of relying on DT) we then have to deal with all the version compatibility issues. If we let them handle it, they deal with it. Are you trying to say something else?

[ljharb]

Some extra commentary (some of which already exists here) in #203.

[wesleytodd]

pillarjs/router#100
pillarjs/router#110
pillarjs/router#107

I was cleaning up the router issues today and came across these. Do we think we need to decide on this before v5? I am still on the fence. I was going around asking folks if they wanted to work on landing things asap but I don't want to promise this unless we have a clear decision on doing this in repo or with DT.

[rafaell-lycan]

My 2 cents on this:

TypeScript brings better DX overall; JSDoc does not bring proper types IMO.

I'd rather have it on the same package if possible, like Fastify did, but devs already know that Express doesn't ship types anyway.

If the TC commits to keeping @types/express up-to-date on the next versions of Express with the community I don't see a problem.

[ljharb]

jsdoc checked by TS, with handwritten d.ts files, gives the same quasi-correctness that TS does.

[wesleytodd]

quasi-correctness

this is key.

JSDoc does not bring proper types

And neither does typescript if that is a thing you care about.

If the TC commits to keeping @types/express up-to-date on the next versions of Express with the community I don't see a problem.

We need a champion for this. If we have that I am no opposed at all to maintaining the types (either in package or in DT, depending on what the champion decides). The problem is we have never had anyone who signed up to do that. And to be clear, I don't mean signing up to re-write it in TS. I would block that or leave the project if there was consensus and I was the only objector.

[broofa]

[Edit: My apologies for the long comment. I recognize that I'm probably not moving the conversation forward all that much, and my conclusion is pretty much the same as that of @wesleytodd. I.e. That this needs a champion for it to be meaningful. This is just me processing my own thoughts on this issue, which I've dealt with numerous times in other projects.]

'Would love to see this made a priority but I fear it's a bit intractable. The problem is you can't really talk about supporting TS users without [eventually] accepting that the conversation has to be about porting a project's codebase to TypeScript.

Is that making a mountain out of a molehill? Yeah, somewhat. But this is a slippery slope that goes something like this...

Step 1: "La la la la. Not our problem!"

"Hey, we can just let "the community" solve this for themselves. That's what the Definitely Typed™ project is for!"

Nope, doesn't work. Oh, sure, @types/express will work for 90% of users. But there will always be small issues and inconsistencies that affect that long-tail 10%, especially if this express switches to a regular release cycle, which seems to be the plan. 'Problem is, the only place for those issues to get reported is here, in the express project. But good luck getting the @types/express ✌️ maintainers✌️ to step up and help out here. (Note: to the extent they do, we've moved on to Step 2 ...)

Step 2: "Okay, fine. It's our problem."

The type-related issues users file is a constant reminder that the buck actually does stop with express when it comes to TS types. So express is eventually going to have to accept responsibility for maintaining the DT types. But manually crafting types is a lot of work, as is meeting the DT project requirements for publishing new releases. Thus, this tends to be a short-lived solution that leads to a) tooling to automate creation of types, and b) bundling types with your project.

Farewell, Definitely Typed. We hardly knew ye.

Step 3: "Pretend we're a TS-project w/out actually being a TS project"

** sigh ** Fine, we'll bundle type declaration files with our project. And we'll somehow autogenerate them. But how?

Oh hey, look, JSDoc!

JSDoc sort of works. It's certainly tangibly better than steps 1 or 2 for a project like express (maybe?) But it just moves the manually-crafted nature of the types out of the *.d.ts files and into source comments. And JSdoc is it's own, different language that not all devs are familiar or comfortable with.

Ultimately, the problem with all three of these options is that type-related issues tend not to get discovered until after a release has been published. Fixing them requires more releases, leading to unnecessary code churn.

Step 4: "Actually become a TS-project"

... and so we arrive at the final form of the types problem.

Porting the project to TS kills two very big birds with one stone:

1. Solves the type declaration problem for real. Transpiling TS -> JS gives you type declarations for free. Problem. Solved.
2. TypeScript results in higher-quality code. Static type checking catches a plethora of errors that linting and unit tests simply don't cover. This is particularly important for an opensource project that takes as many drive-by contributions as express.

To my mind, debating the extent to which Definitely Typed should be involved or how/if JSDocs should be used is kind of postponing the inevitable. The only way of staving this off is if someone is willing devote significant time and effort to maintaining types by hand. Is someone here willing to be that person?

(Full disclosure: personally, I would view anyone volunteering for that with a bit of skepticism. This can't be one or two-off "sure, I'll do that" thing. They should be willing to be "on call" for type-related issues for the foreseeable future.)

[hichemfantar]

@RobinTail and @jakebailey have released new types for v5
perhaps the maintainers can contact them and gauge their interest in rewriting the project in TS?

[wesleytodd]

I am not sure if this conversation implies otherwise, but at this time we have no interest in re-writing express in TS. We have talked about it a few times and there is no consensus among the contributors on if the many sub packages could be written in TS or if there is any benefit to that. I know personally I do not think there is any benefit to authoring in TS. I do, on the other hand, think there is benefit in each package shipping their own types (but with many stipulations), but that is very different than doing rewrites.

[RobinTail]

rewriting the project in TS?

There is no need for that just to maintain types within express itself, @hichemfantar .
However, when the code is written in TS the types declaration can be generated automatically and it would be always in sync with the code.

[ljharb]

It can still be hand-written in d.ts, jsdoc-annotated in JS, and tsc ensures it's always in sync.

[bjohansebas]

I really don’t see why we should rewrite the packages in TypeScript. I keep running into more issues configuring it. I’d be happy to ship types directly in the package, it's something I’ve been considering for compression and session, but I’m still not sure if it’s viable.

[hichemfantar]

@RobinTail I'm suggesting a rewrite exactly to keep things in sync 100%, also the other typescript benefits mentioned by @broofa are quite valid

The types are already there, would a ts rewrite really be a challenge when the types already exist?

@wesleytodd typescript catches so many bugs that go undetected with plain js

@bjohansebas shipping types also works if there are no resources for a ts migration. i suppose you could simply copy the types from definitelytyped

[broofa]

@wesleytodd writes:

I know personally I do not think there is any benefit to authoring in TS.

I know this is neither the time or place for a debate on the merits of TS. Let me just say that as a once-upon-a-time TS hater, I am very much a convert. I've ported several codebases from JS -> TS (uuid most recently) and each time I am surprised at how many undetected bugs and questionable design choices it highlights. It also dramatically improves DX.

@ljharb writes:

It can still be hand-written in d.ts, jsdoc-annotated in JS, and tsc ensures it's always in sync.

Does it? I know you can use it to generate d.ts files from JSDoc, but my brief forays into this have given me the impression that it's pretty easy for JSDoc comments to get out of sync with the code.

Are there many mainstream projects that use this approach (to provide built-in d.ts files)? yjs is the only one I'm aware of.

@hichemfantar

The types are already there, would a ts rewrite really be a challenge when the types already exist?

My experience with TS rewrites is that the 80-20 rule definitely applies. 80% of the code is pretty easy to work through. But 20% will require significant thought and deliberation. For example, the current types make pretty liberal use of any, and that should probably be addressed. And building types with tsc will surface issues / incompatibilities with types in upstream dependencies that are probably flying under the radar at present.

And, too, maintainers need to develop the reflexes needed to properly handle type-related issues. (E.g. Does an issue that breaks types for TS users, but that doesn't affect vanilla JS users, qualify as a "BREAKING CHANGE"?)

[ljharb]

@broofa yep, i've published types in dozens of packages that way, it works great. tsc verifies them, because it checks the tests also.

[wesleytodd]

This discussion is exactly why we have not tried to deeply engage on this discussion. There is nearly 0 chance of a consensus among the maintainers here, and in that case the status quo stays. I am not going to engage on any more of a discussion of porting these code bases, but I will continue to be a proponent of us improving the experience for both authoring types and consuming express in TS code bases.

[RobinTail]

{} is a bad type, @nilsingwersen
https://www.totaltypescript.com/the-empty-object-type-in-typescript

[nilsingwersen]

{} is a bad type, @nilsingwersen https://www.totaltypescript.com/the-empty-object-type-in-typescript

{} by itself yes.

In this case it's good:

https://x.com/mattpocockuk/status/1823380970147369171

It makes the autocomplete work here, as far as I understand it. Please correct me if I'm wrong here.

[RobinTail]

@nilsingwersen , can do the same without {} in Webstorm.

Though, VSCode can not make it without {} for some reason

[casantosmu]

I'm also interested in contributing to this effort

[hichemfantar]

@nilsingwersen , can do the same without {} in Webstorm.

Though, VSCode can not make it without {} for some reason

i use the same thing for vscode, not sure if this is a limitation in vscode or if webstorm implements some workaround to make it work

[ljharb]

@RobinTail it's not a bad type, it's "object-coercible" - it's unknown minus null and undefined.

[gioragutt]

@nilsingwersen , can do the same without {} in Webstorm.

Though, VSCode can not make it without {} for some reason

Hi there, chiming in with some WebStorm/VSCode typescript differences experience, hope you don't mind 🙏🏻

WebStorm implements its own heuristics for TS.

From my experience it can often have deviations from the behavior in typescript itself (which is what VSCode uses and what I personally believe should be the source of truth).

If WebStorm does it and VSCode doesn't, it's likely cause WebStorm implemented this behavior in a non-standard way.

In the end - things may work in WebStorm but the rest of the editors/ides rely on TypeScript itself, so it'd make sense to VSCode as a reference, IMO.

[jakebailey]

@bjohansebas pinged me to chime in here.

If you're not planning on converting the project itself to TypeScript, you may not neccesarily benefit from leaving DT. There are some benefits that DT provides out of the box for you:

• DT tests against the last 10 versions of TS; we have a lot of tooling to make that work.
• PRs to TS are tested against all of DT to look for breaks. We do test other repos, but it's more challenging to verify that we aren't breaking things outside of "not adding new errors".
• The DT tooling itself gives you type testing along with a load of lint rules which (for better or for worse) don't really work outside of DT.

The best option is of course to rewrite in TS, in which case your test code will also be in TS and you'll have "full" type testing, more or less.

In lieu of that, moving the types directly into your package is of course beneficial to end users since they don't have to install anything, and you can update both at the same time. I don't think express' types are that extensive, either, just a couple of packages.

You'll probably need to recreate some of the testing that DT is able to do, e.g. testing against your own supported set of TS versions. You'll also want to be copying the DT types as-is, if you plan on adding types without breaking TS users (TS will use a package's types instead of @types). You'd also want to take the type tests and then set them up with something like tstyche, tsd, etc, to verify that your types still work.

Additionally, any package you reference in your publushed types must be added as full deps, not dev deps; that means express must depend on @types/send, @types/body-parser, @types/serve-static, for example, since its types reference that package. Not doing this will leave end users in a broken state since package managers will not auto install these types. With @types packages, they obviously do depend on other packages for types, but I'm sure someone will complain about "bloat", or something.

Happy to advise in any way I can (or, chat on the TS Discord's DT channel with other people involved in DT).

[RobinTail]

@RobinTail it's not a bad type, it's "object-coercible" - it's unknown minus null and undefined.

this is exactly what makes it bad, @ljharb — it's counterintuitive.
intersecting string with unknown minus null minus undefined does not seem like it's about to make a certain impact on the union of string literals.
Surely it does the hint in VSCode, but It's hacky and unclear when you read it, it's non-straightforward and therefore reduces maintainability.

[nilsingwersen]

The export type CustomHeader = string & {} is just an implemantation detail. What do you guys think about the idea of type safe headers, mime types etc. by itself. If that type hack is used or not I don't care. Just wanted to bring in the idea.

[RobinTail]

I don't mind having it being extracted into a self-descriptive generic type

/* It helps to display the given options in IDE despite accepting any string */
type AutocompleteString<Literals extends string> = Literals | (string & {});

[RobinTail]

What do you guys think about the idea of type safe headers, mime types etc. by itself

I see two problems here:

1. The list of headers is alive thing. New ones get added regularly, old ones get obsoleted. Therefore making it literal would require to maintain such a list, to keep it in sync with probably several sources.
2. The nature of Headers is that they are case-insensitive, so that Content-Type, content-type and even CONTENT-TYPE are all safe upon assignment, but some methods always return them in lowercase, which leads to confusion (see Express 4 and 5 req.get doesnt return header if not set in lowercase express#5288). Therefore it might require an extra effort to deal with it on the types level, @nilsingwersen

But the desire is a bright idea. MIME might be easier.

[ljharb]

@RobinTail that's a (reasonable) complaint about the type hinting behavior when {} is unioned with something, not a complaint about the type itself.

[bjohansebas]

By the way, the fact that this issue is marked as top priority doesn't mean it actually has priority; it's just to ensure the automation registers it in the agenda for work sessions so it can be discussed.

Also, this is not an issue about how types are structured, how they should be defined, or which type is bad. This issue is about outlining a plan to improve the experience for TypeScript developers.

The points raised by @jakebailey are important considerations when deciding whether to maintain types in the repositories or leave them in DefinitelyTyped.

whenever I have wanted to use a library with typescript, I have always liked it when libraries have integrated types. that's why i want to help with this.

Additionally, any package you reference in your publushed types must be added as full deps, not dev deps; that means express must depend on @types/send, @types/body-parser, @types/serve-static

If we add types to all those dependencies, then that problem would no longer exist, right? Therefore, we should start with the smallest dependency in express before applying it to express itself

Before proceeding with any work, I would like to ask the captains (@wesleytodd, @UlisesGascon, @jonchurch, @blakeembrey): Do you agree with adding types directly, just like any other functionality included in a package? This will be maintained by the community, and I know that some may not be entirely familiar with TypeScript. That's why creating a dedicated team for this would be important (similar to the triage team).

I would say that we don’t want to maintain compatibility with too many versions of TypeScript (at least recent versions, but we can discuss that—maybe a minimum of 4.x?). This would break TypeScript users, which means that type integration will be included in a major version.

Also, to be clear, there is no consensus to rewrite the libraries in TypeScript. Please do not attempt it.

[jakebailey]

TypeScript (at least recent versions, but we can discuss that—maybe a minimum of 4.x?)

DT's policy is the last 10 versions, so that's currently 5.1 and up. ts-eslint also follows this, more or less. I would definitely not suggest attempting anything more than that.

[bjohansebas]

I had thought of something else, thanks for the clarification. i think we should follow that.

[zspitz]

RE: the original question of JSDoc vs Typescript types, I highly doubt JSDoc could define types that would recognize an augmented request only after a given middleware has run. For example, where this would be a design-time error:

app.get('/', (req, res) => {
    res.status(200).send(req.user)
//                           ^ error
})

this would not:

app.get('/', authBuilder(true), (req, res) => {
    res.status(200).send(req.user)
//                           ^ no error
})

Typescript could handle this, given the appropriate Express types.

Related

[casantosmu]

RE: the original question of JSDoc vs Typescript types, I highly doubt JSDoc could define types that would recognize an augmented request only after a given middleware has run. For example, where this would be a design-time error:

app.get('/', (req, res) => {
res.status(200).send(req.user)
// ^ error
})
this would not:

app.get('/', authBuilder(true), (req, res) => {
res.status(200).send(req.user)
// ^ no error
})
Typescript could handle this, given the appropriate Express types.

Related

It can be done in .d.ts files

[zspitz]

It can be done in .d.ts files

Of course. I'm arguing here against JSDoc.

[ljharb]

jsdoc + tsc works fine. Non-tsc jsdoc isnt something i think anyone’s arguing for.

[zspitz]

jsdoc + tsc works fine.

Would JSDoc be able to define the types in the scenario I've outlined: unaugmented request before a specific middleware, and augmented after?

[ljharb]

Yes, tsdoc can do anything TS can (caveat; sometimes you'll want to define types in a .d.ts file and @import them in using jsdoc).

[raphendyr]

I have been working on express-session and I wanted to look if and how JSDoc and tsc can interplay. After a bit of iteration, I came to layout where the types of the implementation are manly in JSDoc comments, with some non traditional locations to change types and such. Interfaces and other parts supporting only the type system are in separate files, which are not .d.ts.

This setup seems to support using tsc to generate final .d.ts files for distribution, but also allow tsc to validate the internal code in the library.

Benefit of this over to just include .d.ts files is to have static type checking in within the repo.

Downside is that type unaware developers could get CI errors, if they don't update JSDoc types. Furthermore, I find that working with JSDoc types requires another level of mental gymnastics. In addition, there is very limited documentation and google help about the topic. And finally, tsc has quite many limitations on JSDoc types compared to typescript, which the developer needs to be familiar a bit too often.

I don't know if it's relevant or not, but as a contributor to a library, I benefit from the internal types. Static type checking helps me to catch some problems in the code very fast, which don't always have a good test even with 100% coverage. It also makes it hard to make most fragile JS tricks, which generally don't have a good reason in the first place, in my experience.

Any how, here is my Draft PR for the express-session repo. Note that it doesn't include everything needed. expressjs/session#1031

I hope this provides some examples to help the discussion.

[Roaders]

I confess that I have not read this entire thread but from what I have read there seems to be a concern about Typescript not following semver and there is concern around how express will work with this.

as a tldr I would say just don't worry about this. It would basically be impossible for Typescript to correctly follow semver. Pretty much every release would be a breaking change. It's a type checker so pretty much every change they modify this type checking to some extent. Almost all of these changes would break someone's code somewhere.

but....

this would not be your concern AT ALL as the publishers of a ts library.

express is from what I can see a small library (apologies if I am incorrect on that. When I looked it did not seem that there was huge chunks of code). They types would be very simple. As I mentioned in another comment thread there are many different level of Typescipt integration. I think that all you would do for express, at least initially, would be to rename the files to .ts and add types to the function parameters. "simple" typescript like this will not break when Typescript releases a new version. You really only run into issues with changing versions of ts when you do really fancy stuff with generics or computed types. I really do not think it will be an issue at all for this project.

Even if typescript DID break your types most likely this will not impact your consumers. A compiler option which is turned on by default is skipLibCheck - this means that any typing errors in the published d.ts files are ignored. Consumers won't see the errors but they will still be able to consume the types.

As far as I am aware jsdocs will not be sufficient. If you import anything into a TS file you get an error if it can't find any declaration files for it. At the moment to satisfy it we need to install @types/express but these types are often wrong.

IMHO (and I am obviously biased as a Typescript developer) the easiest and least maintenance way of doing this is to add types to your function params and publish the d.ts files from the code - that way we definitely know that the types are correct.

I may have a go at a PR converting the repo to TS just as an illustration of what the code would look like.

[bjohansebas]

#369