Change signature of `TestingModuleBuilder` to ensure legibility when having lots of overrides

[leandroluk]

trafficstars

Is there an existing issue that is already proposing this?

• [X] I have searched the existing issues

Is your feature request related to a problem? Please describe it

This feature is related only with visual structure when using lots of overrides.

Describe the solution you'd like

The current behavior using eslint or any other formatter will create a structure like this when we have a lots of overrides:

import {Test} from '@nestjs/testing';
// ... omitting imports to brevity

const firstValue = { execute: jest.fn() }
const secondValue = { execute: jest.fn() }
const thirdValue = { execute: jest.fn() }
const fourthValue = { execute: jest.fn() }
const fifthValue = { execute: jest.fn() }

const builder = await Test
  .createTestingModule({ imports: [AppModule] })
  .overrideProvider(FirstService)
  .useValue(firstValue)
  .overrideProvider(SecondService)
  .useValue(secondValue)
  .overrideProvider(ThirdService)
  .useValue(thirdValue)
  .overrideProvider(FourthService)
  .useValue(fourthValue)
  .overrideProvider(fifthService)
  .useValue(fifthValue)
  .compile()

This doesn't offer a great legibility for overrides and is impossible to format chained overrides to improve legibility like this:

const builder = await Test
  .createTestingModule({ imports: [AppModule] })
  .overrideProvider(FirstService).useValue(firstValue)
  .overrideProvider(SecondService).useValue(secondValue)
  .overrideProvider(ThirdService).useValue(thirdValue)
  .overrideProvider(FourthService).useValue(fourthValue)
  .overrideProvider(fifthService).useValue(fifthValue)
  .compile()

So I use a wrapper replacing the default "override{{ArtfactName}}" methods and add a new "override" more generic. The final code are like this

class Builder {
  constructor(private builder: TestingModuleBuilder) {}

  setLogger(testingLogger: LoggerService): this {
    this.builder = this.builder.setLogger(testingLogger);
    return this;
  }

  useMocker(mocker: MockFactory): this {
    this.builder = this.builder.useMocker(mocker);
    return this;
  }

  override<T = any>(override: Builder.Override<T>): this {
    if (override.filter) {
      const {filter, ...overrideBy} = override;
      return this.overrideFilter(filter, overrideBy);
    } else if (override.guard) {
      const {guard, ...overrideBy} = override;
      return this.overrideGuard(guard, overrideBy);
    } else if (override.interceptor) {
      const {interceptor, ...overrideBy} = override;
      return this.overrideInterceptor(interceptor, overrideBy);
    } else if (override.provider) {
      const {provider, ...overrideBy} = override;
      return this.overrideProvider(provider, overrideBy);
    } else {
      throw new TypeError(`Unknown override argument: ${override}`);
    }
  }

  overridePipe<T = any>(typeOrToken: T, overrideBy: Builder.Override.By): this {
    const overridePipe = this.builder.overridePipe(typeOrToken);
    if (overrideBy.useClass) {
      this.builder = overridePipe.useClass(overrideBy.useClass);
    } else if (overrideBy.useFactory) {
      this.builder = overridePipe.useFactory(overrideBy.useFactory);
    } else if (overrideBy.useValue) {
      this.builder = overridePipe.useValue(overrideBy.useValue);
    } else {
      throw new TypeError(`Unknown overrideBy argument: ${overrideBy}`);
    }
    return this;
  }

  overrideFilter<T = any>(typeOrToken: T, overrideBy: Builder.Override.By): this {
    const overridePipe = this.builder.overrideFilter(typeOrToken);
    if (overrideBy.useClass) {
      this.builder = overridePipe.useClass(overrideBy.useClass);
    } else if (overrideBy.useFactory) {
      this.builder = overridePipe.useFactory(overrideBy.useFactory);
    } else if (overrideBy.useValue) {
      this.builder = overridePipe.useValue(overrideBy.useValue);
    } else {
      throw new TypeError(`Unknown overrideBy argument: ${overrideBy}`);
    }
    return this;
  }

  overrideGuard<T = any>(typeOrToken: T, overrideBy: Builder.Override.By): this {
    const overridePipe = this.builder.overrideGuard(typeOrToken);
    if (overrideBy.useClass) {
      this.builder = overridePipe.useClass(overrideBy.useClass);
    } else if (overrideBy.useFactory) {
      this.builder = overridePipe.useFactory(overrideBy.useFactory);
    } else if (overrideBy.useValue) {
      this.builder = overridePipe.useValue(overrideBy.useValue);
    } else {
      throw new TypeError(`Unknown overrideBy argument: ${overrideBy}`);
    }
    return this;
  }

  overrideInterceptor<T = any>(typeOrToken: T, overrideBy: Builder.Override.By): this {
    const overridePipe = this.builder.overrideInterceptor(typeOrToken);
    if (overrideBy.useClass) {
      this.builder = overridePipe.useClass(overrideBy.useClass);
    } else if (overrideBy.useFactory) {
      this.builder = overridePipe.useFactory(overrideBy.useFactory);
    } else if (overrideBy.useValue) {
      this.builder = overridePipe.useValue(overrideBy.useValue);
    } else {
      throw new TypeError(`Unknown overrideBy argument: ${overrideBy}`);
    }
    return this;
  }

