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.

Sign up for GitHub

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

Jump to bottom

Add API Reference to website #5922

Merged
merged 12 commits into from
Nov 14, 2023
Merged

Add API Reference to website #5922

merged 12 commits into from
Nov 14, 2023

Conversation

clemyan
Copy link
Member

@clemyan clemyan commented Nov 2, 2023 •
edited
Loading

What's the problem this PR addresses?

The API reference is not available in new website

How did you fix it?

This is a WIP to bring the API reference to the new website via docusaurus-plugin-typedoc-api.

The current state is has some kinks and issues to work out. The purpose of this draft PR is to track and discuss. I'll sort those out over time but if anyone wants to take over then be my guest.

  • Seems like docusaurus-plugin-typedoc-api need to use the instance from @docusaurus/theme-common and @docusaurus/plugin-content-docs used by the main doc, which were hidden inside @docusaurus/preset-classic. I added package extensions to resolve them but that seems brittle.
  • Docusaurus's MDX loader resolves react and @mdx-js/react from the loaded Markdown file. To include READMEs into the API reference we need every workspace to depend on react and @mdx-js/react which is not very reasonable. For now I use pnpFallbackMode: all so they fallback to the react and @mdx-js/react in the top-level but again seems brittle.
  • Only resolve one entrypoint for each workspace. Tried multiple entries and some pages broke. Maybe try again later?
  • Some stuff have duplicate source links due to virtual paths. Need to deduplicate
  • Functions receive dedicated pages but the header is just "Callable". Ideally I want them to be just in the index page like variables

Checklist

  • I have set the packages that need to be released for my changes to be effective.
  • I will check that all automated PR checks pass before the PR gets reviewed.

@clemyan
Copy link
Member Author

clemyan commented Nov 5, 2023 •
edited
Loading

So I have resolved most of the issues, mostly by patching packages.

Only remaining issue is how to deal with workspaces that have multiple TS entrypoints. For now that's just only:

  • @yarnpkg/cli
  • @yarnpkg/libzip
  • @yarnpkg/pnp
  • @yarnpkg/pnpify

As far as I have tested, the default support for multiple entries in docusaurus-plugin-typedoc-api has a number of limitations:

  • One must declare one of the entrypoints as the "index", which is what is shown as the package's Overview and its exports are shown at the top-level of the package in the sidebar
  • Other entrypoints do not get their own Overview page and must be second-level in the sidebar, though its label can be changed freely

image

I think I will try to further patch docusaurus-plugin-typedoc-api to override those restrictions.

@clemyan
Copy link
Member Author

clemyan commented Nov 7, 2023

I may have gone overzealous with my patches 😅

image

Additions to the docusaurus-plugin-typedoc-api patch I made since my last comment:

  • Each entrypoint gets its own Overview page, instead of just the main entrypoint
  • Instead of pushing the non-main entrypoints to one level deeper in the sidebar than the main entrypoint, multiple entrypoints in the same package get their own dropdown, including the main entrypoint
  • Add a level of non-collapsible headers to packages, so that we can make it just like the v3 API reference
  • Add support for remark/rehype plugins, so that we can inject our AutoLink and CommandLineHighlight plugins

At this point I think the PR is ready

@clemyan clemyan marked this pull request as ready for review November 7, 2023 14:08
arcanis
arcanis reviewed Nov 13, 2023
View reviewed changes
.yarn/patches/docusaurus-plugin-typedoc-api-npm-3.0.1-7dd061feb2.patch
Comment on lines +26 to +27
+ remarkPlugins: [],
+ rehypePlugins: [],
Copy link
Member

Choose a reason for hiding this comment

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

By any chance, can you try sending a PR upstream to get these changes accepted? I'd prefer if we could avoid maintaining permanent patches.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done. milesj/docusaurus-plugin-typedoc-api#123

Copy link
Member

Choose a reason for hiding this comment

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

Thanks!

merceyz
merceyz reviewed Nov 13, 2023
View reviewed changes
.yarn/patches/@docusaurus-mdx-loader-npm-2.4.3-0d95b4751a.patch
Comment on lines +28 to +29
+import React from "${(0, url_1.pathToFileURL)(require.resolve('react'))}";
+import { mdx } from "${(0, url_1.pathToFileURL)(require.resolve('@mdx-js/react'))}";
Copy link
Member

Choose a reason for hiding this comment

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

Would this allow removing the dependencies added in https://github.com/yarnpkg/berry/pull/5625/files#diff-7ae45ad102eab3b6d7e7896acd08c427a9b25b346470d7bc6507b6481575d519?

If you could send a PR upstream to fix this as well that would be great.

Copy link
Member Author

@clemyan clemyan Nov 14, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Would this allow removing the dependencies added in https://github.com/yarnpkg/berry/pull/5625/files#diff-7ae45ad102eab3b6d7e7896acd08c427a9b25b346470d7bc6507b6481575d519?

Hmm.. Probably. I will test later

If you could send a PR upstream to fix this as well that would be great.

I will submit one. Though I do think this code pattern is kinda obscure and won't be surprised if it is rejected. Also, I don't know if they will backport it to v2. If not then I guess we should focus on migrating to Docusaurus v3 before merging this.

Copy link
Member

@merceyz merceyz Nov 14, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

We can keep the patch so that's fine but to make future upgrades easier it would be nice to get it upstream at some point.

@Josh-Cena Indicated that they would be fine with a change like this in https://discord.com/channels/226791405589233664/654372321225605128/949824347529445487

Copy link

@Josh-Cena Josh-Cena Nov 14, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

By the way, Docusaurus now uses MDX v3 and React automatic runtime, so things may be different ;)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah. Last night I started looking into making a PR and found that part of the MDX loader in Docusaurus v3 has changed significantly.

@merceyz Let's wait until after we migrate to Docusaurus v3 to see whether we still need the patch and/or to make an upstream PR

@arcanis arcanis merged commit 017b94a into yarnpkg:master Nov 14, 2023
YoutacRandS-VA

This comment was marked as off-topic.

Sign in to view

@clemyan clemyan deleted the website-api-docs branch April 10, 2024 13:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@arcanis arcanis arcanis left review comments

@merceyz merceyz merceyz left review comments

@Josh-Cena Josh-Cena Josh-Cena left review comments

@YoutacRandS-VA YoutacRandS-VA YoutacRandS-VA left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@clemyan @arcanis @merceyz @Josh-Cena @YoutacRandS-VA