Merge pull request #1117 from jquense/fix-required

fix array required() and string coercion

README.md

@@ -9,7 +9,6 @@ Yup's API is heavily inspired by [Joi](https://github.com/hapijs/joi), but leane
 <!-- START doctoc generated TOC please keep comment here to allow auto update -->
 <!-- DON'T EDIT THIS SECTION, INSTEAD RE-RUN doctoc TO UPDATE -->
 
-
 - [Install](#install)
 - [Usage](#usage)
   - [Using a custom locale dictionary](#using-a-custom-locale-dictionary)
@@ -580,15 +579,19 @@ Indicates that `null` is a valid value for the schema. Without `nullable()`
 
 #### `mixed.required(message?: string | function): Schema`
 
-Mark the schema as required. All field values apart from `undefined` and `null` meet this requirement.
+Mark the schema as required, which will not allow `undefined` or `null` as a value.
+Note that unless a schema is marked as `nullable()` a `null` value is treated as a type error, not a missing value. Mark a schema as `mixed().nullable().required()` treat `null` as missing.
+
+> Watch out! [`string().required`](#stringrequiredmessage-string--function-schema)) works a little
+> different and additionally prevents empty string values (`''`) when required.
 
-#### `mixed.notRequired(): Schema`
+#### `mixed.notRequired(): Schema` Alias: `optional()`
 
-Mark the schema as not required. Passing `undefined` as value will not fail validation.
+Mark the schema as not required. Passing `undefined` (or `null` for nullable schema) as value will not fail validation.
 
 #### `mixed.defined(): Schema`
 
-Mark the schema as required but nullable. All field values apart from `undefined` meet this requirement.
+Require a value for the schema. All field values apart from `undefined` meet this requirement.
 
 #### `mixed.typeError(message: string): Schema`
 
@@ -723,7 +726,7 @@ await schema.isValid('john'); // => false
 Test functions are called with a special context, or `this` value, that exposes some useful metadata
 and functions. Older versions just expose the `this` context using `function ()`, not arrow-func,
 but now it's exposed too as a second argument of the test functions. It's allow you decide which
-approach you prefer. 
+approach you prefer.
 
 - `this.path`: the string path of the current validation
 - `this.schema`: the resolved schema object that the test is running against.
@@ -824,7 +827,7 @@ Failed casts return the input value.
 
 #### `string.required(message?: string | function): Schema`
 
-The same as the `mixed()` schema required, except that empty strings are also considered 'missing' values.
+The same as the `mixed()` schema required, **except** that empty strings are also considered 'missing' values.
 
 #### `string.length(limit: number | Ref, message?: string | function): Schema`
 
@@ -1016,9 +1019,9 @@ Failed casts return: `null`;
 Specify the schema of array elements. `of()` is optional and when omitted the array schema will
 not validate its contents.
 
-#### `array.required(message?: string | function): Schema`
+#### `array.length(length: number | Ref, message?: string | function): Schema`
 
-The same as the `mixed()` schema required, except that empty arrays are also considered 'missing' values.
+Set a specific length requirement for the array. The `${length}` interpolation can be used in the `message` argument.
 
 #### `array.min(limit: number | Ref, message?: string | function): Schema`
 
@@ -1288,8 +1291,7 @@ const personSchema = yup.object({
     .string()
     // Here we use `defined` instead of `required` to more closely align with
     // TypeScript. Both will have the same effect on the resulting type by
-    // excluding `undefined`, but `required` will also disallow other values
-    // such as empty strings.
+    // excluding `undefined`, but `required` will also disallow empty strings.
     .defined(),
   nickName: yup
     .string()

src/array.js

@@ -127,12 +127,6 @@ inherits(ArraySchema, MixedSchema, {
     );
   },
 
-  _isPresent(value) {
-    return (
-      MixedSchema.prototype._isPresent.call(this, value) && value.length > 0
-    );
-  },
-
   of(schema) {
     var next = this.clone();
 
@@ -176,6 +170,19 @@ inherits(ArraySchema, MixedSchema, {
     });
   },
 
+  length(length, message) {
+    message = message || locale.length;
+    return this.test({
+      message,
+      name: 'length',
+      exclusive: true,
+      params: { length },
+      test(value) {
+        return isAbsent(value) || value.length === this.resolve(length);
+      },
+    });
+  },
+
   ensure() {
     return this.default(() => []).transform((val, original) => {
       // We don't want to return `null` for nullable schema

src/string.js

@@ -8,10 +8,12 @@ let rEmail = /^((([a-z]|\d|[!#\$%&'\*\+\-\/=\?\^_`{\|}~]|[\u00A0-\uD7FF\uF900-\u
 // eslint-disable-next-line
 let rUrl = /^((https?|ftp):)?\/\/(((([a-z]|\d|-|\.|_|~|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])|(%[\da-f]{2})|[!\$&'\(\)\*\+,;=]|:)*@)?(((\d|[1-9]\d|1\d\d|2[0-4]\d|25[0-5])\.(\d|[1-9]\d|1\d\d|2[0-4]\d|25[0-5])\.(\d|[1-9]\d|1\d\d|2[0-4]\d|25[0-5])\.(\d|[1-9]\d|1\d\d|2[0-4]\d|25[0-5]))|((([a-z]|\d|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])|(([a-z]|\d|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])([a-z]|\d|-|\.|_|~|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])*([a-z]|\d|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])))\.)+(([a-z]|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])|(([a-z]|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])([a-z]|\d|-|\.|_|~|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])*([a-z]|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])))\.?)(:\d*)?)(\/((([a-z]|\d|-|\.|_|~|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])|(%[\da-f]{2})|[!\$&'\(\)\*\+,;=]|:|@)+(\/(([a-z]|\d|-|\.|_|~|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])|(%[\da-f]{2})|[!\$&'\(\)\*\+,;=]|:|@)*)*)?)?(\?((([a-z]|\d|-|\.|_|~|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])|(%[\da-f]{2})|[!\$&'\(\)\*\+,;=]|:|@)|[\uE000-\uF8FF]|\/|\?)*)?(\#((([a-z]|\d|-|\.|_|~|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])|(%[\da-f]{2})|[!\$&'\(\)\*\+,;=]|:|@)|\/|\?)*)?$/i;
 // eslint-disable-next-line
-let rUUID = /^(?:[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}|00000000-0000-0000-0000-000000000000)$/i
+let rUUID = /^(?:[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}|00000000-0000-0000-0000-000000000000)$/i;
 
 let isTrimmed = (value) => isAbsent(value) || value === value.trim();
 
+let objStringTag = {}.toString();
+
 export default function StringSchema() {
   if (!(this instanceof StringSchema)) return new StringSchema();
 
@@ -20,7 +22,14 @@ export default function StringSchema() {
   this.withMutation(() => {
     this.transform(function (value) {
       if (this.isType(value)) return value;
-      return value != null && value.toString ? value.toString() : value;
+      if (Array.isArray(value)) return value;
+
+      const strValue =
+        value != null && value.toString ? value.toString() : value;
+
+      if (strValue === objStringTag) return value;
+
+      return strValue;
     });
   });
 }
@@ -33,9 +42,7 @@ inherits(StringSchema, MixedSchema, {
   },
 
   _isPresent(value) {
-    return (
-      MixedSchema.prototype._isPresent.call(this, value) && value.length > 0
-    );
+    return MixedSchema.prototype._isPresent.call(this, value) && !!value.length;
   },
 
   length(length, message = locale.length) {

test/array.js

@@ -67,14 +67,39 @@ describe('Array types', () => {
   });
 
   describe('validation', () => {
+    test.each([
+      ['missing', undefined, array().defined()],
+      ['required', undefined, array().required()],
+      ['required', null, array().required()],
+      ['null', null, array()],
+      ['length', [1, 2, 3], array().length(2)],
+    ])('Basic validations fail: %s %p', async (type, value, schema) => {
+      expect(await schema.isValid(value)).to.equal(false);
+    });
+
+    test.each([
+      ['missing', [], array().defined()],
+      ['required', [], array().required()],
+      ['nullable', null, array().nullable()],
+      ['length', [1, 2, 3], array().length(3)],
+    ])('Basic validations pass: %s %p', async (type, value, schema) => {
+      expect(await schema.isValid(value)).to.equal(true);
+    });
+
     it('should allow undefined', async () => {
       await array().of(number().max(5)).isValid().should.become(true);
     });
 
-    it('should not allow null when not nullable', async () => {
-      await array().isValid(null).should.become(false);
+    it('max should replace earlier tests', async () => {
+      expect(await array().max(4).max(10).isValid(Array(5).fill(0))).to.equal(
+        true,
+      );
+    });
 
-      await array().nullable().isValid(null).should.become(true);
+    it('min should replace earlier tests', async () => {
+      expect(await array().min(10).min(4).isValid(Array(5).fill(0))).to.equal(
+        true,
+      );
     });
 
     it('should respect subtype validations', async () => {

test/string.js

@@ -10,14 +10,20 @@ describe('String types', () => {
       valid: [
         [5, '5'],
         ['3', '3'],
-        //[new String('foo'), 'foo'],
+        // [new String('foo'), 'foo'],
         ['', ''],
         [true, 'true'],
         [false, 'false'],
         [0, '0'],
         [null, null, schema.nullable()],
+        [
+          {
+            toString: () => 'hey',
+          },
+          'hey',
+        ],
       ],
-      invalid: [null],
+      invalid: [null, {}, []],
     });
 
     describe('ensure', () => {