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

feat: let ConfigService type know about schema validation #676

Merged
merged 3 commits into from
Nov 3, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 17 additions & 3 deletions lib/config.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,17 @@ import {
} from './config.constants';
import { NoInferType, Path, PathValue } from './types';

/**
* `ExcludeUndefinedIf<ExcludeUndefined, T>
*
* If `ExcludeUndefined` is `true`, remove `undefined` from `T`.
* Otherwise, constructs the type `T` with `undefined`.
*/
type ExcludeUndefinedIf<
ExcludeUndefined extends boolean,
T,
> = ExcludeUndefined extends true ? Exclude<T, undefined> : T | undefined;

export interface ConfigGetOptions {
/**
* If present, "get" method will try to automatically
Expand All @@ -19,7 +30,10 @@ export interface ConfigGetOptions {
}

@Injectable()
export class ConfigService<K = Record<string, unknown>> {
export class ConfigService<
K = Record<string, unknown>,
WasValidated extends boolean = false,
> {
private set isCacheEnabled(value: boolean) {
this._isCacheEnabled = value;
}
Expand All @@ -42,7 +56,7 @@ export class ConfigService<K = Record<string, unknown>> {
* based on property path (you can use dot notation to traverse nested object, e.g. "database.host").
* @param propertyPath
*/
get<T = any>(propertyPath: keyof K): T | undefined;
get<T = any>(propertyPath: keyof K): ExcludeUndefinedIf<WasValidated, T>;
/**
* Get a configuration value (either custom configuration or process environment variable)
* based on property path (you can use dot notation to traverse nested object, e.g. "database.host").
Expand All @@ -52,7 +66,7 @@ export class ConfigService<K = Record<string, unknown>> {
get<T = K, P extends Path<T> = any, R = PathValue<T, P>>(
propertyPath: P,
options: ConfigGetOptions,
): R | undefined;
): ExcludeUndefinedIf<WasValidated, R>;
/**
* Get a configuration value (either custom configuration or process environment variable)
* based on property path (you can use dot notation to traverse nested object, e.g. "database.host").
Expand Down
5 changes: 5 additions & 0 deletions tests/jest-e2e.json
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
{
"globals": {
"ts-jest": {
"tsconfig": "./tests/tsconfig.json"
}
},
"moduleFileExtensions": ["js", "json", "ts"],
"rootDir": ".",
"testEnvironment": "node",
Expand Down
29 changes: 29 additions & 0 deletions tests/src/app.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,44 @@ import { ConfigService } from '../../lib/config.service';
import databaseConfig from './database.config';
import nestedDatabaseConfig from './nested-database.config';

type Config = {
database: ConfigType<typeof databaseConfig> & {
driver: ConfigType<typeof nestedDatabaseConfig>
};
};

@Module({})
export class AppModule {
constructor(
private readonly configService: ConfigService,
// The following is the same object as above but narrowing its types
private readonly configServiceNarrowed: ConfigService<Config, true>,
@Optional()
@Inject(databaseConfig.KEY)
private readonly dbConfig: ConfigType<typeof databaseConfig>,
) {}

/**
* This method is not meant to be used anywhere! It just here for testing
* types defintions while runnig test suites (in some sort).
* If some typings doesn't follows the requirements, Jest will fail due to
* TypeScript errors.
*/
private noop(): void {
Copy link
Member Author

@micalevisk micalevisk Sep 30, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if this strategy is that bad tho. Since I don't want to add libs such as tsd or dtslint to check if my proposing feature for ConfigService is working as expected (and will keep working in the future), I've added this method to do the type checking as a side-effect of running npm run test:integration :p

I could drop this commit if you want to. Or I could add one more to test the this.configServiceNarrowed.get(x) use case (ie., relying on type inference)

// Arrange
const identityString = (v: string) => v;
const identityNumber = (v: number) => v;
// Act
const knowConfig = this.configServiceNarrowed.get<Config['database']>('database');
// Assert
// We don't need type assertions bellow anymore since `knowConfig` is not
// expected to be `undefined` beforehand.
identityString(knowConfig.host);
identityNumber(knowConfig.port);
identityString(knowConfig.driver.host);
identityNumber(knowConfig.driver.port);
}

static withCache(): DynamicModule {
return {
module: AppModule,
Expand Down
14 changes: 14 additions & 0 deletions tests/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{
"extends": "../tsconfig.json",
"compilerOptions": {
"rootDir": "../",
"strictNullChecks": true
},
"include": [
"**/*.spec.ts",
],
"exclude": [
"node_modules",
"dist"
]
}