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

Make it possible for third party cli extensions to use [siteDir] and --config #8903

Open
1 of 2 tasks
Dr-Electron opened this issue Apr 18, 2023 · 5 comments
Open
1 of 2 tasks
Labels
difficulty: advanced Issues that are complex, e.g. large scoping for long-term maintainability. proposal This issue is a proposal, usually non-trivial change

Comments

@Dr-Electron
Copy link
Contributor

Dr-Electron commented Apr 18, 2023

Have you read the Contributing Guidelines on issues?

Motivation

For some use cases, it would be nice to have a possibility for a third party to extend the CLI in a way so that those two options can be passed for custom setups.

As an example, look at this issue, where you would want to use their extendCli functionality, but can't as your config is in a subfolder or named differently.

Obviously plugins could also "just" add siteDir and --config to their extendCli, but that seems weird to me as most get that from the context they get on initialization.

Maybe it would be possible to make those two arguments part of the "main" docusaurus cli, so it can be parsed before and be passed to the plugin. Something like https://www.npmjs.com/package/commander#parsing-configuration

I'm not a plugin dev so maybe I understood something wrong here.
So would be nice to hear your opinion on that topic.

Self-service

  • I'd be willing to do some initial work on this proposal myself.
@Dr-Electron Dr-Electron added proposal This issue is a proposal, usually non-trivial change status: needs triage This issue has not been triaged by maintainers labels Apr 18, 2023
@Josh-Cena
Copy link
Collaborator

This would be very tricky. Commands are provided by plugins, and to initialize plugins we need to know the site dir and the config, so we can find plugins at all. Allowing CLI extensions to read custom config means they have to be loaded in a lifecycle prior to loading the config. The workflow looks like this:

flowchart TD
A[1. The docusaurus command invokes the entrypoint executable] --> B[2. All built-in commands get registered onto the <code>cli</code> instance]
B --> C{"3. Do we recognize<br>the current command?<br>(Using a very ad-hoc parser)"}
C -- Yes --> X[4. Run <code>cli.parse</code>] --> D[5. Commander invokes the corresponding function] --> E[6. The function uses <code>siteDir</code>/<code>config</code> to load plugins]
C -- No --> F[7. Load plugins and let them extend CLI] --> Y[8. Run <code>cli.parse</code>] --> G["9. Commander invokes the corresponding function<br>(Or still no command found by commander, and we output help text)"]
Loading

There are two solutions here:

  1. We can use our "ad-hoc parser" at step 3 (which at the moment is literally process.argv.slice(2)[0]) to parse two options called --config and --site-dir and load plugins with these options, and then make them extend CLI at step 7 as usual.
  2. We change the way that extendCli works and do not allow plugins to manipulate the cli instance. Rather, the core parses the command for them, and they only provide handlers in the form of { command: string, action: (options: object) => Promise<void>, description: string } that the core invokes based on the command it parses out.

i.e.

flowchart TD
A[1. The docusaurus command invokes the entrypoint executable] --> B[2. All built-in commands get registered onto the <code>cli</code> instance] --> C[3. A fallback handler gets registered] --> X[4. Run <code>cli.parse</code>] --> D[5. If nothing handles the current command,<br>the fallback handler handles it]
Loading

I hope all this makes sense... This is much, much harder than it may seem.

@Josh-Cena Josh-Cena added difficulty: advanced Issues that are complex, e.g. large scoping for long-term maintainability. and removed status: needs triage This issue has not been triaged by maintainers labels Apr 19, 2023
@slorber
Copy link
Collaborator

slorber commented Apr 19, 2023

We can use our "ad-hoc parser" at step 3 (which at the moment is literally process.argv.slice(2)[0]) to parse two options called --config and --site-dir and load plugins with these options, and then make them extend CLI at step 7 as usual.

Looks like a good solution.
Apparently, we can create non-global program objects so we could even use a 2nd commander instance to have a better parsing logic for external commands.

import { Command } from https://github.com/tj/commander.js;
const program = new Command();

And then use the siteDir/config here:

import fs from 'fs-extra';
import {loadContext} from '../server';
import {initPlugins} from '../server/plugins/init';
import type {CommanderStatic} from 'commander';

export async function externalCommand(cli: CommanderStatic): Promise<void> {
  const siteDir = await fs.realpath('.');
  const context = await loadContext({siteDir});
  const plugins = await initPlugins(context);

  // Plugin Lifecycle - extendCli.
  plugins.forEach((plugin) => {
    plugin.extendCli?.(cli);
  });
}

Didn't try but maybe it could work 🤷‍♂️

@Dr-Electron
Copy link
Contributor Author

Sorry for my late answer. I basically already discussed option 1 with @jlvandenhout. So I'm fine with that solution.
Maybe we can even help with this issue. But no promises as we are quite busy this days ourselves 😅

@johnmcase
Copy link

Is this change still planned? I too am running into the original issue (PaloAltoNetworks/docusaurus-openapi-docs#490) in the openapi-docs plugin which has since been closed because they are expecting the docusaurus CLI to handle it.

@slorber
Copy link
Collaborator

slorber commented Nov 15, 2024

In #10685 I'm adding escape hatch env variables that could help you execute plugin code in other (non-cwd) siteDir and using custom config file names:

  • DOCUSAURUS_CLI_SITE_DIR
  • DOCUSAURUS_CLI_CONFIG

This is likely not the final implementation but the only idea I have without a breaking change. Should be in the upcoming v3.6.2.

We may be able to do better, but until then at least you have a decent workaround.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
difficulty: advanced Issues that are complex, e.g. large scoping for long-term maintainability. proposal This issue is a proposal, usually non-trivial change
Projects
None yet
Development

No branches or pull requests

4 participants