Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EFI: Publish and maintain types #192

Open
sheplu opened this issue Feb 21, 2024 · 29 comments
Open

EFI: Publish and maintain types #192

sheplu opened this issue Feb 21, 2024 · 29 comments

Comments

@sheplu
Copy link
Member

sheplu commented Feb 21, 2024

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
Copy link
Member

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.

@dougwilson

This comment was marked as outdated.

@timodempwolf
Copy link

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
Copy link
Member

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
Copy link
Member

wesleytodd commented Feb 22, 2024

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
Copy link
Member

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

@ljharb
Copy link
Contributor

ljharb commented Feb 23, 2024

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
Copy link

  1. 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
Copy link
Member

@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?

@wesleytodd wesleytodd mentioned this issue Feb 29, 2024
@ljharb
Copy link
Contributor

ljharb commented Feb 29, 2024

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

@wesleytodd
Copy link
Member

wesleytodd commented Mar 16, 2024

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
Copy link

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
Copy link
Contributor

ljharb commented Apr 17, 2024

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

@wesleytodd
Copy link
Member

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
Copy link

broofa commented Sep 15, 2024

[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
Copy link

@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
Copy link
Member

wesleytodd commented Feb 4, 2025

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
Copy link

RobinTail commented Feb 4, 2025

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
Copy link
Contributor

ljharb commented Feb 4, 2025

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

@bjohansebas
Copy link
Member

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
Copy link

hichemfantar commented Feb 5, 2025

@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
Copy link

broofa commented Feb 5, 2025

@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
Copy link
Contributor

ljharb commented Feb 5, 2025

@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
Copy link
Member

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.

@hichemfantar
Copy link

hichemfantar commented Feb 12, 2025

I suppose the best solution is to move the types internally, this would make express ts ready without third party deps.
is this an acceptable solution to the team?

@RobinTail
Copy link

would a ts rewrite really be a challenge when the types already exist?

I took a look on it, @hichemfantar , and it contains some substantially complicated statements doing tricks with prototypes, which I'm not sure how to express in Typescript correctly. For example:

// request.js, line 30
var req = Object.create(http.IncomingMessage.prototype)

This is not really a class instance that extends IncomingMessage, because IncomingMessage requires Socket as an argument of its constructor which is not even called. This is like a new object that absorbs methods of IncomingMessage class as a mixin 🎩
So... I believe rewriting it using Typescript would look messy and moreover, could introduce new bugs.
Therefore I suggest to keep the implementation as is, but just move the types declaration (also as it is) from DT into express repo.

@hichemfantar
Copy link

sure thing @RobinTail that's the conclusion i've arrived at too
if @wesleytodd would be okay with this then a PR can def be created

@bjohansebas
Copy link
Member

Before keeping the types directly in the repository, I think we should have a types team responsible for maintaining them and resolving any issues that may arise in the future.

@hichemfantar
Copy link

I volunteer, maybe @RobinTail would be interested as well

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests