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
Contributor

@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
Copy link
Contributor 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
Contributor 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
Contributor 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
Contributor 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
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