Refactor mergeConfig without utils.deepMerge (#2844)

* Adding failing test

* Fixing #2587 default custom config persisting

* Adding Concat keys and filter duplicates

* Fixed value from CPE

* update for review feedbacks

* no deepMerge

* only merge between plain objects

* fix rename

* always merge config by mergeConfig

* extract function mergeDeepProperties

* refactor mergeConfig with all keys, and add special logic for validateStatus

* add test for resetting headers

* add lots of tests and fix a bug

* should not inherit `data`

* use simple toString

* revert #1845

Co-authored-by: David Tanner <david.tanner@lifeomic.com>
Co-authored-by: Justin Beckwith <justin.beckwith@gmail.com>

test/specs/core/mergeConfig.spec.js

@@ -80,4 +80,231 @@ describe('core::mergeConfig', function() {
     var merged = mergeConfig(defaults, { foo: 'bar' });
     expect(merged.foo).toEqual('bar');
   });
+
+  it('should allow setting custom default options', function() {
+    var merged = mergeConfig({ foo: 'bar' }, {});
+    expect(merged.foo).toEqual('bar');
+  });
+
+  it('should allow merging custom objects in the config', function() {
+    var merged = mergeConfig({
+      nestedConfig: {
+        propertyOnDefaultConfig: true
+      }
+    }, {
+      nestedConfig: {
+        propertyOnRequestConfig: true
+      }
+    });
+    expect(merged.nestedConfig.propertyOnDefaultConfig).toEqual(true);
+    expect(merged.nestedConfig.propertyOnRequestConfig).toEqual(true);
+  });
+
+  describe('valueFromConfig2Keys', function() {
+    var config1 = {url: '/foo', method: 'post', data: {a: 3}};
+
+    it('should skip if config2 is undefined', function() {
+      expect(mergeConfig(config1, {})).toEqual({});
+    });
+
+    it('should clone config2 if is plain object', function() {
+      var data = {a: 1, b: 2};
+      var merged = mergeConfig(config1, {data: data});
+      expect(merged.data).toEqual(data);
+      expect(merged.data).not.toBe(data);
+    });
+
+    it('should clone config2 if is array', function() {
+      var data = [1, 2, 3];
+      var merged = mergeConfig(config1, {data: data});
+      expect(merged.data).toEqual(data);
+      expect(merged.data).not.toBe(data);
+    });
+
+    it('should set as config2 in other cases', function() {
+      var obj = Object.create({});
+      expect(mergeConfig(config1, {data: 1}).data).toBe(1);
+      expect(mergeConfig(config1, {data: 'str'}).data).toBe('str');
+      expect(mergeConfig(config1, {data: obj}).data).toBe(obj);
+      expect(mergeConfig(config1, {data: null}).data).toBe(null);
+    });
+  });
+
+  describe('mergeDeepPropertiesKeys', function() {
+    it('should skip if both config1 and config2 are undefined', function() {
+      expect(mergeConfig({headers: undefined}, {headers: undefined})).toEqual({});
+    });
+
+    it('should merge if both config1 and config2 are plain object', function() {
+      expect(mergeConfig({headers: {a: 1, b: 1}}, {headers: {b: 2, c: 2}}))
+        .toEqual({headers: {a: 1, b: 2, c: 2}});
+    });
+
+    it('should clone config2 if is plain object', function() {
+      var config1 = {headers: [1, 2, 3]};
+      var config2 = {headers: {a: 1, b: 2}};
+      var merged = mergeConfig(config1, config2);
+      expect(merged.headers).toEqual(config2.headers);
+      expect(merged.headers).not.toBe(config2.headers);
+    });
+
+    it('should clone config2 if is array', function() {
+      var config1 = {headers: {a: 1, b: 1}};
+      var config2 = {headers: [1, 2, 3]};
+      var merged = mergeConfig(config1, config2);
+      expect(merged.headers).toEqual(config2.headers);
+      expect(merged.headers).not.toBe(config2.headers);
+    });
+
+    it('should set as config2 in other cases', function() {
+      var config1 = {headers: {a: 1, b: 1}};
+      var obj = Object.create({});
+      expect(mergeConfig(config1, {headers: 1}).headers).toBe(1);
+      expect(mergeConfig(config1, {headers: 'str'}).headers).toBe('str');
+      expect(mergeConfig(config1, {headers: obj}).headers).toBe(obj);
+      expect(mergeConfig(config1, {headers: null}).headers).toBe(null);
+    });
+
+    it('should clone config1 if is plain object', function() {
+      var config1 = {headers: {a: 1, b: 2}};
+      var config2 = {};
+      var merged = mergeConfig(config1, config2);
+      expect(merged.headers).toEqual(config1.headers);
+      expect(merged.headers).not.toBe(config1.headers);
+    });
+
+    it('should clone config1 if is array', function() {
+      var config1 = {headers: [1, 2, 3]};
+      var config2 = {};
+      var merged = mergeConfig(config1, config2);
+      expect(merged.headers).toEqual(config1.headers);
+      expect(merged.headers).not.toBe(config1.headers);
+    });
+
+    it('should set as config1 in other cases', function() {
+      var config2 = {};
+      var obj = Object.create({});
+      expect(mergeConfig({headers: 1}, config2).headers).toBe(1);
+      expect(mergeConfig({headers: 'str'}, config2).headers).toBe('str');
+      expect(mergeConfig({headers: obj}, config2).headers).toBe(obj);
+      expect(mergeConfig({headers: null}, config2).headers).toBe(null);
+    });
+  });
+
+  describe('defaultToConfig2Keys', function() {
+    it('should skip if both config1 and config2 are undefined', function() {
+      expect(mergeConfig({transformRequest: undefined}, {transformRequest: undefined})).toEqual({});
+    });
+
+    it('should clone config2 if both config1 and config2 are plain object', function() {
+      var config1 = {transformRequest: {a: 1, b: 1}};
+      var config2 = {transformRequest: {b: 2, c: 2}};
+      var merged = mergeConfig(config1, config2);
+      expect(merged.transformRequest).toEqual(config2.transformRequest);
+      expect(merged.transformRequest).not.toBe(config2.transformRequest);
+    });
+
+    it('should clone config2 if is array', function() {
+      var config1 = {transformRequest: {a: 1, b: 1}};
+      var config2 = {transformRequest: [1, 2, 3]};
+      var merged = mergeConfig(config1, config2);
+      expect(merged.transformRequest).toEqual(config2.transformRequest);
+      expect(merged.transformRequest).not.toBe(config2.transformRequest);
+    });
+
+    it('should set as config2 in other cases', function() {
+      var config1 = {transformRequest: {a: 1, b: 1}};
+      var obj = Object.create({});
+      expect(mergeConfig(config1, {transformRequest: 1}).transformRequest).toBe(1);
+      expect(mergeConfig(config1, {transformRequest: 'str'}).transformRequest).toBe('str');
+      expect(mergeConfig(config1, {transformRequest: obj}).transformRequest).toBe(obj);
+      expect(mergeConfig(config1, {transformRequest: null}).transformRequest).toBe(null);
+    });
+
+    it('should clone config1 if is plain object', function() {
+      var config1 = {transformRequest: {a: 1, b: 2}};
+      var config2 = {};
+      var merged = mergeConfig(config1, config2);
+      expect(merged.transformRequest).toEqual(config1.transformRequest);
+      expect(merged.transformRequest).not.toBe(config1.transformRequest);
+    });
+
+    it('should clone config1 if is array', function() {
+      var config1 = {transformRequest: [1, 2, 3]};
+      var config2 = {};
+      var merged = mergeConfig(config1, config2);
+      expect(merged.transformRequest).toEqual(config1.transformRequest);
+      expect(merged.transformRequest).not.toBe(config1.transformRequest);
+    });
+
+    it('should set as config1 in other cases', function() {
+      var config2 = {};
+      var obj = Object.create({});
+      expect(mergeConfig({transformRequest: 1}, config2).transformRequest).toBe(1);
+      expect(mergeConfig({transformRequest: 'str'}, config2).transformRequest).toBe('str');
+      expect(mergeConfig({transformRequest: obj}, config2).transformRequest).toBe(obj);
+      expect(mergeConfig({transformRequest: null}, config2).transformRequest).toBe(null);
+    });
+  });
+
+  describe('directMergeKeys', function() {
+    it('should merge if config2 in keys', function() {
+      expect(mergeConfig({}, {validateStatus: undefined})).toEqual({validateStatus: undefined});
+    });
+
+    it('should merge if both config1 and config2 are plain object', function() {
+      expect(mergeConfig({validateStatus: {a: 1, b: 1}}, {validateStatus: {b: 2, c: 2}}))
+        .toEqual({validateStatus: {a: 1, b: 2, c: 2}});
+    });
+
+    it('should clone config2 if is plain object', function() {
+      var config1 = {validateStatus: [1, 2, 3]};
+      var config2 = {validateStatus: {a: 1, b: 2}};
+      var merged = mergeConfig(config1, config2);
+      expect(merged.validateStatus).toEqual(config2.validateStatus);
+      expect(merged.validateStatus).not.toBe(config2.validateStatus);
+    });
+
+    it('should clone config2 if is array', function() {
+      var config1 = {validateStatus: {a: 1, b: 2}};
+      var config2 = {validateStatus: [1, 2, 3]};
+      var merged = mergeConfig(config1, config2);
+      expect(merged.validateStatus).toEqual(config2.validateStatus);
+      expect(merged.validateStatus).not.toBe(config2.validateStatus);
+    });
+
+    it('should set as config2 in other cases', function() {
+      var config1 = {validateStatus: {a: 1, b: 2}};
+      var obj = Object.create({});
+      expect(mergeConfig(config1, {validateStatus: 1}).validateStatus).toBe(1);
+      expect(mergeConfig(config1, {validateStatus: 'str'}).validateStatus).toBe('str');
+      expect(mergeConfig(config1, {validateStatus: obj}).validateStatus).toBe(obj);
+      expect(mergeConfig(config1, {validateStatus: null}).validateStatus).toBe(null);
+    });
+
+    it('should clone config1 if is plain object', function() {
+      var config1 = {validateStatus: {a: 1, b: 2}};
+      var config2 = {};
+      var merged = mergeConfig(config1, config2);
+      expect(merged.validateStatus).toEqual(config1.validateStatus);
+      expect(merged.validateStatus).not.toBe(config1.validateStatus);
+    });
+
+    it('should clone config1 if is array', function() {
+      var config1 = {validateStatus: [1, 2, 3]};
+      var config2 = {};
+      var merged = mergeConfig(config1, config2);
+      expect(merged.validateStatus).toEqual(config1.validateStatus);
+      expect(merged.validateStatus).not.toBe(config1.validateStatus);
+    });
+
+    it('should set as config1 in other cases', function() {
+      var config2 = {};
+      var obj = Object.create({});
+      expect(mergeConfig({validateStatus: 1}, config2).validateStatus).toBe(1);
+      expect(mergeConfig({validateStatus: 'str'}, config2).validateStatus).toBe('str');
+      expect(mergeConfig({validateStatus: obj}, config2).validateStatus).toBe(obj);
+      expect(mergeConfig({validateStatus: null}, config2).validateStatus).toBe(null);
+    });
+  });
 });

test/specs/headers.spec.js

@@ -42,21 +42,6 @@ describe('headers', function () {
     });
   });
 
-  it('should not set header if value is null', function (done) {
-    expect(axios.defaults.headers.common['Accept']).toEqual('application/json, text/plain, */*');
-
-    axios('/foo', {
-      headers: {
-        Accept: null
-      }
-    });
-
-    getAjaxRequest().then(function (request) {
-      expect(typeof request.requestHeaders['Accept']).toEqual('undefined');
-      done();
-    });
-  });
-
   it('should add extra headers for post', function (done) {
     var headers = axios.defaults.headers.common;
 
@@ -72,6 +57,32 @@ describe('headers', function () {
     });
   });
 
+  it('should reset headers by null or explicit undefined', function (done) {
+    axios.create({
+      headers: {
+        common: {
+          'x-header-a': 'a',
+          'x-header-b': 'b',
+          'x-header-c': 'c'
+        }
+      }
+    }).post('/foo', {fizz: 'buzz'}, {
+      headers: {
+        'Content-Type': null,
+        'x-header-a': null,
+        'x-header-b': undefined
+      }
+    });
+
+    getAjaxRequest().then(function (request) {
+      testHeaderValue(request.requestHeaders, 'Content-Type', null);
+      testHeaderValue(request.requestHeaders, 'x-header-a', null);
+      testHeaderValue(request.requestHeaders, 'x-header-b', undefined);
+      testHeaderValue(request.requestHeaders, 'x-header-c', 'c');
+      done();
+    });
+  });
+
   it('should use application/json when posting an object', function (done) {
     axios.post('/foo/bar', {
       firstName: 'foo',

test/specs/requests.spec.js

@@ -177,6 +177,48 @@ describe('requests', function () {
     });
   });
 
+  it('should resolve when validateStatus is null', function (done) {
+    var resolveSpy = jasmine.createSpy('resolve');
+    var rejectSpy = jasmine.createSpy('reject');
+
+    axios('/foo', {
+      validateStatus: null
+    }).then(resolveSpy)
+      .catch(rejectSpy)
+      .then(function () {
+        expect(resolveSpy).toHaveBeenCalled();
+        expect(rejectSpy).not.toHaveBeenCalled();
+        done();
+      });
+
+    getAjaxRequest().then(function (request) {
+      request.respondWith({
+        status: 500
+      });
+    });
+  });
+
+  it('should resolve when validateStatus is undefined', function (done) {
+    var resolveSpy = jasmine.createSpy('resolve');
+    var rejectSpy = jasmine.createSpy('reject');
+
+    axios('/foo', {
+      validateStatus: undefined
+    }).then(resolveSpy)
+      .catch(rejectSpy)
+      .then(function () {
+        expect(resolveSpy).toHaveBeenCalled();
+        expect(rejectSpy).not.toHaveBeenCalled();
+        done();
+      });
+
+    getAjaxRequest().then(function (request) {
+      request.respondWith({
+        status: 500
+      });
+    });
+  });
+
   // https://github.com/axios/axios/issues/378
   it('should return JSON when rejecting', function (done) {
     var response;

test/specs/utils/deepMerge.spec.js

@@ -1,66 +0,0 @@
-var deepMerge = require('../../../lib/utils').deepMerge;
-
-describe('utils::deepMerge', function () {
-  it('should be immutable', function () {
-    var a = {};
-    var b = {foo: 123};
-    var c = {bar: 456};
-
-    deepMerge(a, b, c);
-
-    expect(typeof a.foo).toEqual('undefined');
-    expect(typeof a.bar).toEqual('undefined');
-    expect(typeof b.bar).toEqual('undefined');
-    expect(typeof c.foo).toEqual('undefined');
-  });
-
-  it('should deepMerge properties', function () {
-    var a = {foo: 123};
-    var b = {bar: 456};
-    var c = {foo: 789};
-    var d = deepMerge(a, b, c);
-
-    expect(d.foo).toEqual(789);
-    expect(d.bar).toEqual(456);
-  });
-
-  it('should deepMerge recursively', function () {
-    var a = {foo: {bar: 123}};
-    var b = {foo: {baz: 456}, bar: {qux: null}};
-
-    expect(deepMerge(a, b)).toEqual({
-      foo: {
-        bar: 123,
-        baz: 456
-      },
-      bar: {
-        qux: null
-      }
-    });
-  });
-
-  it('should remove all references from nested objects', function () {
-    var a = {foo: {bar: 123}};
-    var b = {};
-    var d = deepMerge(a, b);
-
-    expect(d).toEqual({
-      foo: {
-        bar: 123
-      }
-    });
-
-    expect(d.foo).not.toBe(a.foo);
-  });
-
-  it('handles null and undefined arguments', function () {
-    expect(deepMerge(undefined, undefined)).toEqual(undefined);
-    expect(deepMerge(undefined, {foo: 123})).toEqual({foo: 123});
-    expect(deepMerge({foo: 123}, undefined)).toEqual(undefined);
-
-    expect(deepMerge(null, null)).toEqual(null);
-    expect(deepMerge(null, {foo: 123})).toEqual({foo: 123});
-    expect(deepMerge({foo: 123}, null)).toEqual(null);
-  });
-});
-

test/specs/utils/isX.spec.js

@@ -47,9 +47,17 @@ describe('utils::isX', function () {
 
   it('should validate Object', function () {
     expect(utils.isObject({})).toEqual(true);
+    expect(utils.isObject([])).toEqual(true);
     expect(utils.isObject(null)).toEqual(false);
   });
 
+  it('should validate plain Object', function () {
+    expect(utils.isPlainObject({})).toEqual(true);
+    expect(utils.isPlainObject([])).toEqual(false);
+    expect(utils.isPlainObject(null)).toEqual(false);
+    expect(utils.isPlainObject(Object.create({}))).toEqual(false);
+  });
+
   it('should validate Date', function () {
     expect(utils.isDate(new Date())).toEqual(true);
     expect(utils.isDate(Date.now())).toEqual(false);

test/specs/utils/merge.spec.js

@@ -38,5 +38,47 @@ describe('utils::merge', function () {
       }
     });
   });
-});
 
+  it('should remove all references from nested objects', function () {
+    var a = {foo: {bar: 123}};
+    var b = {};
+    var d = merge(a, b);
+
+    expect(d).toEqual({
+      foo: {
+        bar: 123
+      }
+    });
+
+    expect(d.foo).not.toBe(a.foo);
+  });
+
+  it('handles null and undefined arguments', function () {
+    expect(merge(undefined, undefined)).toEqual({});
+    expect(merge(undefined, {foo: 123})).toEqual({foo: 123});
+    expect(merge({foo: 123}, undefined)).toEqual({foo: 123});
+
+    expect(merge(null, null)).toEqual({});
+    expect(merge(null, {foo: 123})).toEqual({foo: 123});
+    expect(merge({foo: 123}, null)).toEqual({foo: 123});
+  });
+
+  it('should replace properties with null', function () {
+    expect(merge({}, {a: null})).toEqual({a: null});
+    expect(merge({a: null}, {})).toEqual({a: null});
+  });
+
+  it('should replace properties with arrays', function () {
+    expect(merge({}, {a: [1, 2, 3]})).toEqual({a: [1, 2, 3]});
+    expect(merge({a: 2}, {a: [1, 2, 3]})).toEqual({a: [1, 2, 3]});
+    expect(merge({a: {b: 2}}, {a: [1, 2, 3]})).toEqual({a: [1, 2, 3]});
+  });
+
+  it('should replace properties with cloned arrays', function () {
+    var a = [1, 2, 3];
+    var d = merge({}, {a: a});
+
+    expect(d).toEqual({a: [1, 2, 3]});
+    expect(d.a).not.toBe(a);
+  });
+});