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(common): improve extensibility on few built-in pipes #9496

Merged
merged 1 commit into from May 13, 2022
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
22 changes: 20 additions & 2 deletions packages/common/pipes/parse-bool.pipe.ts
Expand Up @@ -48,14 +48,32 @@ export class ParseBoolPipe
value: string | boolean,
metadata: ArgumentMetadata,
): Promise<boolean> {
if (value === true || value === 'true') {
if (this.isTrue(value)) {
return true;
}
if (value === false || value === 'false') {
if (this.isFalse(value)) {
return false;
}
throw this.exceptionFactory(
'Validation failed (boolean string is expected)',
);
}

/**
* @param value currently processed route argument
* @returns `true` if `value` is said 'true', ie., if it is equal to the boolean
* `true` or the string `"true"`
*/
protected isTrue(value: string | boolean): boolean {
return value === true || value === 'true';
}

/**
* @param value currently processed route argument
* @returns `true` if `value` is said 'false', ie., if it is equal to the boolean
* `false` or the string `"false"`
*/
protected isFalse(value: string | boolean): boolean {
return value === false || value === 'false';
}
}
18 changes: 13 additions & 5 deletions packages/common/pipes/parse-float.pipe.ts
Expand Up @@ -39,15 +39,23 @@ export class ParseFloatPipe implements PipeTransform<string> {
* @param metadata contains metadata about the currently processed route argument
*/
async transform(value: string, metadata: ArgumentMetadata): Promise<number> {
const isNumeric =
['string', 'number'].includes(typeof value) &&
!isNaN(parseFloat(value)) &&
isFinite(value as any);
if (!isNumeric) {
if (!this.isNumeric(value)) {
throw this.exceptionFactory(
'Validation failed (numeric string is expected)',
);
}
return parseFloat(value);
}

/**
* @param value currently processed route argument
* @returns `true` if `value` is a valid float number
*/
protected isNumeric(value: string): boolean {
return (
['string', 'number'].includes(typeof value) &&
Copy link
Member Author

Choose a reason for hiding this comment

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

side note: idk why we're checking if typeof value === 'number' as value:string but I didn't change this because I'm unsure if value could be number somehow

Maybe we need to adjust the type def of value here and in ParseIntPipe too?

!isNaN(parseFloat(value)) &&
isFinite(value as any)
);
}
}
18 changes: 13 additions & 5 deletions packages/common/pipes/parse-int.pipe.ts
Expand Up @@ -44,15 +44,23 @@ export class ParseIntPipe implements PipeTransform<string> {
* @param metadata contains metadata about the currently processed route argument
*/
async transform(value: string, metadata: ArgumentMetadata): Promise<number> {
const isNumeric =
['string', 'number'].includes(typeof value) &&
/^-?\d+$/.test(value) &&
isFinite(value as any);
if (!isNumeric) {
if (!this.isNumeric(value)) {
throw this.exceptionFactory(
'Validation failed (numeric string is expected)',
);
}
return parseInt(value, 10);
}

/**
* @param value currently processed route argument
* @returns `true` if `value` is a valid integer number
*/
protected isNumeric(value: string): boolean {
return (
['string', 'number'].includes(typeof value) &&
/^-?\d+$/.test(value) &&
isFinite(value as any)
);
}
}