dustingetz / react-cursor

Immutable state for React.js
1.03k stars 50 forks source link

API to remove me from parent #21

Closed krawaller closed 9 years ago

krawaller commented 10 years ago

During my plays with react-cursor I've often wished I could delete a cursor, removing it from its parent (if it has one). Or am I misunderstanding something in barking up this tree? Say for example we wanted to add a delete button to the Clicker components in the helloworld webapp example. Is there a better way to do that without my imaginary delete API?

dustingetz commented 10 years ago

Here is how I do it

var counts = cursor.refine('very', 'deeply', 'nested', 'counts'); counts.refine(0).onChange(42); // set the first counter to 42 counts.onChange(_.tail(counts.value)); // remove the first counter

I think in general to remove something from the collection you'll want to work with a cursor to the collection as a whole - because you want access to the entire collection API to remove/add/filter etc

krawaller commented 10 years ago

Ok, how about if I wanted to add a delete button to the JsonEditor and JsonLeafEditor classes of the JSON editor example? As per your suggestion I could do it by giving all components access to the full cursor, but that just seems so.. brutish and inelegant.

EDIT: I guess I could do it by giving all components their key and a cursor pointing to their parent. Still not as clean as a .delete call would be, but not too inelegant either.

dustingetz commented 10 years ago

giving all components their key and a cursor pointing to their parent

This is how I would do it.

I have not thought a delete() method through all the way yet and will think out loud: delete() will need to know the collection type. So if we limit cursors to only work with JSON types, we can teach delete() about maps and arrays. react-cursor isn't going to work too well with non-JSON types anyway since it relies on React.addons.update, so maybe that would be okay.

Note that if we finish https://github.com/dustingetz/react-cursor/issues/10, we would have access to splice directly from the cursor. I presume React.addons.update provides an immutable implementation of splice because the native javascript version mutates the target collection which is forbidden in react/cursors.

krawaller commented 10 years ago

Seems weird that React.addons.update doesn't have a $delete command for objects. Made a PR to add that.

More thinking out loud: A cursor could have a pointer to its parent (so we must hack refine to account for that). When .remove is called (remove is a better name for the method since it can then be map/array-agnostic), it calls splice or delete on the parent to remove the child.

A remove call should also make subsequent calls to any API method throw an error. As should calling remove on a non-refined cursor.

Hmm - maybe having all refined cursors carry a pointer to their parents is a can of worms?

krawaller commented 10 years ago

Hmm continued - since I might have a reference to a descendant elsewhere, when I remove a cursor, I must do a recursive dive into all children to throwify their API methods too.

EDIT: That won't work, as we won't get to eventual references that way. But if all cursors have access to their parents anyway, we can do a quick recursive check through parents to see if an ancestor has been deleted.

krawaller commented 10 years ago

And we must also decide what value a removed cursor should return. Or maybe trying to display a removed cursor is always an error? Ideally it should never happen, if the cursor is state in the top-level component.

dustingetz commented 10 years ago

Adding support to navigate back up the path wouldn't be a hack, real functional zippers support that. E.g. https://github.com/xsc/rewrite-clj

The argument against allowing react-cursor to navigate up, is that it breaks "encapsulation" (that word is loosely used as an analogy). One design goal is to be able to hand off a cursor to a react component, and be assured that the component's state is "boxed" to that cursor; it can't accidentally write to state somewhere else in the tree. So you can have a bunch of instances of a component, and each instance gets its own cursor, and you are guaranteed that all the instances have their own state and can't muck around with state that isn't theirs.

krawaller commented 10 years ago

I think that argument is a strong one. Spontaneously I say that although we internally keep a pointer to the parent in order to implement .remove, that should not be exposed to the user.

krawaller commented 10 years ago

Seems React are deprecating React.addons.update in favour of ImmutableJS.

dustingetz commented 10 years ago

Interesting, I don't know what to do about that, I guess we can keep an eye on it

dustingetz commented 9 years ago

Declined