Yomguithereal / baobab

JavaScript & TypeScript persistent and optionally immutable data tree with cursors.
MIT License
3.15k stars 117 forks source link

Merge and update event: excessive event handler's calls #471

Open ruscoder opened 7 years ago

ruscoder commented 7 years ago

I found the odd behavior of update handler and merge.

Example:

const tree = new Baobab();
tree.set('test', {});
tree.select('test', 'value').set(1);

// Add update event handler
tree.select('test', 'value').on('update', (e) => console.log('update:', e.data.currentData, e.data.previousData)); 

tree.select('test', 'value').set(2)
// Update handler's output: update: 2 1

tree.select('test', 'value').set(2)
// Update handler wasn't called

tree.select('test', 'value').set(3)
// Update handler's output: update: 3 2

tree.select('test', 'anothervalue').set(1);
// Update handler wasn't called because it is another cursor

Before, everything was OK. Update handler was called only when data was changed.

tree.select('test').merge({ 'anothervalue': 1});
// Update handler's output: update: 3 3

tree.select('test').merge({ 'anothervalue': 1});
// Update handler's output: update: 3 3

I think it is bad behavior because data wasn't changed and cursor ['test', 'value'] always had the same value.

What do you think about this?

Yomguithereal commented 7 years ago

I am not sure to understand your case but what you need to know is that Baobab never perform data diff for performance reasons but instead compare paths. In the merge case, you edit the parent path, so I cannot infer that the underlying values have change or not, hence the event.

Yomguithereal commented 7 years ago

But I might change some handler strategies in the future.