Revert "Replace @ndhoule/defaults with ES6 spread syntax merging (#185)"

This reverts commit 7b59f34.

Author: juliofarah

lib/analytics.ts

@@ -5,13 +5,12 @@ import {
   InitOptions,
   SegmentAnalytics,
   SegmentOpts,
-  SegmentIntegration,
-  PageDefaults
+  SegmentIntegration
 } from './types';
 var url = require('component-url');
 
-import cloneDeep from 'lodash.clonedeep'
-import pick from 'lodash.pick'
+import cloneDeep from 'lodash.clonedeep';
+import pick from 'lodash.pick';
 
 var _analytics = global.analytics;
 
@@ -45,6 +44,7 @@ var normalize = require('./normalize');
 var on = require('component-event').bind;
 var pageDefaults = require('./pageDefaults');
 var prevent = require('@segment/prevent-default');
+var defaults = require('@ndhoule/defaults');
 var store = require('./store');
 var user = require('./user');
 var type = require('component-type');
@@ -322,13 +322,8 @@ Analytics.prototype.identify = function(
   });
 
   // Add the initialize integrations so the server-side ones can be disabled too
-  // NOTE: We need to merge integrations, not override them with assign
-  // since it is possible to change the initialized integrations at runtime.
   if (this.options.integrations) {
-    msg.integrations = {
-      ...this.options.integrations,
-      ...msg.integrations
-    };
+    defaults(msg.integrations, this.options.integrations);
   }
 
   this._invoke('identify', new Identify(msg));
@@ -383,13 +378,8 @@ Analytics.prototype.group = function(
   });
 
   // Add the initialize integrations so the server-side ones can be disabled too
-  // NOTE: We need to merge integrations, not override them with assign
-  // since it is possible to change the initialized integrations at runtime.
   if (this.options.integrations) {
-    msg.integrations = {
-      ...this.options.integrations,
-      ...msg.integrations
-    };
+    defaults(msg.integrations, this.options.integrations);
   }
 
   this._invoke('group', new Group(msg));
@@ -453,12 +443,10 @@ Analytics.prototype.track = function(
   }
 
   // Add the initialize integrations so the server-side ones can be disabled too
-  // NOTE: We need to merge integrations, not override them with assign
-  // since it is possible to change the initialized integrations at runtime.
-  msg.integrations = {
-    ...this._mergeInitializeAndPlanIntegrations(planIntegrationOptions),
-    ...msg.integrations
-  };
+  defaults(
+    msg.integrations,
+    this._mergeInitializeAndPlanIntegrations(planIntegrationOptions)
+  );
 
   this._invoke('track', new Track(msg));
 
@@ -611,15 +599,8 @@ Analytics.prototype.page = function(
 
   // Ensure properties has baseline spec properties.
   // TODO: Eventually move these entirely to `options.context.page`
-  // FIXME: This is purposely not overriding `defs`. There was a bug in the logic implemented by `@ndhoule/defaults`.
-  //        This bug made it so we only would overwrite values in `defs` that were set to `undefined`.
-  //        In some cases, though, pageDefaults  will return defaults with values set to "" (such as `window.location.search` defaulting to "").
-  //        The decision to not fix this bus was made to preserve backwards compatibility.
   const defs = pageDefaults();
-  properties = {
-    ...properties,
-    ...defs
-  };
+  defaults(properties, defs);
 
   // Mirror user overrides to `options.context.page` (but exclude custom properties)
   // (Any page defaults get applied in `this.normalize` for consistency.)
@@ -639,13 +620,8 @@ Analytics.prototype.page = function(
   });
 
   // Add the initialize integrations so the server-side ones can be disabled too
-  // NOTE: We need to merge integrations, not override them with assign
-  // since it is possible to change the initialized integrations at runtime.
   if (this.options.integrations) {
-    msg.integrations = {
-      ...this.options.integrations,
-      ...msg.integrations
-    };
+    defaults(msg.integrations, this.options.integrations);
   }
 
   this._invoke('page', new Page(msg));
@@ -697,13 +673,8 @@ Analytics.prototype.alias = function(
   });
 
   // Add the initialize integrations so the server-side ones can be disabled too
-  // NOTE: We need to merge integrations, not override them with assign
-  // since it is possible to change the initialized integrations at runtime.
   if (this.options.integrations) {
-    msg.integrations = {
-      ...this.options.integrations,
-      ...msg.integrations
-    };
+    defaults(msg.integrations, this.options.integrations);
   }
 
   this._invoke('alias', new Alias(msg));
@@ -1002,19 +973,15 @@ Analytics.prototype._parseQuery = function(query: string): SegmentAnalytics {
  */
 
 Analytics.prototype.normalize = function(msg: {
-  options: { [key: string]: unknown };
-  context: { page: Partial<PageDefaults> };
+  context: { page };
   anonymousId: string;
 }): object {
   msg = normalize(msg, Object.keys(this._integrations));
   if (msg.anonymousId) user.anonymousId(msg.anonymousId);
   msg.anonymousId = user.anonymousId();
 
   // Ensure all outgoing requests include page data in their contexts.
-  msg.context.page = {
-    ...pageDefaults(),
-    ...msg.context.page
-  };
+  msg.context.page = defaults(msg.context.page || {}, pageDefaults());
 
   return msg;
 };

lib/cookie.ts

@@ -10,10 +10,9 @@ import cloneDeep from 'lodash.clonedeep'
 var bindAll = require('bind-all');
 var cookie = require('@segment/cookie');
 var debug = require('debug')('analytics.js:cookie');
+var defaults = require('@ndhoule/defaults');
 var topDomain = require('@segment/top-domain');
 
-const MAX_AGE_ONE_YEAR = 31536000000
-
 /**
  * Initialize a new `Cookie` with `options`.
  *
@@ -33,20 +32,16 @@ Cookie.prototype.options = function(options?: CookieOptions) {
 
   options = options || {};
 
-  let domain = '.' + topDomain(window.location.href);
+  var domain = '.' + topDomain(window.location.href);
   if (domain === '.') domain = null;
 
-  const defaults: CookieOptions  = {
-    maxage: MAX_AGE_ONE_YEAR,
-    domain: domain,
+  this._options = defaults(options, {
+    // default to a year
+    maxage: 31536000000,
     path: '/',
+    domain: domain,
     sameSite: 'Lax'
-  }
-
-  this._options = {
-    ...defaults,
-    ...options
-  };
+  });
 
   // http://curl.haxx.se/rfc/cookie_spec.html
   // https://publicsuffix.org/list/effective_tld_names.dat

lib/entity.ts

@@ -10,6 +10,7 @@ import assignIn from 'lodash.assignin'
 
 var cookie = require('./cookie');
 var debug = require('debug')('analytics:entity');
+var defaults = require('@ndhoule/defaults');
 var memory = require('./memory');
 var store = require('./store');
 var isodateTraverse = require('@segment/isodate-traverse');
@@ -73,10 +74,7 @@ Entity.prototype.storage = function() {
 
 Entity.prototype.options = function(options?: InitOptions) {
   if (arguments.length === 0) return this._options;
-  this._options = {
-    ...this.defaults,
-    ...options
-  }
+  this._options = defaults(options || {}, this.defaults || {});
 };
 
 /**

lib/normalize.ts

@@ -6,11 +6,11 @@ import includes from 'lodash.includes'
  */
 
 var debug = require('debug')('analytics.js:normalize');
+var defaults = require('@ndhoule/defaults');
 var type = require('component-type');
 var uuid = require('uuid/v4');
 var md5 = require('spark-md5').hash;
 
-
 /**
  * HOP.
  */
@@ -88,11 +88,7 @@ function normalize(msg: Message, list: Array<any>): NormalizedMessage {
   delete msg.options;
   ret.integrations = integrations;
   ret.context = context;
-  ret = {
-    ...msg,
-    ...ret
-  }
-
+  ret = defaults(ret, msg);
   debug('->', ret);
   return ret;
 

lib/store.ts

@@ -1,12 +1,11 @@
 'use strict';
 
-import { StoreOptions } from './types';
-
 /*
  * Module dependencies.
  */
 
 var bindAll = require('bind-all');
+var defaults = require('@ndhoule/defaults');
 var store = require('@segment/store');
 
 /**
@@ -23,14 +22,11 @@ function Store(options?: { enabled: boolean }) {
  * Set the `options` for the store.
  */
 
-Store.prototype.options = function(options?: StoreOptions) {
+Store.prototype.options = function(options?: { enabled?: boolean }) {
   if (arguments.length === 0) return this._options;
 
   options = options || {};
-  options = {
-    enabled: true,
-    ...options
-  };
+  defaults(options, { enabled: true });
 
   this.enabled = options.enabled && store.enabled;
   this._options = options;

lib/types.ts

@@ -37,7 +37,6 @@ export interface CookieOptions {
   domain?: string;
   path?: string;
   secure?: boolean;
-  sameSite?: string
 }
 
 export interface MetricsOptions {
@@ -47,7 +46,7 @@ export interface MetricsOptions {
   maxQueueSize?: number;
 }
 
-export interface StoreOptions {
+interface StoreOptions {
   enabled?: boolean;
 }
 

package.json

@@ -1,7 +1,7 @@
 {
   "name": "@segment/analytics.js-core",
   "author": "Segment <friends@segment.com>",
-  "version": "4.1.5",
+  "version": "4.1.4",
   "description": "The hassle-free way to integrate analytics into any web application.",
   "types": "lib/index.d.ts",
   "keywords": [
@@ -30,6 +30,7 @@
   },
   "homepage": "https://github.com/segmentio/analytics.js-core#readme",
   "dependencies": {
+    "@ndhoule/defaults": "^2.0.1",
     "@segment/canonical": "^1.0.0",
     "@segment/cookie": "^1.1.5",
     "@segment/is-meta": "^1.0.0",

test/analytics.test.ts

@@ -11,7 +11,6 @@ var pageDefaults = require('../build/pageDefaults');
 var sinon = require('sinon');
 var tick = require('next-tick');
 var trigger = require('compat-trigger-event');
-
 var Identify = Facade.Identify;
 var cookie = Analytics.cookie;
 var group = analytics.group();
@@ -829,22 +828,6 @@ describe('Analytics', function() {
       });
     });
 
-    it('should should not overwrite context.page values due to an existing bug', function() {
-      analytics.page({ prop: true, context: { page: { search: "lol" } } }, { context: { page: { search: "" } } });
-      var page = analytics._invoke.args[0][1];
-      // FIXME: This assert should fail once the bug is fixed
-      assert.notDeepEqual(page.context(), {
-        page: {
-          ...defaults,
-          search: "lol",
-        },
-      });
-      // FIXME: This assert should fail once the bug is fixed
-      assert.deepEqual(page.context(), {
-        page: defaults,
-      });
-    });
-
     it('should emit page', function(done) {
       analytics.once('page', function(category, name, props, opts) {
         assert(category === 'category');
@@ -1625,29 +1608,6 @@ describe('Analytics', function() {
       assert.deepEqual(track.context(), { page: contextPage });
     });
 
-    it('should support overwriting context.page fields', function() {
-      analytics.track(
-        'event',
-        {},
-        {
-          context: {
-            page: {
-              title: 'lol'
-            }
-          }
-        }
-      );
-
-      var track = analytics._invoke.args[0][1];
-      assert.deepEqual(
-        track.context().page,
-        {
-          ...contextPage,
-          title: 'lol'
-        }
-      );
-    });
-
     it('should accept context.traits', function() {
       analytics.track('event', { prop: 1 }, { traits: { trait: true } });
       var track = analytics._invoke.args[0][1];

test/cookie.test.ts

@@ -63,15 +63,6 @@ describe('cookie', function() {
       assert(cookie.options().maxage === 31536000000);
     });
 
-    it('should have default options', function() {
-      cookie.options({ domain: '' });
-
-      assert(cookie.options().maxage === 31536000000);
-      assert(cookie.options().path === '/');
-      assert(cookie.options().domain === '');
-      assert(cookie.options().sameSite === 'Lax');
-    });
-
     it('should set the domain correctly', function() {
       cookie.options({ domain: '' });
       assert(cookie.options().domain === '');

test/normalize.test.ts

@@ -64,18 +64,6 @@ describe('normalize', function() {
         }
       });
     });
-
-    it('should merge with defaults', function() {
-      opts.context = { foo: 5 };
-      var out = normalize(msg, list);
-      assert.deepEqual(out.integrations, {});
-      assert.deepEqual(out.context, { foo: 5 });
-
-      msg.options = { integrations: { Segment: true }, context: { foo: 6 } };
-      out = normalize(msg, list);
-      assert.deepEqual(out.integrations, { Segment: true });
-      assert.deepEqual(out.context, { foo: 6 });
-    });
   });
 
   describe('integrations', function() {