  overrideProvider<T = any>(typeOrToken: T, overrideBy: Builder.Override.By): this {
    const overridePipe = this.builder.overrideProvider(typeOrToken);
    if (overrideBy.useClass) {
      this.builder = overridePipe.useClass(overrideBy.useClass);
    } else if (overrideBy.useFactory) {
      this.builder = overridePipe.useFactory(overrideBy.useFactory);
    } else if (overrideBy.useValue) {
      this.builder = overridePipe.useValue(overrideBy.useValue);
    } else {
      throw new TypeError(`Unknown overrideBy argument: ${overrideBy}`);
    }
    return this;
  }

  overrideModule(moduleToOverride: ModuleDefinition, newModule: ModuleDefinition): this {
    this.builder = this.builder.overrideModule(moduleToOverride).useModule(newModule);
    return this;
  }

  async compile(options?: Pick<NestApplicationContextOptions, 'snapshot' | 'preview'>): Promise<TestingModule> {
    return await this.builder.compile(options);
  }
}
export namespace Builder {
  type XOR<T, U> = T | U extends object
    ? (T & {[K in Exclude<keyof U, keyof T>]?: never}) | (U & {[K in Exclude<keyof T, keyof U>]?: never})
    : T | U;

  export type Override<T = any> = XOR<Override.Pipe<T>, XOR<Override.Filter<T>, XOR<Override.Guard<T>, XOR<Override.Interceptor<T>, Override.Provider<T>>>>>; // prettier-ignore
  export namespace Override {
    export type Pipe<T = any> = {pipe: T} & By;
    export type Filter<T = any> = {filter: T} & By;
    export type Guard<T = any> = {guard: T} & By;
    export type Interceptor<T = any> = {interceptor: T} & By;
    export type Provider<T = any> = {provider: T} & By;

    export type By = XOR<By.Class, XOR<By.Factory, By.Value>>;
    export namespace By {
      export type Class = {useClass: Parameters<DefaultOverridedBy['useClass']>[0]};
      export type Factory = {useFactory: Parameters<DefaultOverridedBy['useFactory']>[0]};
      export type Value = {useValue: Parameters<DefaultOverridedBy['useValue']>[0]};
    }
  }
}

export function createTestingModule(metadata: ModuleMetadata): Builder {
  return new Builder(Test.createTestingModule(metadata));
}

So, today I had created a Wrapper replacing the default "override{{ArtifactName}}" methods and add a new "override" accepting few combinations with final code like this:

Teachability, documentation, adoption, migration strategy

For refactor and implement this new feature its only required to verify the number of arguments received switching between the old behavior and the new one, some like this:

import { OverrideBy, TestingModuleBuilder } from '@nestjs/testing';

class NewTestingModuleBuilder extends TestingModuleBuilder {
  overridePipe<T = any>(typeOrToken: T): OverrideBy;
  overridePipe<T = any>(typeOrToken: T, overrideBy?: NewTestingModuleBuilder.Override.By): OverrideBy | this {
    if (!overrideBy) {
      return super.overridePipe(typeOrToken) as OverrideBy
    } else {
      const overridePipe = super.overridePipe(typeOrToken);
      if (overrideBy.useClass) {
        return overridePipe.useClass(overrideBy.useClass) as this;
      } 
      if (overrideBy.useFactory) {
        return overridePipe.useFactory(overrideBy.useFactory!) as this;
      } 
      if (overrideBy.useValue) {
        return overridePipe.useValue(overrideBy.useValue) as this;
      } 
      throw new TypeError(`Unknown overrideBy argument: ${overrideBy}`);
    }
  }
}
export namespace NewTestingModuleBuilder {
  type XOR<T, U> = T | U extends object
    ? (T & {[K in Exclude<keyof U, keyof T>]?: never}) | (U & {[K in Exclude<keyof T, keyof U>]?: never})
    : T | U;

  export type Override<T = any> = XOR<Override.Pipe<T>, XOR<Override.Filter<T>, XOR<Override.Guard<T>, XOR<Override.Interceptor<T>, Override.Provider<T>>>>>; // prettier-ignore
  export namespace Override {
    export type Pipe<T = any> = {pipe: T} & By;
    export type Filter<T = any> = {filter: T} & By;
    export type Guard<T = any> = {guard: T} & By;
    export type Interceptor<T = any> = {interceptor: T} & By;
    export type Provider<T = any> = {provider: T} & By;

    export type By = XOR<By.Class, XOR<By.Factory, By.Value>>;
    export namespace By {
      export type Class = {useClass: Parameters<OverrideBy['useClass']>[0]};
      export type Factory = {useFactory: Parameters<OverrideBy['useFactory']>[0]};
      export type Value = {useValue: Parameters<OverrideBy['useValue']>[0]};
    }
  }
}

What is the motivation / use case for changing the behavior?

I believe that the more readable and simple to use the library is, the more followers there will be of it, so these changes will make the tests easier and more objective to read without impacting the current code.