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

Let ConfigService know about the validation defined in ConfigModule#forRoot to avoid type assertions #668

Closed
micalevisk opened this issue Sep 12, 2021 · 2 comments

Comments

@micalevisk
Copy link
Member

micalevisk commented Sep 12, 2021

I'm submitting a...


[ ] Regression 
[ ] Bug report
[x] Feature request
[ ] Documentation issue or request
[ ] Support request => Please do not submit support request here, instead post your question on Stack Overflow.

Current behavior

Regardless if we set up an schema validation or not, the type of ConfigService#get is undefined (when strictNullChecks:true from TSC) because ConfigService doesn't know about ConfigModule#forRoot usage. Thus we end up using the non-null assertion operator or type assertions whenever we use ConfigService#get

@Injectable()
export class AppService {
  constructor(private readonly configService: ConfigService<{ foo: string, a: {b:number} }>) {}

  getFoo(): string {
    const a = this.configService.get('a')
    const b: number = a!.b
    //                 ^ This could be annoying if we type it many times!!
    return this.configService.get('foo')!
    //                                  ^
  }
}

Expected behavior

We can tell to ConfigService that the validation was properly set up, then ConfigService#get cannot return undefined when we pass some valid prop

No more type assertions!

  getFoo(): string {
    const a = this.configService.get('foo')
    const b: number = a.b

    this.configService.get('bar') // TSC ERROR, as usual

    return this.configService.get('foo')
  }

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

Improve developer experience.

image from discord

Possible solution

Add another (optional) type arg to ConfigService generics:

@Injectable()
export class AppService {
  constructor(private readonly configService: ConfigService<{ foo: string }, true>) {}
  //                                                                         ^^^^ Telling to ConfigService
  //                                                                              that everything was validated
}

I prototyped this one here:

config.service.d.ts we become something like
import { NoInferType, Path, PathValue } from './types';
export interface ConfigGetOptions {
    infer: true;
}

/**
 * `WithUndefined<HasValidation, T>`
 *
 * Evaluates to `T` if `HasValidation` is `true`. Otherwise, `T | undefined`
 */
type WithUndefined<HasValidation extends boolean, T> = HasValidation extends true ? T : T | undefined

export declare class ConfigService<K = Record<string, unknown>, HasValidation extends boolean = false> {
    private readonly internalConfig;
    private set isCacheEnabled(value);
    private get isCacheEnabled();
    private readonly cache;
    private _isCacheEnabled;
    constructor(internalConfig?: Record<string, any>);

    // get<T = any>(propertyPath: keyof K): T | undefined;
    get<T = any>(propertyPath: keyof K): WithUndefined<HasValidation, T>;

    // get<T = K, P extends Path<T> = any, R = PathValue<T, P>>(propertyPath: P, options: ConfigGetOptions): R | undefined;
    get<T = K, P extends Path<T> = any, R = PathValue<T, P>>(propertyPath: P, options: ConfigGetOptions): WithUndefined<HasValidation, R>;

    get<T = any>(propertyPath: keyof K, defaultValue: NoInferType<T>): T;

    // get<T = K, P extends Path<T> = any, R = PathValue<T, P>>(propertyPath: P, defaultValue: NoInferType<R>, options: ConfigGetOptions): R | undefined;
    get<T = K, P extends Path<T> = any, R = PathValue<T, P>>(propertyPath: P, defaultValue: NoInferType<R>, options: ConfigGetOptions): WithUndefined<HasValidation, R>;

    private getFromCache;
    private getFromValidatedEnv;
    private getFromProcessEnv;
    private getFromInternalConfig;
    private setInCacheIfDefined;
    private isGetOptionsObject;
}

Environment


Nest version: ^8
@nestjs/config: ^1
typescript: ^4.4
 
For Tooling issues:
- Node version: XX  
- Platform: Linux

Others:

@micalevisk micalevisk changed the title Let ConfigService know about the validation defined in ConfigRoot to avoid type assertions Let ConfigService know about the validation defined in ConfigModule#forRoot to avoid type assertions Sep 12, 2021
@kamilmysliwiec
Copy link
Member

Looks like a nice addition! Would you like to create a PR for this issue?

@kamilmysliwiec
Copy link
Member

Let's track this here #676

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

No branches or pull requests

2 participants