From 1cfaf27694780bbd0d56f9a270d1ccf4d9aace29 Mon Sep 17 00:00:00 2001
From: Trevor Bekolay <tbekolay@gmail.com>
Date: Thu, 14 Dec 2017 14:56:31 -0500
Subject: [PATCH] Fix aliased default options

Because extend does a shallow copy of object properties, some
options in interactable objects were aliased to objects in
the default objects. In some cases, setting options on one
interactable would bleed over to interactables made subsequently.

This commit adds a clone utility which is similar to extend,
but makes new objects instead of aliasing them. This allow the
newly added tests to pass.

Re: #567
---
 src/Interactable.js   |  3 +-
 src/utils/clone.js    | 13 ++++++
 tests/Interactable.js | 96 +++++++++++++++++++++++++++++++++++++++++++
 tests/index.js        |  1 +
 4 files changed, 112 insertions(+), 1 deletion(-)
 create mode 100644 src/utils/clone.js
 create mode 100644 tests/Interactable.js

diff --git a/src/Interactable.js b/src/Interactable.js
index 211a5ad3f..c48593b11 100644
--- a/src/Interactable.js
+++ b/src/Interactable.js
@@ -1,3 +1,4 @@
+const clone     = require('./utils/clone');
 const is        = require('./utils/is');
 const events    = require('./utils/events');
 const extend    = require('./utils/extend');
@@ -62,7 +63,7 @@ class Interactable {
         // copy the object
         actionOptions[optionName] = extend(
           actionOptions[optionName] || {},
-          optionValue);
+          clone(optionValue));
 
         // set anabled field to true if it exists in the defaults
         if (is.object(defaults.perAction[optionName]) && 'enabled' in defaults.perAction[optionName]) {
diff --git a/src/utils/clone.js b/src/utils/clone.js
new file mode 100644
index 000000000..4a4c80122
--- /dev/null
+++ b/src/utils/clone.js
@@ -0,0 +1,13 @@
+const is = require('./is');
+
+module.exports = function clone (source) {
+  const dest = {};
+  for (const prop in source) {
+    if (is.object(source[prop])) {
+      dest[prop] = clone(source[prop]);
+    } else {
+      dest[prop] = source[prop];
+    }
+  }
+  return dest;
+};
diff --git a/tests/Interactable.js b/tests/Interactable.js
new file mode 100644
index 000000000..4467560b5
--- /dev/null
+++ b/tests/Interactable.js
@@ -0,0 +1,96 @@
+const test = require('./test');
+const d = require('./domator');
+
+const Interactable = require('../src/Interactable');
+const scope = require('../src/scope');
+
+
+test('Interactable copies and extends defaults', t => {
+  scope.actions = {
+    methodDict: { test: 'testize' },
+  };
+
+  Interactable.prototype.testize = function (options) {
+    this.setPerAction('test', options);
+  };
+
+  const defaults = require('../src/defaultOptions');
+  defaults.test = {
+    fromDefault: { a: 1, b: 2 },
+    specified: { c: 1, d: 2 },
+  };
+
+  const specified = { specified: 'parent' };
+
+  const div = d('div');
+  const interactable = new Interactable(div, { test: specified });
+
+  t.deepEqual(interactable.options.test.specified, specified.specified,
+    'specified options are properly set');
+  t.deepEqual(interactable.options.test.fromDefault, defaults.test.fromDefault,
+    'default options are properly set');
+  t.notEqual(interactable.options.test.fromDefault, defaults.test.fromDefault,
+    'defaults are not aliased');
+
+  defaults.test.fromDefault.c = 3;
+  t.notOk('c' in interactable.options.test.fromDefault,
+    'modifying defaults does not affect constructed interactables');
+
+  // Undo global changes
+  delete scope.actions;
+  delete Interactable.prototype.testize;
+  delete defaults.test;
+
+  t.end();
+});
+
+test('Interactable copies and extends per action defaults', t => {
+  scope.actions = {
+    methodDict: { test: 'testize' },
+  };
+
+  Interactable.prototype.testize = function (options) {
+    this.setPerAction('test', options);
+  };
+
+  const defaults = require('../src/defaultOptions');
+  defaults.perAction.testModifier = {
+    fromDefault: { a: 1, b: 2 },
+    specified: null,
+  };
+  defaults.test = { testModifier: defaults.perAction.testModifier };
+
+  const div = d('div');
+  const interactable = new Interactable(div, {});
+  interactable.testize({ testModifier: { specified: 'parent' } });
+
+  t.deepEqual(interactable.options.test, {
+    enabled: false,
+    origin: { x: 0, y: 0 },
+
+    testModifier: {
+      fromDefault: { a: 1, b: 2},
+      specified: 'parent',
+    },
+  }, 'specified options are properly set');
+  t.deepEqual(
+    interactable.options.test.testModifier.fromDefault,
+    defaults.perAction.testModifier.fromDefault,
+    'default options are properly set');
+  t.notEqual(
+    interactable.options.test.testModifier.fromDefault,
+    defaults.perAction.testModifier.fromDefault,
+    'defaults are not aliased');
+
+  defaults.perAction.testModifier.fromDefault.c = 3;
+  t.notOk('c' in interactable.options.test.testModifier.fromDefault,
+    'modifying defaults does not affect constructed interactables');
+
+  // Undo global changes
+  delete scope.actions;
+  delete Interactable.prototype.test;
+  delete defaults.test;
+  delete defaults.perAction.testModifier;
+
+  t.end();
+});
diff --git a/tests/index.js b/tests/index.js
index 40191ae27..3cefd7bf1 100644
--- a/tests/index.js
+++ b/tests/index.js
@@ -1,3 +1,4 @@
+require('./Interactable');
 require('./Interaction');
 require('./interactions');