Repository: jquense/yup
Pull request #444 (2019-02-03 14:04:16)

fix: concat of mixed and subtype
Separate PR from #443.

```js
mixed().concat(number()).validateSync([]) // should throw
```

vonagam: By the way, fixes #399 (in issue's example a problem in concating mixed and date, not in when).
vonagam: Additional commit (can be separated into separate PR, but then i will have to make rebase after this PR will be merged). Small one, but fixes #381.

Smallest repo of a problem:

```js
yup.object({
  conditional: yup.object().when("$check", {
    is: true,
    then: yup.object({
      lazy: yup.lazy(val => yup.number()),
    })
  }),
}).validateSync({}, {context: {check: true}});
```

It fixes it, but underlying cause of the issue still there:

- `validate` in non-strict mode calls `cast`.

- In `object._cast` field get [`resolve`d](https://github.com/jquense/yup/blob/d02ff5e59e004b4c5189d1b9fc0055cff45c61df/src/object.js#L104) then [`cast`ed](https://github.com/jquense/yup/blob/d02ff5e59e004b4c5189d1b9fc0055cff45c61df/src/object.js#L113).

- `cast` [calls](https://github.com/jquense/yup/blob/d02ff5e59e004b4c5189d1b9fc0055cff45c61df/src/mixed.js#L152) `resolve` inside itself. So you end up with `schema.resolve().resolve()`.

- after `resolve` with conditions is executed resulted schema still contains original schema conditions, they are not stripped. So when you call `resolve` on resulted schema, those conditions will be called again and their result will be `concat`ed again.

- and if in a result of conditions was a lazy schema, it ended up `concat`ing with itself, but `Lazy` do not have `concat` method and error was thrown.

Current solution solves the last point: skip merging same schema's or values.

Breaking addition will be a removal of original `_conditions` from `resolve` result. It will also allow to easily implement nested `when`s.


Patch:
diff --git a/src/mixed.js b/src/mixed.js
index ac45454..540057e 100644
--- a/src/mixed.js
+++ b/src/mixed.js
@@ -5,7 +5,7 @@ import toArray from 'lodash/toArray';
 import { mixed as locale } from './locale';
 import Condition from './Condition';
 import runValidations from './util/runValidations';
-import merge from './util/merge';
+import prependDeep from './util/prependDeep';
 import isSchema from './util/isSchema';
 import isAbsent from './util/isAbsent';
 import createValidation from './util/createValidation';
@@ -103,7 +103,7 @@ const proto = (SchemaType.prototype = {
   },
 
   concat(schema) {
-    if (!schema) return this;
+    if (!schema || schema === this) return this;
 
     if (schema._type !== this._type && this._type !== 'mixed')
       throw new TypeError(
@@ -111,14 +111,14 @@ const proto = (SchemaType.prototype = {
           this._type
         } and ${schema._type}`,
       );
-    var cloned = this.clone();
-    var next = merge(this.clone(), schema.clone());
 
-    // undefined isn't merged over, but is a valid value for default
+    var next = prependDeep(schema.clone(), this);
+
+    // new undefined default is overriden by old non-undefined one, revert
     if (has(schema, '_default')) next._default = schema._default;
 
-    next.tests = cloned.tests;
-    next._exclusive = cloned._exclusive;
+    next.tests = this.tests;
+    next._exclusive = this._exclusive;
 
     // manually add the new tests to ensure
     // the deduping logic is consistent
@@ -126,8 +126,6 @@ const proto = (SchemaType.prototype = {
       next = next.test(fn.OPTIONS);
     });
 
-    next._type = schema._type;
-
     return next;
   },
 
diff --git a/src/util/merge.js b/src/util/merge.js
deleted file mode 100644
index 79ab8a2..0000000
--- a/src/util/merge.js
+++ /dev/null
@@ -1,30 +0,0 @@
-import has from 'lodash/has';
-import isSchema from './isSchema';
-
-let isObject = obj => Object.prototype.toString.call(obj) === '[object Object]';
-
-export default function merge(target, source) {
-  for (var key in source)
-    if (has(source, key)) {
-      var targetVal = target[key],
-        sourceVal = source[key];
-
-      if (sourceVal === undefined) continue;
-
-      if (isSchema(sourceVal)) {
-        target[key] = isSchema(targetVal)
-          ? targetVal.concat(sourceVal)
-          : sourceVal;
-      } else if (isObject(sourceVal)) {
-        target[key] = isObject(targetVal)
-          ? merge(targetVal, sourceVal)
-          : sourceVal;
-      } else if (Array.isArray(sourceVal)) {
-        target[key] = Array.isArray(targetVal)
-          ? targetVal.concat(sourceVal)
-          : sourceVal;
-      } else target[key] = source[key];
-    }
-
-  return target;
-}
diff --git a/src/util/prependDeep.js b/src/util/prependDeep.js
new file mode 100644
index 0000000..6d00e75
--- /dev/null
+++ b/src/util/prependDeep.js
@@ -0,0 +1,27 @@
+import has from 'lodash/has';
+import isSchema from './isSchema';
+
+let isObject = obj => Object.prototype.toString.call(obj) === '[object Object]';
+
+export default function prependDeep(target, source) {
+  for (var key in source)
+    if (has(source, key)) {
+      var sourceVal = source[key],
+        targetVal = target[key];
+
+      if (targetVal === undefined) {
+        target[key] = sourceVal;
+      } else if (targetVal === sourceVal) {
+        continue;
+      } else if (isSchema(targetVal)) {
+        if (isSchema(sourceVal)) target[key] = sourceVal.concat(targetVal);
+      } else if (isObject(targetVal)) {
+        if (isObject(sourceVal))
+          target[key] = prependDeep(targetVal, sourceVal);
+      } else if (Array.isArray(targetVal)) {
+        if (Array.isArray(sourceVal)) target[key] = sourceVal.concat(targetVal);
+      }
+    }
+
+  return target;
+}


Test patch:
diff --git a/test/mixed.js b/test/mixed.js
index 153f79e..a71efe8 100644
--- a/test/mixed.js
+++ b/test/mixed.js
@@ -1,4 +1,15 @@
-import { array, mixed, string, number, object, ref, reach, bool } from '../src';
+import {
+  array,
+  mixed,
+  string,
+  number,
+  object,
+  ref,
+  reach,
+  bool,
+  ValidationError,
+} from '../src';
+
 let noop = () => {};
 
 function ensureSync(fn) {
@@ -585,6 +596,14 @@ describe('Mixed Types ', () => {
     }.should.not.throw(TypeError));
   });
 
+  it('concat should validate with mixed and other type', async function() {
+    let inst = mixed().concat(number());
+
+    await inst
+      .validate([])
+      .should.be.rejected(ValidationError, /should be a `number`/);
+  });
+
   it('concat should maintain undefined defaults', function() {
     let inst = string().default('hi');
 
diff --git a/test/yup.js b/test/yup.js
index 51f54e5..9ec8d51 100644
--- a/test/yup.js
+++ b/test/yup.js
@@ -1,5 +1,5 @@
 import reach, { getIn } from '../src/util/reach';
-import merge from '../src/util/merge';
+import prependDeep from '../src/util/prependDeep';
 import { settled } from '../src/util/runValidations';
 
 import { object, array, string, lazy, number } from '../src';
@@ -36,11 +36,11 @@ describe('Yup', function() {
     ]);
   });
 
-  it('should merge', function() {
-    var a = { a: 1, b: 'hello', c: [1, 2, 3], d: { a: /hi/ }, e: { b: 5 } };
-    var b = { a: 4, c: [4, 5, 3], d: { b: 'hello' }, f: { c: 5 }, g: null };
+  it('should prepend deeply', function() {
+    var a = { a: 4, c: [4, 5, 3], d: { b: 'hello' }, f: { c: 5 }, g: null };
+    var b = { a: 1, b: 'hello', c: [1, 2, 3], d: { a: /hi/ }, e: { b: 5 } };
 
-    merge(a, b).should.deep.eql({
+    prependDeep(a, b).should.deep.eql({
       a: 4,
       b: 'hello',
       c: [1, 2, 3, 4, 5, 3],
@@ -54,6 +54,17 @@ describe('Yup', function() {
     });
   });
 
+  it('should not prepend needlesly', function() {
+    var schema = string();
+    var spy = sinon.spy(schema, 'concat');
+    var a = { schema };
+    var b = { schema };
+    var c = prependDeep(a, b);
+
+    c.schema.should.equal(schema);
+    spy.should.not.have.been.called();
+  });
+
   it('should getIn correctly', async () => {
     var num = number(),
       inst = object().shape({