Add onChange hook
- Adds an ~~onOperation~~ onChange hook that intercepts all proxy paths
- Very minimal performance impact
I'm not sure if this addresses #72 , but I'm opening this PR for discussion since the onOperation hook as implemented by this PR addresses the sequence and interleaving issue I needed a solution to.
Disclosure: Used claude code, with personal review of code.
Thanks for your PR. From an implementation perspective, it's cleaner to add an extra change hook in src/utils/mark.ts and use the JSON Patch structure.
Ah, interesting. I didn't notice this "all paths lead to markChanged" control point. Thank you.
@unadlib What do you think about this latest version? Changes of note:
- I've changed the name from
onOperationtoonChange, which is closer to your original proposal. ~~I could also cut outhooksif needed.~~ Removedhooks:. - I've consolidated the logic to center on
markChanged.
To accomplish the 2nd point, I needed to add additional key and operation params in order to pass this in to onChanged. I worry this may impact performance. Perhaps we should split into two functions?
- markChanged
- markChangedWithOnChange
Then we could have a ternary at each call point:
options.hooks?.onChange
? markChangedWithOnChange(target, key, { kind: 'set', prev: current, next: value })
: markChanged(target);
I haven't benchmarked this as I'm not familiar with your best practices.
I'm just re-reading your comment about using JSON Patch. I'm not sure if I understand this part of your reply, because the ProxyDraft passed to markChanged does not contain JSON Patch data afaict.
EDIT: Ah, I see I missed finalities. But wouldn't that mean the onChange is only aware of final patch data, not the actual each-line-item changes as they occur? (I believe my use case requires the latter).
I'm just re-reading your comment about using JSON Patch. I'm not sure if I understand this part of your reply, because the ProxyDraft passed to markChanged does not contain JSON Patch data afaict.
EDIT: Ah, I see I missed
finalities. But wouldn't that mean the onChange is only aware of final patch data, not the actual each-line-item changes as they occur? (I believe my use case requires the latter).
In fact, we need to add new parameters to markChanged for collecting the op type of each change that executes this function, as well as the paths of the changed node. Then, we trigger the onChange callback. This is likely the most concise approach and also eliminates the need to consider finalities.