Skip to content

Add enabled flag to string.trim()#1532

Merged
Marsup merged 3 commits intohapijs:masterfrom
rokoroku:patch-1
Jul 30, 2018
Merged

Add enabled flag to string.trim()#1532
Marsup merged 3 commits intohapijs:masterfrom
rokoroku:patch-1

Conversation

@rokoroku
Copy link
Contributor

Resolves #1525


it('disable existing trim flag when passing enabled: false', () => {

const schema = Joi.string().trim().trim(false);
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like .trim() is repeated twice here.

I also don't see any tests for .trim(true) or when the parameter to trim is not a boolean to validate that we are checking the type like other calls (ie. .trim(42)).

Last piece on this part would be documentation, this new functionality should be listed in the docs also.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, I just follow the implementation of the .truncate(enabled?)

But if it really needed, I'll write some assertion here.

trim(enabled) {

const obj = this._test('trim', undefined, function (value, state, options) {
enabled = enabled === undefined ? true : !!enabled;
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of coercing the value here where someone making the call .trim('yes please') would enable trim I would expect us to check the data type like we do in other validation methods. For example: Hoek.assert(typeof base64Options.paddingRequired === 'undefined' || typeof base64Options.paddingRequired === 'boolean', Hoek.assert(typeof base64Options.paddingRequired === 'undefined' || typeof base64Options.paddingRequired === 'boolean', 'paddingRequired must be boolean');

@Marsup Marsup added this to the 13.5.0 milestone Jul 3, 2018
@Marsup Marsup self-assigned this Jul 3, 2018
@Marsup Marsup added the feature New functionality or improvement label Jul 3, 2018
@Marsup Marsup merged commit 3372df0 into hapijs:master Jul 30, 2018
Marsup added a commit that referenced this pull request Jul 30, 2018
@rokoroku rokoroku deleted the patch-1 branch August 2, 2018 07:50
@lock
Copy link

lock bot commented Jan 9, 2020

This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

feature New functionality or improvement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add boolean flag to Joi.string().trim()

3 participants