Yomguithereal / baobab

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

Provide a means for deepMerge to unset paths #382

Closed naturalethic closed 8 years ago

naturalethic commented 9 years ago

It would be convenient for deepMerge to unset properties with a value of undefined. Incidentally, properties with a value of undefined cannot be unset at all.

Yomguithereal commented 9 years ago

Hello @naturalethic. Making deepMerge unset undefined properties would not be a good thing if your properties are set (even to undefined). However, it you cannot unset properties that have undefined as value, then this is a bug I must fix.

naturalethic commented 9 years ago

Yeah, I admit the approach would possibly be surprising to the unaware. Perhaps then a sibling api deepMergeTrim -- or the like, where you can pass either a function of value to compare for unset, for trimming the tree. As of now, I must do a tree walk manually to unset anything that has been marked to be deleted by being set to undefined.

Yomguithereal commented 9 years ago

Can you describe your use case here @naturalethic, please? Why do you "mark" things to be deleted rather than deleting them right away?

naturalethic commented 9 years ago

For both transactional and syntactic purposes. We have a core data lib that manages a state object and any cursors on it. When I must call out to a supporting library to process a branch (cursor), and mutate it, I first serialize it to an object, then pass them that. This way they don't need to be aware of any baobab semantics. I take the returned, mutated object, and deepMerge it with the original cursor. However, I must be able to remove items that are so marked in the returned object.

I know I could achieve transactions with cursor.commit, and just forego the syntactic simplicity of pure objects, and I may end up doing that.

Yomguithereal commented 9 years ago

Good evening @naturalethic. I am not sure I should add a variant of deepMerge trimming undefined values. This should probably remain user-land. But would something like a withMutation method help you achieve your goal more easily?

naturalethic commented 9 years ago

I've decided to provide an unset function to the user which wraps the cursor method. What would a withMutation method provide?

Yomguithereal commented 9 years ago

A way to perform mutations on the data with performance in mind.

Example

cursor.set('a', 1);
cursor.set('b', 2);
// is more costly than
cursor.withMutations(function(object) {
  object.a = 1;
  object.b = 2;
  return object;
});

This would provide straightforward ways to apply series of complex mutations on targets efficiently in some scenario where perfs are paramount without compromising the tree's immutability.

naturalethic commented 9 years ago

Ok, would this also fire update events?

Yomguithereal commented 9 years ago

Of course. But at cursor level only. For instance, in the example above, the path updated would the one of the cursor, not cursor + a and cursor + b. I cannot fire updates lower without performing a diff or without using Object.observe and both methods are notoriously not performant.

Yomguithereal commented 9 years ago

You can see this as firing the same events as cursor.set(wholeNewData) would.

naturalethic commented 9 years ago

Ok. I don't know if this is going to work. I need for my listeners to know when anything they're interested in the tree changes, regardless of which cursor was responsible for the change. It may be that I will have to fork this, or find another library if that level of observability is not yet possible.

I understand that those two techniques you mentioned have performance costs, but that is a trade-off I'd be willing to make in order to maintain the simplicity of the architecture for the user.

It may be too much to ask of you, and perhaps I could implement it if you'd accept the PR, but would you be willing to provide a non-performant withDeeplyObservableMutations that fires events on all cursors watching the tree below the cursor being manipulated?

Yomguithereal commented 9 years ago

Cursor are memoized. Every other cursor everywhere will receive relevant events. And if you really need deep observations on your data, then the first method described in the above example will be more performant than observing deep mutations by hand with a dedicated method.

Yomguithereal commented 9 years ago

To be more precise, the cursors will fire an update regardless of which cursor was responsible:

var cursor1 = tree.select('a');
var cursor2 = tree.select('a');

cursor1.set(something);
cursor2.on('update', fn); // will of course fire

// under the hood
cursor1 === cursor2

What needs to be understood here is that cursor fire updates based on which paths of the tree have been affected, and not based on some cursor instance etc.

naturalethic commented 9 years ago

I understand that cursors are memoized. However, using your example above I need cursor + a and cursor +b to fire if their respective values change. I have been using deepMerge thus far, and it's implementation appears to respect that requirement.

naturalethic commented 9 years ago

I've decided that baobab is not quite the right fit for what I'm trying to do. I really appreciate your effort to understand my problem and explain the finer points of baobab. I hope that this conversation will be useful to others down the road.

If you're curious, I'm experimenting with fast-json-patch to keep me aware of deep object changes, as I use this library anyway to maintain consistency between state objects shared by the client and the server. For immutability, I'm simply making copies of a single session state.

Thanks again, you've been very helpful.

Yomguithereal commented 9 years ago

No problem @naturalethic. Don't hesitate to come back to this issue if you find things with fast-json-path that could improve the way the library is built.

If I recall correctly, fast-json-path perform diffs to track modifications (efficient ones, but diffs nonetheless). Unfortunately I cannot provide Baobab's users with such diffs as I perform comparisons on paths only (with some handy exceptions). This is more performant but oblivious of certain use cases that tend not to arrive when dealing with an app's centralized state.

Is the app you are building Open Source per chance? It sure would be interesting to me to understand your use case more thoroughly.

naturalethic commented 9 years ago

It is public however the architecture has diverged from the documentation. When things get a bit more solid I'll go back to the documentation and put together an architecture summary. If you're still curious, the relevant branch is here: https://github.com/naturalethic/olio/tree/ten18

naturalethic commented 9 years ago

@Yomguithereal I've extracted the work I've done for my overall problem into its own project, this should give you some clarity into exactly what it was I was needing.

https://github.com/naturalethic/rivulet

Yomguithereal commented 9 years ago

Thanks @naturalethic. Will check that.