-
Notifications
You must be signed in to change notification settings - Fork 30k
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
Solid Node.js with official type system support #32022
Comments
|
Are you proposing a separate, officially maintained npm package, or bundling it with Node.js somehow? |
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. |
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. |
Isn't that the issue that @gengjiawen is trying to address with this proposal? |
I'm sceptical about the proposed ways to address this.
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. |
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. |
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 |
I'm also skeptical that separate releases/repos can really solve this problem.
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 |
@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. As a long time 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. |
As a long time Node.js user, my experience with the e.g: the custom A lot of types are also not very strict, in the sense that an encoding option is a Of course adding type definitions for internal bindings (which are technically public) and the level of strictness of the type definitions is subjective. |
I've put some thoughts into this for a long time. Let me list the current issues I've seen when using
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 If we feel we should make this happen, I think the best path forward is:
I'm happy to facilitate all the above. |
@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. |
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. |
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) |
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. |
It's probably worth noting that there's absolutely no way in TS for a "vendored" copy of |
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 |
That goes back to @mcollina's statement here:
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. |
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 |
I'm -1 to doing this, but not because I'm against Node type definitions.
|
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 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.) |
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. |
I believe @bnb has been working on something related, do you have any insight into the issue people raised here? |
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) |
@joyeecheung I created an issue on that tooling here: #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 ❤️ |
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 ❤️ |
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. |
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. |
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. |
@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. |
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 #31460 retrospectively for previous releases. |
This would work best if the common source is typescript 😁. 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 Anyhow, every improvement in doc area is for sure an improvement for maintaining the types. |
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. |
TL;DR: IMO, we should include types inside Node.js and deprecate
I think it is the same thing with every npm package like is it better a 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 The most important argument to why we should include the types inside Node.js itself instead of 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). |
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. |
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. |
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. |
FWIW small update on this, if #41025 is implemented we can automate .d.ts generation from our documentation with relative ease. |
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. |
This comment was marked as outdated.
This comment was marked as outdated.
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. |
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.
The text was updated successfully, but these errors were encountered: