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.

Re: #567
  • Loading branch information
tbekolay authored and taye committed Jan 2, 2018
1 parent c05a05c commit 1cfaf27
Show file tree
Hide file tree
Showing 4 changed files with 112 additions and 1 deletion.
3 changes: 2 additions & 1 deletion 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 @@ -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]) {
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;
};
96 changes: 96 additions & 0 deletions tests/Interactable.js
Original file line number Diff line number Diff line change
@@ -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();
});
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');
require('./interactions');

Expand Down

0 comments on commit 1cfaf27

Please sign in to comment.