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

docs(store): add API reference #1762

Merged
merged 9 commits into from
Oct 14, 2023
Merged

docs(store): add API reference #1762

merged 9 commits into from
Oct 14, 2023

Conversation

alvrs
Copy link
Member

@alvrs alvrs commented Oct 12, 2023

This approach lets us generate API docs from NatSpec and render them on the docs page.
Limitations:

  • No sidebar quickjumps for headlines (nextra limitation with imported markdown)
  • Links to github source and links to eg other interfaces in the docs don't work (bc forge is generating invalid links for github, and the nextra sitemap is different from the one expected by forge)

We could work around both limitations by creating a script that parses the raw forge markdown output, processes it (eg to replace links and change headlines from H1 to H2 etc), and render it as plain mdx file in the docs.

Curious about your thoughts on this approach @holic @qbzzt

@alvrs alvrs requested a review from holic as a code owner October 12, 2023 19:30
@changeset-bot
Copy link

changeset-bot bot commented Oct 12, 2023

⚠️ No Changeset found

Latest commit: a82dcf3

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@alvrs alvrs requested a review from qbzzt October 12, 2023 19:35
@holic
Copy link
Member

holic commented Oct 13, 2023

We could work around both limitations by creating a script that parses the raw forge markdown output, processes it (eg to replace links and change headlines from H1 to H2 etc), and render it as plain mdx file in the docs.

another approach could be a component that wraps each markdown file and does ~the same (though it prob wouldn't solve sidebar issues)

@holic
Copy link
Member

holic commented Oct 13, 2023

Looks decent, despite shortcomings!

https://mud-next-docs-fv7gsl0r0-latticexyz.vercel.app/store/reference/store-core

Seems like the imported markdown is also missing from search, which is a big downside IMO. Bigger than the ones mentioned above.

@alvrs
Copy link
Member Author

alvrs commented Oct 13, 2023

Seems like the imported markdown is also missing from search, which is a big downside IMO. Bigger than the ones mentioned above.

Good catch! I think to solve this we have to print the markdown instead of using the nextra component

@alvrs
Copy link
Member Author

alvrs commented Oct 13, 2023

Added a script to post-process the forge doc output and render it as raw mdx in the docs

Copy link
Contributor

@qbzzt qbzzt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I love it. There is some formatting weirdness (compare http://localhost:3000/store/reference/store-core#getfieldlayout and http://localhost:3000/store/reference/store-core#getvalueschema, for example). Also, I'd expect the italicized test to be the one in the purple box, not what's in the green one.

Screenshot 2023-10-13 at 12 53 51 PM

However, these are minor and we have lots of docs to go through. I saw merge it now and deal with formatting later. It's clear enough.

@alvrs
Copy link
Member Author

alvrs commented Oct 13, 2023 via email

@alvrs alvrs merged commit aa5b227 into main Oct 14, 2023
@alvrs alvrs deleted the alvrs/contract-references branch October 14, 2023 09:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants