Item.set doesn't work as advertised

[aneilbaboo]

I noticed two problems in Item.set.

1. The two-argument usage advertised in the docs here:

tweet.set('content', 'This is the content')

doesn't work.

Inspecting tweet.attrs reveals the value to be

   { '0': 'c',
     '1': 'o',
     '2': 'n',
     '3': 't',
     '4': 'e',
     '5': 'n',
     '6': 't' }

2. Object values are merged

draft.set({foo: {a: 1}});
draft.get('foo'); // => {'a': 1} as expected

draft.set({foo: {b: 2}}); 
draft.get('foo'); // => {'a': 1, 'b': 2} !!!
                  // expecting {'b': 2}

The problem is that _.merge is used, which causes recursive merging.

_.extend should be used instead.

[rchl]

Changing from merge to extend would probably be backwards-incompatible change and potentially even dangerous if there are existing users that depend on merging strategy right now, no? It should probably be configurable either from the set call or globally but default to current behavior.

[aneilbaboo]

That's a legitimate worry. However, the current behavior is a bug. So perhaps there's a path to fixing this:

1. Add a property mergeWhenSettingObjectAttributes to the dynogels top level which sets an internal property
2. Add a conditional warning:
   • if this value is undefined, and use the default _.merge behavior:

   "Warning: Please explicitly set mergeWhenSettingObjectAttributes. see https:\\github.com\clarkie\dynogels\issues\174 for details. Current default behavior of Item.set, when value is an object, is to merge the new object with the existing object. In a future major release, this will change to overwrite. Use dynogels.mergeWhenSettingsObjectAttributes=false for overwrite behavior. Set to true for merge behavior."

   • else If value is set to falsey, use _.extend,
   • else value is truthy, so use _.merge.
3. On next major version bump, switch the behavior to _.extend.

[zolaemil]

Unexpected as it is, it seems like the behaviour is by design, because Item.prototype.update uses Item.prototype.set

dynogels/lib/item.js

Line 67 in 6246c13

self.set(item.attrs);