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

Add the ability to provide custom messages #177

Merged
merged 6 commits into from
Sep 30, 2020

Conversation

GentileFulvio
Copy link
Contributor

@GentileFulvio GentileFulvio commented May 1, 2020

This PR was highly inspired by #156. I hope this fixes #122 . Let me know if you have some feedback


IssueHunt Summary

Referenced issues

This pull request has been submitted to:


@sindresorhus sindresorhus changed the title Add a way to provide custom messages - Fixes #122 Add a way to provide custom messages May 6, 2020
readme.md Outdated Show resolved Hide resolved
readme.md Outdated Show resolved Hide resolved
readme.md Outdated Show resolved Hide resolved
readme.md Outdated Show resolved Hide resolved
readme.md Outdated Show resolved Hide resolved
source/predicates/predicate.ts Outdated Show resolved Hide resolved
@@ -164,6 +166,25 @@ export class Predicate<T = unknown> implements BasePredicate<T> {
});
}

/**
Override the message thrown
Copy link
Owner

Choose a reason for hiding this comment

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

This needs a better and more verbose description.

/**
Override the message thrown

@param newMessage | Either a string containing the new message or a function returning the new message
Copy link
Owner

Choose a reason for hiding this comment

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

This is not the correct doc comment formatting.

readme.md Outdated Show resolved Hide resolved
@GentileFulvio
Copy link
Contributor Author

I think all comments are fixed accordingly

source/predicates/predicate.ts Outdated Show resolved Hide resolved
Function executed when the provided validation fails.
The first argument provided to the function is the provided `value` for the property,
the second argument is the optional `label` for the property.
The returned value will be the error message.
Copy link
Owner

Choose a reason for hiding this comment

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

JSDoc has special syntax for documenting parameters and return value.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Used the @param and @returns syntax

source/predicates/predicate.ts Outdated Show resolved Hide resolved
source/predicates/predicate.ts Outdated Show resolved Hide resolved
@@ -164,6 +172,25 @@ export class Predicate<T = unknown> implements BasePredicate<T> {
});
}

/**
Provide an new error message to be thrown when the validation fails.
Copy link
Owner

Choose a reason for hiding this comment

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

Typo

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

the second argument is the optional `label` for the property.
The returned value will be the error message.
*/
export type ValidatorMessage<T> = (value: T, label?: string) => string;
Copy link
Owner

Choose a reason for hiding this comment

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

It's not a validator message though, it creates the validator message. I think the name here could be improved.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed it to ValidatorMessageBuilder, couldn't really think on anything better. Any suggestions perhaps ?

.url.message('This is no url')
);
//=> ArgumentError: This is no url
```
Copy link
Owner

Choose a reason for hiding this comment

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

The readme and TS doc comments should be in sync as much as possible. These examples are missing from the TS doc comments.

Copy link
Contributor Author

@GentileFulvio GentileFulvio Jun 6, 2020

Choose a reason for hiding this comment

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

Added two examples to the message function in source/predicates/predicate.ts. Would you like me to add all of the examples above ?

Copy link
Owner

Choose a reason for hiding this comment

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

Yes, if they are in the readme, they should go in index.d.ts.

Copy link
Owner

Choose a reason for hiding this comment

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

Also make sure the formatting is the same. It's not now.

@sindresorhus sindresorhus changed the title Add a way to provide custom messages Add the ability to provide custom messages Sep 30, 2020
@sindresorhus sindresorhus merged commit 57a1f5b into sindresorhus:master Sep 30, 2020
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

Successfully merging this pull request may close these issues.

Custom error message
3 participants