Skip to content
This repository has been archived by the owner on Oct 14, 2018. It is now read-only.

API to remove me from parent #21

Closed
krawaller opened this issue Oct 12, 2014 · 11 comments
Closed

API to remove me from parent #21

krawaller opened this issue Oct 12, 2014 · 11 comments

Comments

@krawaller
Copy link
Contributor

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
Copy link
Owner

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
Copy link
Contributor Author

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
Copy link
Owner

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 #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
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Owner

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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

@dustingetz
Copy link
Owner

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

@dustingetz
Copy link
Owner

Declined

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants