-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Should immutable scope copy the object before passing it to the scope? #23
Comments
I ran into this today, with an object containing a Set. The Set is replaced by a new empty Set before the patch function is applied. console.log(O({object: {set: new Set([1,2])}}, {
object: O({
set: O(oldSet => {
const s = new Set([...oldSet])
s.add(3)
return s
}),
}),
}))
// Expected result (“fixed” version of Patchinko):
// { object: { set: Set (3) {1, 2, 3} } }
// Actual result (current version of Patchinko):
// { object: { set: Set (1) {3} } } |
@zkf thanks - this seals the deal for me. Unless I back out this procedural clone feature, there's currently no way for immutable Patchinko to deal with objects like Sets or Maps without special-casing for them. Because there's a potentially infinite number of conceivable object types, native and author-land, where the desirable way of cloning them can't be inferred (should a DOM node in a data structure be cloned with |
As suffered by @robinchew on the Meiosis chatroom, Patchinko explicitly (attempts) copies target objects before passing them to scopes. This is irritating, especially as seen in this test case, where the scope seems to be written purposely to avoid that behaviour.
I recall being conflicted about this before and justifying to myself that although this seemed to be taking immutability to bloody-minded levels, that was kind of the point of writing a specifically immutable version of Patchinko - if you don't want that, you can always use the explicit or constant flavours.
But, whenever I think about it, I can't really think of a case where an author would want a scope to be fed a copy. Scopes are designed for the purposes of explicit fine-grained control - the original use case was proxying guards around functions, where keeping the unmodified original reference is often essential. - and even where the author might for whatever reason really need a copy, well they can do that within the scope using Patchinko.
Here's the fixed version, where I simply drop the condition. Yay or nay?
The text was updated successfully, but these errors were encountered: