-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Smarter enhancement of JSDoc comments with a JSDoc parser #4310
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can safely say I can't maintain this code. Just reviewing it so far has taken me a not-insignificant amount of time.
I've been thinking about it quite a lot by now and I can safely say I don't feel comfortable at all landing this.
Would love to land this if:
- The parser is in a maintained and trusted external crate. Just remembered oxc, which gives me an orders of magnitude better impression so far than SWC. If you can do the research I'm happy to look into and make a decision on if a specific dependency is acceptable.
- It is opt-in, preferably via an attribute (alternatively I proposed just a line saying
wbg::jsdoc-start
or something like that). Again: I'm really not a fan of parsing doc comments by default. But I'm happy to discuss this point again and be convinced otherwise.
If this doesn't work out, the easy alternative is still to support third-party crates via a bunch of WBG attributes. Definitely the less appealing way.
That's a shame.
To repeat my previous findings: There are essentially 3 packages on crates.io that need to be considered (all others aren't JSDoc parsers):
While Anyway, the JSDoc parser itself seems usable. I did some tests, and it's not 100% correct when parsing type expressions, but it should be good enough. The only issue is that their JSDoc AST is immutable, so we'll have to implement the modifying and printing ourselves. So yeah, that's the state of JSDoc parsing in the Rust ecosystem. The choice is either
As I said before, the whole point is to change the default of WBG just adding
As I explained before, this is not possible. Only the CLI knows the final return/parameter types. No matter what attributes we may add, only the CLI has the necessary information to perform this task. There is no alternative. |
I've already made a decision on
We have already discussed this, I deemed this a non-goal for WBG. As I said many times before: our target audience are developers who want to builds tools on top of In this case I have reasons I laid out already why I think this isn't an ideal situation to do by default. Which I find more important then convenience in this case. If the point is not convenience, then I apparently missed the point.
The last time we have discussed this we established that this is possible via attributes. From your comment here I think there is a misunderstanding. I'm talking about a WBG-CLI implementation. The JSDoc is just passed via attributes instead of parsing Rustdoc comments. A third-party crate can parse the JSDoc in the Rustdoc and fill in these attributes instead of WBG doing it. The difference is that attributes can be split up into attributes on arguments, a If I'm missing some specific interaction that can not be support this way then please elaborate. |
Resolves #4223.
Resolves #1847.
This PR improves the type information is added to JSDoc comments. Instead of just adding
@param
and@returns
tags to existing JSDoc comments, WBG will now parse the existing comment and add type information to existing tags (if any). This makesskip_jsdoc
virtually unnecessary, since WBG won't generate tag that conflict with existing tags anymore. (Note thatskip_jsdoc
still has niche use cases.)Example:
This behavior can be turned off with
skip_jsdoc
With the new smarter behavior, users documenting their code with standard JSDoc/TSDoc can now use
@param
/@returns
tags without having WBG mess it up or having to usingskip_jsdoc
and duplicating the type info. WBG's JSDoc comment enhancement now works together with our users.Moving forward, I believe that we should teach people to WBG will automatically add type information for them, and they should not manually add type info to
@param
and@returns
tags at all. This has several advantages, one being that users don't get any ideas about trying to declare custom types as@param {1 | 2 | 3} arg
, which does not affect the TS types. Once we have a way for users to declare custom TS type annotations, they will just work with the new JSDoc enhancements.The main complexity of this PR is:
The parser is written in a way that makes any string valid JSDoc. This can also be seen by the signature of the parsing function:
fn parse(comment: &str) -> JsDoc
. The doc comment on the function explains how this is done. In generally, I extensively documented the parser. If you have any question regarding how the parser works, the answer is probably in a comment.For the tags that are supported, the implementation is fairly complete. The parser supports all sorts of mean constructs like multiple-line TS types, comments in TS types, default values, and argument paths (e.g.
@param {number} arg0.age
). So it's fairly complete for what it supports. See the snapshot test for many more examples.