Solid Node.js with official type system support

gengjiawen commented 4 years ago

Is your feature request related to a problem? Please describe.

While js became more and more popular, typescript and flow appears to deal with stability and maintainability for large projects, yet we lack support for type definitions for Node.js core api. I think the needs goes strong and strong, one proof is that https://www.npmjs.com/package/@types/node has reached 23 million download weekly.

Community already has @type/node, should we really support it in core ? I think we should, here are the reasons:

It's not reflect the api change quickly enough

For example, the wasi api takes some time added to the repo. Apis like newly or changed takes time to get adopted, developers will has to wait for this.

It's not related to Node.js version consistently

this package doesn't related Node.js versions. But we has lts, latest and other versions, this can be problematic and got surprised behavior. If we have official support, developers can choose the related version and got precise result.

other

It's only for typescript. We can expand our official type system to flow or other newly solutions. Also contribute to this package is too much pain.

Describe the solution you'd like

I am thinking we generate types file from our markdown doc or js source file (the libs folder ), not sure whether this eventually can work. Another solution is we invent a new dsl :)

Eventually we should make should publish types package when we release a new Node.js version.

himself65 commented 4 years ago

jsdoc to typescript must a good one solution

example: https://www.npmjs.com/package/tsd-jsdoc

tniessen commented 4 years ago

Are you proposing a separate, officially maintained npm package, or bundling it with Node.js somehow?

devsnek commented 4 years ago

I would be against anything that requires node core maintainers to have knowledge of typescript (for example requiring us to update the types with our other core changes). Beyond that, I have no issues.

ronag commented 4 years ago

I'm a little sceptical about this kind of thing unless we actually write in e.g. TypeScript. From my experience this quickly becomes outdated and incorrect causing both confusion and a false sense of safety.

tniessen commented 4 years ago

From my experience this quickly becomes outdated and incorrect causing both confusion and a false sense of safety.

Isn't that the issue that @gengjiawen is trying to address with this proposal?

ronag commented 4 years ago

Isn't that the issue that @gengjiawen is trying to address with this proposal?

I'm sceptical about the proposed ways to address this.

generate types file from our markdown doc or js source file (the libs folder ), not sure whether this eventually can work. Another solution is we invent a new dsl.

Unless we actually write the code in e.g. TypeScript I don't believe they will ever be entirely correct, which for me defeats the purpose.

jasnell commented 4 years ago

I'd be much more in favor of bringing @types/node into the Node.js organization but maintaining it as a separate project. I would not want to bake it in to core. That said, we could be doing a lot better at documenting core types to make it easier to generate and maintain.

tniessen commented 4 years ago

Oh, that's what you mean. I agree, there's a tendency for types and code to diverge, whether it's in core or not.

I have been using @types/node for a long time, always using the most recently published version. Maybe it makes more sense for us to get involved there, than trying to achieve the same in Node.js core?

Flarna commented 4 years ago

I'm also skeptical that separate releases/repos can really solve this problem.

@types/node has just a handful of active maintainers continuously looking into it. There are quite some additional people raising PRs to fix something. Not sure if this would improve just by moving it into node organization.

DefinitelyTyped provides quite an infrastructure for testing and deployment to NPM for easy consumption. Decoupling node definitions from the >1000 dependent definitions there may easily result in breaking some of them even with simple changes.

fyi @SimonSchick

SimonSchick commented 4 years ago

@Himself65 The last time I've seen a larger project (puppeteer) try to ship their own typings generated from typedoc it failed horribly as they were very inaccurate, didn't cover edge cases and don't even get me started on generics. Suffice to say they rolled that back pretty quickly.

As a long time @types/nodejs maintainer I feel the pain though as I usually try to roll out new type versions relatively quickly to stay up to date. I often have trouble translating changelogs into type definitions due to silly quirks (like assigning properties to function to export them, circular references etc.). I also often miss changes as they aren't mentioned in the changelogs and going through the commits indivudually is a huge PITA.

I think making the contributors/maintainers write typings would often lead to highly consistent definitions and act as a double check for more sane design decisions (eg. if you can't model it in TS easily, you are likely doing something 'hacky').

I also concur with @Flarna though, moving the typings would be quite an effort. It would probably be possible to move the type defs into node and then automate PRs into DT for checking/releases. That adds quite a lot of work to releases and might cause all sorts of problems however.

Mickael-van-der-Beek commented 4 years ago

As a long time Node.js user, my experience with the @types/node type definitions was that they were decent for the common function arguments but quickly broke down for lesser known features.

e.g: the custom lookup function that can be passed to http.connect() and net.connect() e.g2: exposed APIs like the HTTP parser (process.binding('http_parser').HTTPParser)

A lot of types are also not very strict, in the sense that an encoding option is a string and not say an enum type with utf8, binary, etc. as possible values.

Of course adding type definitions for internal bindings (which are technically public) and the level of strictness of the type definitions is subjective.

mcollina commented 4 years ago

I've put some thoughts into this for a long time. Let me list the current issues I've seen when using @types/node:

A user must select which version of Node.js they are targeting. This creates a barrier for most module authors as they likely need to support multiple version of Node.js.
The API changes are not kept up to date. Having them follow the usual Node.js LTS process would mean that they would land in master and be released and backported with the rest of the content.
The Node.js collaborators have no control on how a significant portion of their users use their work, as @types/node is not governed by the collaborators.
We could add some deliberate automated tests of the typescript experience.

There are however a few technical challenges of adding this to core:

a. We'd need to place our types within the Node.js install dir, and typescript (and all correlated tools, including VSCode) needs to be made aware that file exist and look it up. b. Backward compatibility needs to be designed carefully so that we could land it as opt-in in v10 and v12, and possibly make it a default in v14 (or v15). c. there should be no requirement that a PR into core adding an option or changing an API modify the types. Most core collaborators do not know typescript anyway, so this would be a significant blocker. It'd be kind of easy to add a need-types label or create good-first-issues to add the typings.

If we feel we should make this happen, I think the best path forward is:

Devise a plan with the typescript team, and see if they are onboard
Have a PR created that brings the most up-to-date version of @types/node into core as a basis
Create a typescript team or working group and onboard the interested maintainers of @types/node into it.

I'm happy to facilitate all the above.

tniessen commented 4 years ago

@mcollina What concerns me about shipping types with Node.js is that it will make it impossible or at least impractical to fix the types without upgrading Node.js to a newer version. That is one of the main reasons why I think it makes sense to maintain the types separately.

weswigham commented 4 years ago

Node's release schedule doesn't allow for shipping patches to incorrect type definitions; it's probably best to leave it handled by the community, or at least to continue publishing them out of band.

jasnell commented 4 years ago

So long as the types definition is not mandatory to be updated as we go, then all updates to the types definition in core could be landed and released in Node.js patch releases, which definitely come out far more regularly. The only question then becomes, is it regular enough? I definitely still see advantages in keeping it as a separate project (but still under the nodejs org)

jasnell commented 4 years ago

Another way we could do this: we could make it a separate project bundled as a vendored-in dependency/module not dissimilar to the way we handle npm now. User code could still use the standalone version or it can use the bundled version.

weswigham commented 4 years ago

It's probably worth noting that there's absolutely no way in TS for a "vendored" copy of @types/node packaged with node itself to actually be found; so "vendoring" it doesn't serve a direct purpose at present.

weswigham commented 4 years ago

And to restate our usual guidance for packages: A package which doesn't generate its types from its code probably shouldn't maintain its own types in-tree, especially not if it's not willing to publish releases just to fix issues in those types. In those cases, maintenance on DT (where releases are decoupled from the underlying project) is much preferred. I do not fundamentally believe node is exempt from this guidance, especially if there's no commitment to actually update them alongside changes to the underlying projects. Everyone would be much better served just by having the interested eyes and ears over at the PRs to @types/node on DefinitelyTyped.

jasnell commented 4 years ago

@weswigham:

It's probably worth noting that there's absolutely no way in TS for a "vendored" copy of @types/node packaged with node itself to actually be found; so "vendoring" it doesn't serve a direct purpose at present.

That goes back to @mcollina's statement here:

We'd need to place our types within the Node.js install dir, and typescript (and all correlated tools, including VSCode) needs to be made aware that file exist and look it up.

Vendored-in, we'd still have to make sure it could be discovered and used.

I'm +1 on the idea that we should not bake the types into core at all. It should be maintained as a separate project. But we should explore the feasibility of bundling them.

weswigham commented 4 years ago

Again, what use is vendoring a bundle that'll just drift out of date (since, again, the types update much more often than node itself does, as people make what are essentially documentation fixes)? Both VS and VSCode (and any editor that uses typescript's typingsInstaller utility) automatically download an appropriate @types/node version for js editor tooling, and any project that actually ships as a ts library with dts files has an enumerated dependency on @types/node that's installed through npm. Having a copy with node itself is just extra bytes in the installer, that no tool has a compelling reason to use.

cjihrig commented 4 years ago

I'm -1 to doing this, but not because I'm against Node type definitions.

It seems to be agreed upon that we aren't going to force core devs to work on the types (which I agree with). I'm quite certain that we won't do as good of a job keeping up with them as the community does.
When we inevitably drop the ball on the type definitions, are we going to cut a whole new Node release just to correct them? Updating Node versions is more disruptive to people trying to get real work done. The community modules can be updated and shipped a lot faster (as others have previously mentioned).
Moving things into the nodejs org causes a lot more overhead for everyone involved for no gain other than saying "but it comes from the nodejs org."
As others have mentioned, existing tools would need to be updated. Again, for little to no gain - just making work for people.

weswigham commented 4 years ago

Like, IMHO, the biggest impact thing the node core maintainers could do would be to actually document all of node core, and ideally programatically guarantee there are docs for all of node core, in node core (since then the node core code might actually be a useful reference). The current situation where most node core js code is uncommented densely written JS with at best implied argument types is not great, either for new contributors, or for humans trying to distill what's going on to create good type definitions for it. The lib folder in node is huge volume of JS code, large parts of which are undocumented at the endpoints. Here's a case study, using the commonly used fs.watchFile. For someone editing this code, is there anything that fully enumerates what members the options argument supports? That it's optional? That listener is optional (or not)? What shape listener is meant to take? If I go to the (out of band) docs I can find some of this information, but even then, it has ambiguities - listener is a Function, but it has current and previous members? Or are those arguments it will receive? What arguments is listener meant to take? Or return? And interestingly, in the code I've noticed reference to a bigint flag supported on options, which is completely undocumented for watchFile, but seems to be officially supported (that's probably a 2-year old doc bug I just randomly stumbled on)?

I think it'd be very high ROI (potential-contributor-wise) if the node core documentation was generated from inline code comments somehow, so you could measure (and improve) documentation coverage and provide context to the code. (And, ideally if the code itself was checked against that documentation, to try to prevent issues like the one I just found, but I can understand resistance to that, at least a little.)

jasnell commented 4 years ago

@cjihrig:

I'm -1 to doing this

I'm good with that answer also. They key thing I'd like to figure out is what, if anything, we can do in core to make it at least easier for the folks in the community to maintain/update the types definitions.

joyeecheung commented 4 years ago

I believe @bnb has been working on something related, do you have any insight into the issue people raised here?

joyeecheung commented 4 years ago

Like, IMHO, the biggest impact thing the node core maintainers could do would be to actually document all of node core, and ideally programatically guarantee there are docs for all of node core, in node core (since then the node core code might actually be a useful reference).

As someone who sometimes sprinkle jsdoc-style type annotations in the code base I am very much in favor of this (AFAIK, @bcoe also shares the same habit)

bnb commented 4 years ago

@joyeecheung I created an issue on that tooling here: https://github.com/nodejs/node/issues/32206

The tooling I'm recommending can leverage @electron/typescript-definitions. This removes the needs to independently maintain a .d.ts file and instead puts the focus on ensuring documentation accurately represents the API Docs. Find an error in the definitions? Fix the docs ❤️

bnb commented 4 years ago

Reading further context above:

To @jasnell's point, theoretically if we wrote our docs in leveraging docs-parser, the TypeScript team could just run @electron/typescript-definitions against it and we wouldn't have to do anything beyond ensuring the exposed API doesn't horrendously break ❤️

mhdawson commented 4 years ago

I'm in support of us working towards making our docs suitable for generation of the types. I think the information/structure would be useful even without considering its use with TypeScript.

SimonSchick commented 4 years ago

I personally have serious doubts about being able to generate types from docs and achieve on par type accuracy/quality of the ones we have right now. The puppeteer team tried this about a year ago and it didn't work out well at all.

weswigham commented 4 years ago

I also doubt they'll be immediately useful to TS consumers, too (especially without any checks on internal consistency or explicit continuous ecosystem testing for compatability), but having a more rigorous/checked docs process is an OK first step in the direction towards the kind of rigor and consistency a TS user would hope for, while hopefully like @mhdawson remarks, also providing some baseline benefit to the project itself, in terms of maintenance and structure, and investing in that is probably worth it, seperate from any TS concerns.

codebytere commented 4 years ago

@SimonSchick i agree it's a hard problem! No solution is going to hit every edge case in the short term - but the Electron team has had pretty remarkable success with it: here are the generated types from our last stable release. Our parser is able to handle a pretty wide range of types and needs, and is always welcoming of improvements and ways to encompass more of them.

As a starting point I think it's worth trying and iterating on - it's always possible to make a different decision down the road if this one feels less satisfactory than the current state of affairs.

tniessen commented 4 years ago

I personally have serious doubts about being able to generate types from docs and achieve on par type accuracy/quality of the ones we have right now.

What about generating both the documentation and the type definitions from a common source? Personally, I am a big fan of markdown, but maybe it makes sense to document APIs (and types) differently, maybe as JSON or YAML? That might also allow us to apply rendering patches such as https://github.com/nodejs/node/pull/31460 retrospectively for previous releases.

Flarna commented 4 years ago

What about generating both the documentation and the type definitions from a common source?

This would work best if the common source is typescript :grin:. If there is a plan to port NodeJs core to use typescript I'm onboard. But I don't expect this to happen. And even if this would happen the problem for module authors to support as max nodejs versions as possible would persist.

As long as source and doc/types are split there will be a gap. The level of detail in types is much higher then in docs for some areas. In special in the area of null/undefined and overloaded APIs (e.g. all the different key/encryption types,...). Besides that new or missing typescript features have also some impact on types.

Anyhow, every improvement in doc area is for sure an improvement for maintaining the types.

tniessen commented 4 years ago

As long as source and doc/types are split there will be a gap.

The gap concerns me less than the versioning. Right now, if a gap exists, TypeScript users can create a PR to DefinitelyTyped, and it will be a faster process than waiting for a new Node.js release.

Sure, if we were to decide to rewrite Node.js in TypeScript, we wouldn't have these problems, and probably fewer bugs.

theoludwig commented 3 years ago

TL;DR: IMO, we should include types inside Node.js and deprecate @types/node package and I think the path described by @mcollina in https://github.com/nodejs/node/issues/32022#issuecomment-595236693 is the right thing to do :

Devise a plan with the typescript team, and see if they are onboard

Have a PR created that brings the most up-to-date version of @types/node into core as a basis

Create a typescript team or working group and onboard the interested maintainers of @types/node into it.

The gap concerns me less than the versioning. Right now, if a gap exists, TypeScript users can create a PR to DefinitelyTyped, and it will be a faster process than waiting for a new Node.js release.

I think it is the same thing with every npm package like is it better a npm package does the feature or is it better that it is included in Node.js? The "faster process than waiting for a new Node.js release" could apply to everything inside Node.js, what if instead of providing fs, http, child_process, etc inside Node.js, it would be separated npm packages? Maybe I'm mistaken but actually, it could be possible thanks to C++ addons right? Node.js would only include the runtime itself without too many extra things, but is it something really wanted?

IMO, it would be better to include typings inside Node.js as it is closely related and TypeScript become the defacto for modern applications nowadays, also currently @types/node has 45,878,540 weekly downloads and it will continue to increase.

The most important argument to why we should include the types inside Node.js itself instead of @types/node is: to stay the APIs in sync with the types, and the same thing for versioning, as far as I know, @types/node don't handle current releases, I mean as I can see there is support for v13 inside @types/node but not for v15 :thinking:. Also, there is a new release that came out, v16 and there is no typings for it yet, maybe the types would not change too much, but still things like deprecation notices are also on JSDoc in @types/node, and currently they are not documented inside @types/node for example.

Even if I'm a big fan of TypeScript, I don't think we should rewrite Node.js in TypeScript even if as said "we wouldn't have these problems, and probably fewer bugs." and we should not include execution of TypeScript builtin inside Node.js as Deno do, because we should stay as close as possible to browsers, and browsers don't support yet TypeScript (yes I said "yet", hoping that in a maybe far feature, we could execute TypeScript inside most browsers, maybe something that need to be done for the V8 engine mostly).

Flarna commented 3 years ago

The most important argument to why we should include the types inside Node.js itself instead of @types/node is : to stay the APIs in sync with the types, and same thing for versioning, as far as I know, @types/node don't handle current releases, I mean as I can see there is support for v13 inside @types/node but not for v15 🤔. Also there is a new release that came out, v16 and there is no typings for it yet, maybe the types would not change too much, but still things like deprecation notice are also on JSDoc in @types/node, and currently they are not documented inside @types/node for exemple.

This is just caused by lack of people working on this. If there are too less people willing to work on node types at dt repo it will be most likely not better in the node repo.

github-actions[bot] commented 2 years ago

There has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment.

For more information on how the project manages feature requests, please consult the feature request management document.

github-actions[bot] commented 2 years ago

There has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment.

For more information on how the project manages feature requests, please consult the feature request management document.

bnb commented 2 years ago

FWIW small update on this, if https://github.com/nodejs/node/pull/41025 is implemented we can automate .d.ts generation from our documentation with relative ease.

github-actions[bot] commented 2 years ago

There has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment.

For more information on how the project manages feature requests, please consult the feature request management document.

github-actions[bot] commented 1 year ago

There has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment.

For more information on how the project manages feature requests, please consult the feature request management document.

github-actions[bot] commented 1 year ago

There has been no activity on this feature request and it is being closed. If you feel closing this issue is not the right thing to do, please leave a comment.

For more information on how the project manages feature requests, please consult the feature request management document.

nodejs / node