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

Allow readonly arrays as providers, imports, etc. #13326

Open
1 task done
Alexnortung opened this issue Mar 15, 2024 · 3 comments
Open
1 task done

Allow readonly arrays as providers, imports, etc. #13326

Alexnortung opened this issue Mar 15, 2024 · 3 comments
Labels
needs triage This issue has not been looked into type: enhancement 🐺

Comments

@Alexnortung
Copy link

Is there an existing issue that is already proposing this?

  • I have searched the existing issues

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

Not really a problem, just makes things easier if you use as const

Describe the solution you'd like

I would like these types to allow readonly arrays.

Teachability, documentation, adoption, migration strategy

I don't think this is necessary

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

I'm working on a module which should have some of the same providers for forRoot and forRootAsync, to get these I call a function that returns a readonly array. I could just not use a readonly array, but I don't see it as a bad thing to utilize the typescript type system, so I don't accidentally do something weird with the array that was returned.

@Module({})
export class MindsdbModule {
  public static forRoot(options?: MindsdbModuleOptions): DynamicModule {
    const mindsdbModuleOptions = {
      provide: MINDSDB_MODULE_OPTIONS,
      useValue: options,
    };

    const providers = [
      ...this.getBaseProviders(),
      mindsdbModuleOptions,
    ];
    return {
      global: true,
      module: MindsdbModule,
      imports: [ScheduleModule.forRoot()],
      providers,
      exports: [MindsdbService, RetrainJobService],
    };
  }

  private static getBaseProviders() {
    return [
      MindsdbService,
      RetrainJobService,
    ] as const;
  }

  private static getBaseExports() {
    return [
      MindsdbService,
      RetrainJobService,
    ] as const;
  }

  public static forRootAsync(options: MindsdbModuleAsyncOptions): DynamicModule {
    const providers = [
      {
        provide: MINDSDB_MODULE_OPTIONS,
        useFactory: options.useFactory,
        inject: options.inject || [],
      },
      ...this.getBaseProviders(),
      ...(options.extraProviders || []),
    ];

    return {
      module: MindsdbModule,
      imports: options.imports,
      providers,
      exports: this.getBaseExports(),
    };
  }
}
@Alexnortung Alexnortung added needs triage This issue has not been looked into type: enhancement 🐺 labels Mar 15, 2024
@micalevisk
Copy link
Member

I guess that's just a matter of changing the following file

@Alexnortung
Copy link
Author

Yes, I think so too

@micalevisk
Copy link
Member

I didn't manage to change that without introducing a breaking changing at type level

my attempt
type AnArray<T> = Array<T> | ReadonlyArray<T>;

/**
 * Interface defining the property object that describes the module.
 *
 * @see [Modules](https://docs.nestjs.com/modules)
 *
 * @publicApi
 */
export interface ModuleMetadata {
  /**
   * Optional list of imported modules that export the providers which are
   * required in this module.
   */
  imports?: AnArray<
    Type<any> | DynamicModule | Promise<DynamicModule> | ForwardReference
  >;
  /**
   * Optional list of controllers defined in this module which have to be
   * instantiated.
   */
  controllers?: AnArray<Type<any>>;
  /**
   * Optional list of providers that will be instantiated by the Nest injector
   * and that may be shared at least across this module.
   */
  providers?: AnArray<Provider>;
  /**
   * Optional list of the subset of providers that are provided by this module
   * and should be available in other modules which import this module.
   */
  exports?: AnArray<
    | DynamicModule
    | Promise<DynamicModule>
    | string
    | symbol
    | Provider
    | ForwardReference
    | Abstract<any>
    | Function
  >;
}

it's breaking the following line

await this.addDynamicModules(imports, scope);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs triage This issue has not been looked into type: enhancement 🐺
Projects
None yet
Development

No branches or pull requests

2 participants