diff --git a/index.js b/index.js index dcb4964..ced8952 100644 --- a/index.js +++ b/index.js @@ -2,8 +2,11 @@ // TODO: Rename this to something more appropriate, like any/anyOf? How to make sure that doesn't cause confusion with `oneOf`? +const splitFilter = require("split-filter"); + const ValidationError = require("@validatem/error"); const combinator = require("@validatem/combinator"); +const validationResult = require("@validatem/validation-result"); module.exports = function (alternatives) { if (!Array.isArray(alternatives)) { @@ -27,13 +30,32 @@ module.exports = function (alternatives) { } } - let errorList = allErrors - .map((error) => { - return `"${error.message}"`; - }) - .join(", "); + /* + We want to separate out the errors that occurred at this "level"; that is, errors that *didn't* originate from a nested validation combinator like `has-shape` or `array-of`. + + Otherwise, the user could get very confusing errors that combine the `either` alternatives and some deeper validation error into a single list, without denoting the path. + + An example of such a confusing error: + - At options -> credentials: Must satisfy at least one of: "Must be a plain object (eg. object literal)", "Encountered an unexpected property 'foo'" + + With this implementation, it becomes a perfectly reasonable error instead: + - At options -> credentials -> 1: Encountered an unexpected property 'foo'" + */ - throw new ValidationError(`Must satisfy at least one of: ${errorList}`, { errors: allErrors }); + let [ sameLevelErrors, nestedErrors ] = splitFilter(allErrors, (error) => { + return (error.path.length === 0); + }); + + if (nestedErrors.length > 0) { + // At least one of the alternatives *did* match, but it failed somewhere further down the tree. Chances are that the user intended to match that branch, so we pretend that the other alternatives don't exist, and pass through the original error(s). + return validationResult({ errors: nestedErrors }); + } else { + let alternativesList = sameLevelErrors + .map((error) => `"${error.message}"`) + .join(", "); + + throw new ValidationError(`Must satisfy at least one of: ${alternativesList}`, { errors: sameLevelErrors }); + } }); } }; diff --git a/package.json b/package.json index 10d252c..a0ddf28 100644 --- a/package.json +++ b/package.json @@ -18,6 +18,8 @@ }, "dependencies": { "@validatem/combinator": "^0.1.1", - "@validatem/error": "^1.0.0" + "@validatem/error": "^1.0.0", + "@validatem/validation-result": "^0.1.2", + "split-filter": "^1.1.3" } } diff --git a/yarn.lock b/yarn.lock index 065de15..28a7bee 100644 --- a/yarn.lock +++ b/yarn.lock @@ -132,7 +132,7 @@ resolved "https://registry.yarnpkg.com/@validatem/required/-/required-0.1.1.tgz#64f4a87333fc5955511634036b7f8948ed269170" integrity sha512-vI4NzLfay4RFAzp7xyU34PHb8sAo6w/3frrNh1EY9Xjnw2zxjY5oaxwmbFP1jVevBE6QQEnKogtzUHz/Zuvh6g== -"@validatem/validation-result@^0.1.1": +"@validatem/validation-result@^0.1.1", "@validatem/validation-result@^0.1.2": version "0.1.2" resolved "https://registry.yarnpkg.com/@validatem/validation-result/-/validation-result-0.1.2.tgz#4e75cfd87305fc78f8d05ac84921a2c99a0348e0" integrity sha512-okmP8JarIwIgfpaVcvZGuQ1yOsLKT3Egt49Ynz6h1MAeGsP/bGHXkkXtbiWOVsk5Tzku5vDVFSrFnF+5IEHKxw== @@ -195,3 +195,8 @@ is-string@^1.0.5: version "1.0.5" resolved "https://registry.yarnpkg.com/is-string/-/is-string-1.0.5.tgz#40493ed198ef3ff477b8c7f92f644ec82a5cd3a6" integrity sha512-buY6VNRjhQMiF1qWDouloZlQbRhDPCebwxSjxMjxgemYT46YMd2NR0/H+fBhEfWX4A/w9TBJ+ol+okqJKFE6vQ== + +split-filter@^1.1.3: + version "1.1.3" + resolved "https://registry.yarnpkg.com/split-filter/-/split-filter-1.1.3.tgz#c68cc598783d88f60d16e7b452dacfe95ba60539" + integrity sha512-2xXwhWeJUFrYE8CL+qoy9mCohu5/E+uglvpqL1FVXz1XbvTwivafVC6oTDeg/9ksOAxg6DvyCF44Dvf5crFU0w==