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

Adds support for lazy commands #154

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

Adds support for lazy commands #154

wants to merge 8 commits into from

Conversation

arcanis
Copy link
Owner

@arcanis arcanis commented Nov 21, 2023

This PR implements lazy commands, as described in #151.

Of note, I didn't go the road I initially described in #151 (comment). I was worried the implementation would be very complicated and hard to follow / maintain, and perhaps would suffer in performances.

Instead, I opted to implement an helper that, given a argument list, can generate the set of commands that will be relevant. It means we essentially process the argv array twice, but since it's usually of small size, that shouldn't matter too much.

Two implementations are currently provided:

  • Cli.lazyTree checks a recursive record to see whether subcommands exist that match the provided path.
  • Cli.lazyFileSystem incrementally checks the filesystem to see if files matching the provided command paths exist.

When writing large CLI application, we find ourselves in a pickle. Let's say we have commands similar to:

import something from './lib/something';
import somethingElse from './lib/somethingElse';

export class MyCommand extends Command {
  async execute() {
    something();
    somethingElse();
  }
}

The something and somethingElse functions aren't needed until MyCommand is executed, but since they are in a top-level import the generated code will still import them before even evaluating the command file. At the scale of a large application, those imports start to slow down the startup by a significant factor. We can mitigate it a little by doing something like this:

export class MyCommand extends Command {
  async execute() {
    const [{default: something}, {default: somethingElse}] = await Promise.all([
      import('something'),
      import('somethingElse'),
    ]);

    something();
    somethingElse();
  }
}

But that's really verbose, and that's not even what people doing things like this do (they instead just call import multiple times in a row, like top-level imports, except that it prevents the runtime from fetching / parsing the modules in parallel, making sync something that could be parallelized).

A second problem is that even if the imports are moved into execute, just running files has a cost. They need to be read, parsed, evaluated, and all that when they don't actually contribute to anything at all for the purpose of the command parsing. This problem is exacerbated when using transpilers, as the cost can easily reach hundreds of ms for larger CLIs.

The first point can be solved by the Deferring Module Evaluation proposal, but it's currently still at stage 2 (cc @nicolo-ribaudo in case you're interested by this thread / practical use case), and even with that we'd still have the problem of the files being executed at all (probably not as much a problem if you don't use a transpiler).

Ideally, I'd like to find a way to solve both points.

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.

1 participant