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

Generate CLI docs as a Docusaurus plugin #6218

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

clemyan
Copy link
Member

@clemyan clemyan commented Apr 11, 2024

What's the problem this PR addresses?

The current method of CLI docs generation is very inefficient because it is synchronously blocking and memory-intensive. It can easily take up 40% of the time needed to start the dev server.

How did you fix it?

Created a Docusaurus plugin that generates the CLI docs. This has a number of advantages over the existing method:

  • It is async and happens in parallel to other Docusaurus plugins, thus much faster in terms of total build time.
  • It is written in TypeScript, so type-checked and has better DX
  • It obtains the command definitions by running the TypeScript source directly instead of going through a shell, saving overhead
  • It writes the result to disk via Docusaurus APIs, saving memory

Performance

# Before
Warm dev startup
  Time (mean ± σ):     68.887 s ±  1.508 s    [User: 114.795 s, System: 18.496 s]
  Range (min … max):   67.447 s … 71.328 s    5 runs

Cold dev startup
  Time (mean ± σ):     68.346 s ±  0.204 s    [User: 104.264 s, System: 15.938 s]
  Range (min … max):   68.195 s … 68.675 s    5 runs

Warm Build
  Time (mean ± σ):     110.748 s ±  0.781 s    [User: 139.832 s, System: 15.310 s]
  Range (min … max):   109.902 s … 111.528 s    5 runs

Cold Build
  Time (mean ± σ):     306.749 s ± 10.032 s    [User: 1588.512 s, System: 300.103 s]
  Range (min … max):   298.550 s … 322.576 s    5 runs
  
# After
Warm dev startup
  Time (mean ± σ):     42.410 s ±  0.546 s    [User: 60.527 s, System: 6.049 s]
  Range (min … max):   41.807 s … 43.265 s    5 runs

Cold dev startup
  Time (mean ± σ):     42.275 s ±  0.326 s    [User: 59.571 s, System: 5.824 s]
  Range (min … max):   41.760 s … 42.657 s    5 runs

Warm Build
  Time (mean ± σ):     85.861 s ±  2.109 s    [User: 95.237 s, System: 5.144 s]
  Range (min … max):   83.617 s … 88.499 s    5 runs

Cold Build
  Time (mean ± σ):     149.498 s ±  7.973 s    [User: 440.898 s, System: 15.700 s]
  Range (min … max):   140.074 s … 160.208 s    5 runs

Index pages

I have also taken the opportunity to take the @yarnpkg/cli index page and adapted it to the other binaries

⚠️ URL changes

Unfortunately, generating the CLI docs as a separate plugin makes the original URL scheme conflict with the main docs plugin, so we either have to

  • move all generated CLI docs under a single path prefix (e.g. /cli)
  • lose state (in particular, sidebar scroll state) when navigating between pages for different binaries

I have opted to move everything under /cli

Other Changes

The change causes a few visual changes to existing stuff:

  • The previous page and next page links have been removed. Should be easy to recreate but I don't feel like they have much value
  • The sidebar is now sorted in lexicographical order
  • The additional binaries' examples now include the yarn invocation, and thus is properly styled
  • Very minor, but the column widths in the options table have shifted very slightly

Future work

This PR is ready, but I am still experimenting with a few things that may or may not make it into the PR.

  • I'm trying whether it is possible to use Docusaurus's watch mechanism to hot rebuild the pages.
  • Also, as you can see, there are practically no difference in warm and cold startup time. Maybe checking mtimes can avoid some work?

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 clemyan force-pushed the website/cli-docs-plugin branch 2 times, most recently from d0e62d4 to ef0d749 Compare April 13, 2024 14:33
@clemyan
Copy link
Member Author

clemyan commented Apr 13, 2024

Got hot rebuild to work, using jiti (which is also used by Docusaurus) to re-compile the command files every time a rebuild is triggered by Docusaurus's watcher. Compilation uses esbuild by incorporating some changes from #5581.

But the deploy preview is failing now, will investigate.

