Skip to content

Commit

Permalink
Fix aliased default options
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
tbekolay committed Dec 19, 2017
1 parent b065bef commit 7db5cc4
Show file tree
Hide file tree
Showing 4 changed files with 105 additions and 5 deletions.
12 changes: 7 additions & 5 deletions src/Interactable.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
const clone = require('./utils/clone');
const is = require('./utils/is');
const events = require('./utils/events');
const extend = require('./utils/extend');
Expand Down Expand Up @@ -63,8 +64,9 @@ class Interactable {
if (option in defaults[action]) {
// if the option in the options arg is an object value
if (is.object(options[option])) {
// duplicate the object
this.options[action][option] = extend(this.options[action][option] || {}, options[option]);
// duplicate the object and merge
this.options[action][option] = clone(this.options[action][option] || {});
extend(this.options[action][option], options[option]);

if (is.object(defaults.perAction[option]) && 'enabled' in defaults.perAction[option]) {
this.options[action][option].enabled = options[option].enabled === false? false : true;
Expand Down Expand Up @@ -294,14 +296,14 @@ class Interactable {
options = {};
}

this.options = extend({}, defaults.base);
this.options = clone(defaults.base);

const perActions = extend({}, defaults.perAction);
const perActions = clone(defaults.perAction);

for (const actionName in actions.methodDict) {
const methodName = actions.methodDict[actionName];

this.options[actionName] = extend({}, defaults[actionName]);
this.options[actionName] = clone(defaults[actionName]);

this.setPerAction(actionName, perActions);

Expand Down
13 changes: 13 additions & 0 deletions src/utils/clone.js
Original file line number Diff line number Diff line change
@@ -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;
};
84 changes: 84 additions & 0 deletions tests/Interactable.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
const test = require('./test');
const d = require('./domator');

const Interactable = require('../src/Interactable');
const actions = require('../src/actions/base');

test('Interactable copies and extends defaults', t => {
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 actions.methodDict.test;
delete Interactable.prototype.testize;
delete defaults.test;

t.end();
});

test('Interactable copies and extends per action defaults', t => {
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, { 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 actions.methodDict.test;
delete Interactable.prototype.test;
delete defaults.test;
delete defaults.perAction.testModifier;

t.end();
});
1 change: 1 addition & 0 deletions tests/index.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
require('./Interactable');
require('./Interaction');

// Legacy browser support
Expand Down

0 comments on commit 7db5cc4

Please sign in to comment.