search

pnp / cli-microsoft365

Manage Microsoft 365 and SharePoint Framework projects on any platform
https://aka.ms/cli-m365
MIT License
887 stars 314 forks source link

Refactor command configuration #3409

Closed waldekmastykarz closed 1 year ago

waldekmastykarz commented 2 years ago

Refactor command configuration so that we can have a central place for configuring validation, options, telemetry, etc.

Update commands:

OK, to make things more tangible, what if we had something like this:

// base Command class that contains logic shared by all commands
abstract class Command {
  // arrays that hold delegates for configuring the command
  public options: CommandOption[] = [];
  public validations: ((args: CommandArgs) => boolean | string)[] = [];
  public telemetry: ((args: CommandArgs) => void)[] = [];

  protected telemetryProperties: any = {};

  // we configure the command directly in the constructor so that we don't
  // need to do any extra calls anywhere else
  constructor() {
    // we break down configuration into separate functions for readability
    this.#initOptions();
    this.#initValidations();
    this.#initTelemetry();

    // these functions must be defined with # so that they're truly private
    // otherwise you'll get a ts2415 error (Types have separate declarations of a private property 'x'.)
    // another way to avoid it is to init everything directly in the constructor
    // without breaking it down into separate functions
    // `private` in TS is a design-time flag and private members end-up being
    // regular class properties that would collide on runtime, which is why we need the extra `#`
  }

  #initTelemetry(): void {
    // rather than configuring the telemetryProperties object directly,
    // we return a function that we'll run later on, because when the command
    // is instantiated, args haven't been parsed and validated yet
    this.telemetry.push(
      (args) => {
        this.telemetryProperties.query = args.options.query;
        this.telemetryProperties.output = args.options.output || 'json';
      }
    );
  }

  #initOptions(): void {
    this.options.push(
      { option: '--query [query]' },
      {
        option: '-o, --output [output]',
        autocomplete: ['csv', 'json', 'text']
      },
      { option: '--verbose' },
      { option: '--debug' }
    );
  }

  #initValidations(): void {
    this.validations.push(
      // common validation logic that we move from the CLI runtime to the Command
      (args) => this.validateRequiredOptions(args),
      (args) => this.validateOptionSets(args),
      // command-specific validation function; it's up to us to decide if
      // each validation is a separate function or if we want to move them
      // as-is from the current `validate` method in each command
      (args) => {
        if (args.options.output) {
          const outputOption = this.options.find(o => o.option.indexOf('--output') > -1);
          if (outputOption!.autocomplete!.indexOf(args.options.output) < 0) {
            return `${args.options.output} is not a valid output. Allowed values are ${outputOption!.autocomplete!.join(', ')}`;
          }
        }

        return true;
      }
    );
  }

  // validation logic from the CLI runtime that we move to the Command
  private validateRequiredOptions(args: CommandArgs): boolean | string {
  }
  private validateOptionSets(args: CommandArgs): boolean | string {
  }

  // called by the CLI runtime to validate command's args
  public validate(args: CommandArgs): boolean | string {
    for (const validation of this.validations) {
      const result = validation(args);
      if (typeof result === 'string') {
        return result;
      }
    }

    return true;
  }
}

// sample command
class SpoSiteGetCommand extends Command {
  constructor() {
    // we call super to include config from the base command class
    super();

    this.#initOptions();
    this.#initValidations();
    this.#initTelemetry();
  }

  #initTelemetry(): void {
    this.telemetry.push(
      (args) => {
        this.telemetryProperties.url = typeof args.options.url !== 'undefined';
        this.telemetryProperties.id = typeof args.options.id !== 'undefined';
      }
    );
  }

  #initOptions(): void {
    this.options.push(
      { option: '--url [url]' },
      { option: '--id [id]' }
    );
  }

  #initValidations(): void {
    this.validations.push(
      (args) => {
        if (args.options.url) {
          return validation.isValidSharePointUrl(args.options.url);
        }

        return true;
      }
    );
  }
}

I haven't included everything in the example above (types, option sets, aliases, default properties, processing options), but I hope the example gives a clearer idea of the possible direction we could take. Features missing from this example would be basically 'more of the same' (array with functions that the runtime would execute).

Originally posted by @waldekmastykarz in https://github.com/pnp/cli-microsoft365/issues/3218#issuecomment-1154815279

waldekmastykarz commented 2 years ago

I've updated the base Command structure and other base classes, updated the CLI runtime to use the new structure and updated 3 commands (aad app add, spo site list and spo site get) to see how it would look like: https://github.com/pnp/cli-microsoft365/compare/main...waldekmastykarz:command-refactoring

waldekmastykarz commented 1 year ago

We're at 100% coverage again. Next step is for me to sync the latest changes that have been added to the CLI over the last few weeks.

martinlingstuyl commented 1 year ago

Respect for the speed with which you've implemented this @waldekmastykarz! 👏

waldekmastykarz commented 1 year ago

All ready, rebased with latest changes from main. Will submit a PR and merge in a couple of days.

martinlingstuyl commented 1 year ago

Well done @waldekmastykarz!