Allow cursor onChange function to intercept the cursor's new value

[pluma]

This is a small change that makes the cursor's onChange function more useful.

The cursor's update method's callback can currently return arbitrary data. This data is always passed to the onChange function, but then the cursor chokes on invalid data when it tries to return a new cursor.

This change adjusts the behaviour of the onChange function so it can optionally return new rootData for the cursor that should be used instead of what the update method's callback returned.

For an example where this change would allow a nicer application structure see this issue. Allowing cursors to receive arbitrary data that is then transformed into the correct format "upstream" (e.g. simply calling Immutable.fromJS on it) can be useful in React.

Example:

Old sadness:

var cursor = Cursor.from(someData, function (newData) {
  magic(newData); // some side-effect
});

// elsewhere

cursor.update(() => Immutable.fromJS({some: {raw: 'data'}}));

New hotness:

var cursor = Cursor.from(someData, function (rawNewData) {
  var newData = Immutable.fromJS(rawNewData);
  magic(newData); // some side-effect
  return newData;
});

// elsewhere

cursor.update(() => {some: {raw: 'data'}}); // no need to require('immutable')

[leebyron]

What if the return value wants to be falsey or undefined? It seems like this implementation would discard it.

For example, maybe I want to use this enhancement to make sure the cursor to my "a" key is always an even number. This test will fail, because the return value 0 is treated as falsey and so the new value will continue even though my validator said it should not.

var data = Immutable.Map({a:0});
var cursor = Cursor.from(data, ['a'], (newValue, prevValue) =>
  newValue % 2 === 0 ? newValue : prevValue;
);
cursor.update(() => 1);
assert(cursor.deref() % 2 === 0);

[pluma]

I didn't want to break backwards-compatibility. I guess it could check for undefined instead of false-iness, though. That only leaves the edge case of explicitly returning undefined.

I think having the current behaviour (just use the new value) is useful and not returning anything or explicitly returning undefined should still trigger that.

Anyway, non-Immutable (root) values currently throw in makeCursor, so it's probably more of a hypothetical problem than a practical one.

[pluma]

I've adjusted the PR to only exclude undefined as return value. This maintains backwards-compatibility (otherwise all onChange functions would have to explicitly return their input if they don't want to alter the value).

If makeCursor should be able to wrap non-Immutable values (might be useful so we can have cursors that simply wrap an arbitrary value as a "leaf"), I'd suggest taking that to a separate PR or Gitter/IRC.

[pluma]

Any input/comments?

[natew]

👍 on this, would appreciate the functionality.