feat(compiler): expand class api doc extraction by jelbourn · Pull Request #51685 · angular/angular

jelbourn · 2023-09-06T23:57:59Z

Based on top of #51682

This expands on the skeleton previously added to extract docs info for classes, including properties, methods, and method parameters. Type information and Angular-specific info (e.g. inputs) will come in future PRs.

cc @devversion in case you're interested

packages/compiler-cli/src/ngtsc/docs/src/entities.ts

devversion · 2023-09-07T11:31:32Z

packages/compiler-cli/src/ngtsc/core/src/compiler.ts

NIt: Could also use .push

My understanding is that concat is faster than push with destructuring, no?

Not sure that performance matters here, and I'm also not sure if this is true anymore.

Feel to just leave as is. This is a nit (but you used push in other places)

Out of curiosity, is there a trusted source for the perf comparison? The best I could find myself was just running a benchmark https://www.measurethat.net/Benchmarks/Show/13985/0/concat-two-arrays-push-vs-destructuring

devversion · 2023-09-07T11:34:04Z

packages/compiler-cli/src/ngtsc/docs/src/extractor.ts

One thing I'd like to start conversation on: In Dgeni our TypeScript extraction was ultimately only "documenting" symbols that were exported from the package entry-point. e.g. @angular/core/index.

Are we always analyzing everything, or should we rather traverse based on the entry-point and e.g. discover/extract referenced members?

I'm going to worry about filtering down the set of APIs we either extract or document later in the process; for now I'm focusing on extracting all of the information we need for docs. But yeah, it's reasonable to use an entry-point rather than all files in the program.

devversion · 2023-09-07T11:35:44Z

packages/compiler-cli/src/ngtsc/docs/src/extractor.ts

Nit: I know this is still TODO, but I think we could use the compiler type for ClassDeclaration, or maybe even the reflector. That way we wouldn't need the non-null assert. See TypeScriptReflectionHost.

I've already done this in a follow-up commit, so I'll leave it as-is for this one.

devversion · 2023-09-07T11:38:24Z

packages/compiler-cli/src/ngtsc/docs/src/entities.ts

Is there really the concept of an optional member? seems like this information is not useful for documentation? and technically it just means that something can be undefined. This can also be expressed using an explicit type. e.g.

bla: string|undefined;

There is nuance between a member being optional and accepting an undefined value. This matters for config objects:
https://www.typescriptlang.org/play?#code/JYOwLgpgTgZghgYwgAgLJzAEWHANgewHMBhfEGYQ5AbwChlkxgxcIAuZAZzClEIG56yOLzgAZOACMIuDt14gqAH2QBXEABMIFEBA2CAvrVqhIsRCnRgAyiEQBrSSNLlKNIUxbsuPPoIYiOBLSuAD8cr6KhsYw6ghMZMj4AA4QINh4RAAUCGQUhBxWGQQkeZQAlO4MuSCc+KwAdCU5ZYTl0bSxIPHAiSlptg5OUC2uBWgYgwiOzq2VdNVkdY3NNfnttEa0-ek4zQuMzKwcAOTWEGBMipwnmxvbqSBTMyMHnsfIJwCCrFBgJwAaO78IA

When a config field is optional, you can omit it. When the value may be undefined, you must still specify the field.

That makes sense. Might be worth adding a comment for this. nly thinking about this in the context of "describing the member in a local sense".

devversion · 2023-09-07T11:39:28Z

packages/compiler-cli/src/ngtsc/docs/src/extractor.ts

For the above checks, I'd recommend using ts.isMethodDeclaration etc to avoid the casts

Oh, I meant to use my own utility method here and forgot to follow through, but also hadn't realized isMethodDeclaration and isPropertyDeclaration existed, so I've replaced them all with these.

devversion · 2023-09-07T11:40:50Z

packages/compiler-cli/src/ngtsc/docs/src/extractor.ts

Just an early question based on our discussions: Do we plan on using the metadata from the existing ngtsc decorator handlers, or will we just have our own simpler extraction here that we co-maintain in @angular/compiler-cli?

Yeah, this will reuse the existing metadata reader

jelbourn

Comments addressed

jelbourn · 2023-09-07T15:55:50Z

packages/compiler-cli/src/ngtsc/core/src/compiler.ts

My understanding is that concat is faster than push with destructuring, no?

jelbourn · 2023-09-07T15:59:59Z

packages/compiler-cli/src/ngtsc/docs/src/entities.ts

There is nuance between a member being optional and accepting an undefined value. This matters for config objects:
https://www.typescriptlang.org/play?#code/JYOwLgpgTgZghgYwgAgLJzAEWHANgewHMBhfEGYQ5AbwChlkxgxcIAuZAZzClEIG56yOLzgAZOACMIuDt14gqAH2QBXEABMIFEBA2CAvrVqhIsRCnRgAyiEQBrSSNLlKNIUxbsuPPoIYiOBLSuAD8cr6KhsYw6ghMZMj4AA4QINh4RAAUCGQUhBxWGQQkeZQAlO4MuSCc+KwAdCU5ZYTl0bSxIPHAiSlptg5OUC2uBWgYgwiOzq2VdNVkdY3NNfnttEa0-ek4zQuMzKwcAOTWEGBMipwnmxvbqSBTMyMHnsfIJwCCrFBgJwAaO78IA

When a config field is optional, you can omit it. When the value may be undefined, you must still specify the field.

jelbourn · 2023-09-07T16:02:08Z

packages/compiler-cli/src/ngtsc/docs/src/extractor.ts

I'm going to worry about filtering down the set of APIs we either extract or document later in the process; for now I'm focusing on extracting all of the information we need for docs. But yeah, it's reasonable to use an entry-point rather than all files in the program.

jelbourn · 2023-09-07T16:03:01Z

packages/compiler-cli/src/ngtsc/docs/src/extractor.ts

I've already done this in a follow-up commit, so I'll leave it as-is for this one.

jelbourn · 2023-09-07T16:06:34Z

packages/compiler-cli/src/ngtsc/docs/src/extractor.ts

Oh, I meant to use my own utility method here and forgot to follow through, but also hadn't realized isMethodDeclaration and isPropertyDeclaration existed, so I've replaced them all with these.

jelbourn · 2023-09-07T16:07:05Z

packages/compiler-cli/src/ngtsc/docs/src/extractor.ts

Yeah, this will reuse the existing metadata reader

atscott · 2023-09-08T15:52:39Z

packages/compiler-cli/src/ngtsc/docs/src/entities.ts

Any reason for these to be string enums?

They'll need to be serialized to json, and this makes them both stable and human-readable

atscott · 2023-09-08T15:53:03Z

packages/compiler-cli/src/ngtsc/docs/src/entities.ts

nit: I feel like the enums should be UpperCamelCase (though the internal TS style guide says CONSTANT_CASE)

Ack, I'll change this in a follow-up commit since doing it here will be merge conflict city

@alxhub

This commit adds a barebones skeleton for extracting information to be used for extracting info that can be used for API reference generation. Subsequent PRs will expand on this with increasingly real extraction. I started with @alxhub's angular#51615 and very slightly polished to get to this minimal commit.

Based on top of angular#51682 This expands on the skeleton previously added to extract docs info for classes, including properties, methods, and method parameters. Type information and Angular-specific info (e.g. inputs) will come in future PRs.

Based on top of angular#51685 This expands on the extraction with information for directives, including inputs and outputs. As part of this change, I've refactored the extraction code related to class and to directives into their own extractor classes to more cleanly separate extraction logic based on type of statement.

Based on top of #51685 This expands on the extraction with information for directives, including inputs and outputs. As part of this change, I've refactored the extraction code related to class and to directives into their own extractor classes to more cleanly separate extraction logic based on type of statement. PR Close #51733

jelbourn · 2023-09-18T23:45:31Z

Changes merged in #51733

angular-automatic-lock-bot · 2023-10-19T00:11:32Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

jelbourn added area: compiler Issues related to `ngc`, Angular's template compiler target: minor This PR is targeted for the next minor release labels Sep 6, 2023

jelbourn requested a review from alxhub September 6, 2023 23:57

ngbot bot modified the milestone: Backlog Sep 6, 2023

angular-robot bot added the detected: feature PR contains a feature commit label Sep 6, 2023

jelbourn force-pushed the more-api-gen branch from d9bac82 to dd1beb9 Compare September 7, 2023 02:34

devversion reviewed Sep 7, 2023

View reviewed changes

jelbourn force-pushed the more-api-gen branch from dd1beb9 to a68f776 Compare September 7, 2023 16:08

jelbourn commented Sep 7, 2023

View reviewed changes

devversion approved these changes Sep 7, 2023

View reviewed changes

jelbourn requested a review from atscott September 7, 2023 20:23

jelbourn mentioned this pull request Sep 7, 2023

feat(compiler): extract directive docs info #51697

Closed

atscott approved these changes Sep 8, 2023

View reviewed changes

jelbourn force-pushed the more-api-gen branch from a68f776 to e364379 Compare September 8, 2023 16:56

jelbourn added 2 commits September 9, 2023 11:55

jelbourn force-pushed the more-api-gen branch from e364379 to 5d962ad Compare September 9, 2023 18:57

jelbourn mentioned this pull request Sep 15, 2023

feat(compiler): extract docs info for enums, pipes, and NgModules #51733

Closed

jelbourn closed this Sep 18, 2023

angular-automatic-lock-bot bot locked and limited conversation to collaborators Oct 19, 2023

Conversation

jelbourn commented Sep 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jelbourn Sep 7, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jelbourn left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jelbourn Sep 7, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jelbourn commented Sep 18, 2023

Uh oh!

angular-automatic-lock-bot bot commented Oct 19, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jelbourn commented Sep 6, 2023 •

edited

Loading

jelbourn Sep 7, 2023 •

edited

Loading

jelbourn Sep 7, 2023 •

edited

Loading