lib: extract validateString validator - #22101
Conversation
Sorry, something went wrong.
This comment has been minimized.
There was a problem hiding this comment.
What about trying to create another new function called "isEmptyString" to check whether your string value is empty:
function isEmptyString (s){
return (typeof s === 'string' && s.trim() === '');
}
Most of your functions requires a param value that isn't null or empty.
Maybe when both of the things meet, we can continue going on.
Sorry, something went wrong.
@Maledong Not sure if this is the case -- where do you see this check? I'd rather not add new validations in this PR, since that would be a semver-major change |
Sorry, something went wrong.
|
@Maledong Arguably s.trim() === '' is ERR_INVALID_ARG_VALUE and type of s !== 'string' is ERR_INVALID_ARG_TYPE so it may not make sense to mix the validations together. And yeah that'd be semver-major for a couple of APIs. |
Sorry, something went wrong.
Sorry, something went wrong.
There was a problem hiding this comment.
Name changed from "options.path" to "path". I'm on mobile so can't check which one is correct.
Sorry, something went wrong.
There was a problem hiding this comment.
Technically both are valid.
Sorry, something went wrong.
There was a problem hiding this comment.
I think changing the name was a mistake, but going to put this back to options.path instead of path to make sure this is definitely not semver-major :)
Sorry, something went wrong.
|
CI again: https://ci.nodejs.org/job/node-test-pull-request/16182/ (Didn't use Resume Build because windows-fanned failed and Resume Build does not cooperate with windows-fanned.) |
Sorry, something went wrong.
Sorry, something went wrong.
Sorry, something went wrong.
|
Still LGMT. |
Sorry, something went wrong.
Pulls out a common argument validator to `internal/validators`
|
Landed in e570ae7, thank you for the reviews! |
Sorry, something went wrong.
Pulls out a common argument validator to `internal/validators` PR-URL: #22101 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Pulls out a common argument validator to `internal/validators` PR-URL: #22101 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs#24863 Refs: nodejs#22101 Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
PR-URL: nodejs#24960 Refs: nodejs#22101 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
PR-URL: nodejs#24863 Refs: nodejs#22101 Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
PR-URL: nodejs#24960 Refs: nodejs#22101 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Pulls out a common argument validator to `internal/validators` PR-URL: nodejs/node#22101 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Pulls out a common argument validator to internal/validators
Checklist