@clemyan
Copy link
Member Author

clemyan commented Apr 13, 2024

Looks like using esbuild to compile via jiti was the culprit. Removed that integration for now to unblock the PR.

@clemyan
Copy link
Member Author

clemyan commented Apr 15, 2024

Re: esbuild + jiti. Turns out code generated by esbuild is not always reentrancy-safe (i.e. it can't handle circular requires in some cases), and jiti's implementation of interopDefault triggers the reentrancy-unsafe case. It can be made to work but performance is actually worse than just using the default babel-based compilation pipeline.

Also investigated caching to improve warm start, but that does not give a significant perf boost compared to the complexity increase. The limiting factor is probably somewhere else.

Comment on lines +16 to +19
const loader = jiti(__filename, {
cache: true,
requireCache: false,
});
Copy link
Member

Choose a reason for hiding this comment

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

What is this for? I see you mentioned jiti in the PR's comment, but that you also intended to remove it

Copy link
Member Author

Choose a reason for hiding this comment

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

jiti is a library to create loader functions that are drop-in replacements for require with on-the-fly TS compilation, used by Docusaurus itself (e.g. to require docusaurus.config.ts).

I want the CLI doc plugin to do hot reload. That is, if I change a command's metadata while the dev server is running, I want the change to reflect on the page without restarting the dev server. The problem with that is the cli-docs.ts is compiled by Docusaurus's internal jiti loader instance, so any import or require I use will just be intercepted. But that jiti instance has an internal, hidden cache, so it will just load the cached stale instance of the changed command file, breaking hot reload. To get around that, I needed to create a separate jiti loader instance which properly recompile every time it is called.

The trouble comes when I tried to integrate our own on-the-fly TS compilation pipeline (scripts/setup-ts-execution) into that jiti loader instance. Turns out jiti and esbuild have some subtle incompatibilities, so I just removed the esbuild integration and use jiti with its built-in babel-based compilation pipeline. The intention was never to remove jiti.

@clemyan clemyan mentioned this pull request May 2, 2024
3 tasks
arcanis pushed a commit that referenced this pull request Jun 19, 2024
## What's the problem this PR addresses?

Some minor/nitpick-level problems with the website

Note: This PR has *some* overlap with #6218. I'll rebase one when the
other is merged

## How did you fix it?

- Removed remnants of TypeScript misconfiguration created when first
adding the docusaurus workspace via `create-docusaurus`
- Reorganized the directory structure of the docusaurus workspace. In
particular, moved stuff that is run in build-time (except the
`docusaurus.config.ts` itself) to a `config` directory. May not seem
like much but as more stuff gets added this can keep thing clean and
manageable.
- Used admonitions instead of plain text where appropriate
- Made the remark plugins apply to the CHANGELOG (they weren't before)
- Cleaned up and reorganized the dependencies. Of course there are
different schools of philosophy regarding what should count as a
dependency vs devDependency in a frontend app. Ultimately I decided for
Docusaurus, it makes more sense to say everything needed to run `yarn
build` successfully is a dep and devDeps are those that are purely for
DX (e.g. types)
- Removed babel and browserslist config as we are not using them at all

## Checklist

<!--- Don't worry if you miss something, chores are automatically
tested. -->
<!--- This checklist exists to help you remember doing the chores when
you submit a PR. -->
<!--- Put an `x` in all the boxes that apply. -->
- [x] I have read the [Contributing
Guide](https://yarnpkg.com/advanced/contributing).

<!-- See
https://yarnpkg.com/advanced/contributing#preparing-your-pr-to-be-released
for more details. -->
<!-- Check with `yarn version check` and fix with `yarn version check
-i` -->
- [x] I have set the packages that need to be released for my changes to
be effective.

<!-- The "Testing chores" workflow validates that your PR follows our
guidelines. -->
<!-- If it doesn't pass, click on it to see details as to what your PR
might be missing. -->
- [x] I will check that all automated PR checks pass before the PR gets
reviewed.
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.

None yet

2 participants