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

feat: ui primitive api autodocs #464

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

thatsamsonkid
Copy link
Collaborator

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

Which package are you modifying?

  • accordion
  • alert
  • alert-dialog
  • aspect-ratio
  • avatar
  • badge
  • button
  • calendar
  • card
  • checkbox
  • collapsible
  • combobox
  • command
  • context-menu
  • data-table
  • date-picker
  • dialog
  • dropdown-menu
  • hover-card
  • icon
  • input
  • label
  • menubar
  • navigation-menu
  • pagination
  • popover
  • progress
  • radio-group
  • scroll-area
  • select
  • separator
  • sheet
  • skeleton
  • slider
  • sonner
  • spinner
  • switch
  • table
  • tabs
  • textarea
  • toast
  • toggle
  • tooltip
  • typography

What is the current behavior?

Closes #247

What is the new behavior?

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@thatsamsonkid thatsamsonkid added this to the 1.0 Stable Release milestone Nov 9, 2024
@thatsamsonkid
Copy link
Collaborator Author

Wanted you all's opinion on this. So I think I got a semi-decent executor to autodoc the UI primitives API.

Output is a JSON file with an object where each property is the class name for a given component/directive with the following structure

"BrnAccordionItemDirective": {
    "file": "libs/ui/accordion/brain/src/lib/brn-accordion.directive.ts",
    "inputs": [
      {
        "name": "isOpened",
        "type": "boolean",
        "description": "Allows to set initial state of accordion"
      }
    ],
    "outputs": []
  },

it would require us to start self document each input output like so (it's currently not doing so well without explicit type on signal and assuming any in that case)

/**
* Allows to set initial state of accordion
*/
public readonly isOpened = input<boolean>(false);

my hope is we won't have to maintain the UI api docs as it's own task and we can just make sure it's well doc'ed in the code. Curious to hear your thoughts. My plan was place the static JSON in the site app's public assets folder and just read it in based on the className to feed them into a table on the page

@ashley-hunter @elite-benni @goetzrobin

@ashley-hunter
Copy link
Collaborator

ashley-hunter commented Nov 9, 2024

Personally, I have been using this approach in projects for many years and it has worked well. The benefit of this approach is users also get docs in their editor, for example if I highlight over a property or ctrl click to definition I can see docs right inline in my editor as well as them being available in a docs site. And it also means the docs are derived from the code so less likely to be out of date. So I'm all for this approach!

I haven't looked into your code much yet (as I'm on my phone), and it may do everything we need, but if not, just so you're aware, there are some solutions out there that are available if you don't want to write your own from scratch. One is compodoc (https://compodoc.app/) which actually comes built in with the Nx storybook executor, you can just enable it and get it to emit JSON. It's not perfect, but not bad.

Another option which could be used within our own executor like you have been doing but may help extract information is Angular's own API extraction tooling - although it's not really documented but it's pretty good:

https://github.com/angular/angular/tree/main/packages/compiler-cli/src/ngtsc/docs/src

I really like the idea, just wanted to share these in case they are of any use to you!

@thatsamsonkid
Copy link
Collaborator Author

Personally, I have been using this approach in projects for many years and it has worked well. The benefit of this approach is users also get docs in their editor, for example if I highlight over a property or ctrl click to definition I can see docs right inline in my editor as well as them being available in a docs site. And it also means the docs are derived from the code so less likely to be out of date. So I'm all for this approach!

I haven't looked into your code much yet (as I'm on my phone), and it may do everything we need, but if not, just so you're aware, there are some solutions out there that are available if you don't want to write your own from scratch. One is compodoc (https://compodoc.app/) which actually comes built in with the Nx storybook executor, you can just enable it and get it to emit JSON. It's not perfect, but not bad.

Another option which could be used within our own executor like you have been doing but may help extract information is Angular's own API extraction tooling - although it's not really documented but it's pretty good:

https://github.com/angular/angular/tree/main/packages/compiler-cli/src/ngtsc/docs/src

I really like the idea, just wanted to share these in case they are of any use to you!

Yea I was thinking to myself afterwards I was like there's probably already solutions for this 😅, but i'm all for either. We can leverage compodoc if we prefer or I can work to probably make this more robust by leveraging the angular one you mentioned. I wasn't aware of it so thanks for mentioning them!

@ashley-hunter
Copy link
Collaborator

Yea I was thinking to myself afterwards I was like there's probably already solutions for this 😅, but i'm all for either. We can leverage compodoc if we prefer or I can work to probably make this more robust by leveraging the angular one you mentioned. I wasn't aware of it so thanks for mentioning them!

You may not need either if you have it working fine already 😊 it may be easier not to rely on an additional tool at all.

I guess we could also extract some additional things like the selector and exportAs values, which could also be useful in the docs.

@elite-benni
Copy link
Collaborator

I am a big fan of keeping things together.
If we can generate documentation from the comment, that is also used by the ide this would be very nice.
If you always need to think about updating the api in the documentation page, it has a high probability to get out of synch.

I have nothing to add to Ashleys comments, because I have pretty much the same opinion.

@thatsamsonkid
Copy link
Collaborator Author

Awesome sounds good, I guess since i'm this far let me see if I can also add grabbing the selector and exportAs if available, great suggestion @ashley-hunter. Thanks all for the input, just wanted to make sure we were good with starting to self document this in the code. I know the trade off is slightly "busier" looking code with the extra comments but I think it's better than risking having out of date api docs

@thatsamsonkid
Copy link
Collaborator Author

Updated the structure to be like below so we can easily pull all the "accordion" api's for example and easily loop and create brain and helm sections on the page docs and dynamically create tables for each respective component/directive

{
  "accordion": {
    "brain": {
      "BrnAccordionContentComponent": {
        "file": "libs/ui/accordion/brain/src/lib/brn-accordion-content.component.ts",
        "inputs": [],
        "outputs": [],
        "selector": "brn-accordion-content",
        "exportAs": null
      },
   },
   "helm":{
      "HlmAccordionContentComponent": {
        "file": "libs/ui/accordion/helm/src/lib/hlm-accordion-content.component.ts",
        "inputs": [
          {
            "name": "userClass",
            "type": "ClassValue",
            "description": ""
          }
        ],
        "outputs": [],
        "selector": "hlm-accordion-content",
        "exportAs": null
      },
},
...

@thatsamsonkid
Copy link
Collaborator Author

thatsamsonkid commented Dec 3, 2024

Just wanted to give an update I think I mainly have the few things below left for this:

  • Fix some typing issues
  • Ensuring the docs get generated on app build
  • Fix issue with docs not loading on initial page load
  • Add Tests
  • Fixing an issue with the page nav stickyness
  • Updating all primitive pages with the api docs sections and extra data parameters

For this last one should I just go ahead and add the sections to all the primitive pages as part of this and fix any issues we find or should we go one by one after merging this to make sure the docs for each primitive are complete and correct first

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

Successfully merging this pull request may close these issues.

Docs: API reference for all components
3 participants