.reference() for neverending cursors

[torgeir]

A PR is easier for cooperation

[torgeir]

This will allow for something like

var clock = quiz.reference('clock');

let Clock = component('Clock', [ forceUpdateOn(clock) ], () =>
  div({},
    String(clock.cursor().deref())));

function forceUpdateOn (ref) {
  var bound;
  return {
    componentDidMount: function () {
      bound = ref.onChange(_ => this.forceUpdate());
    },
    componentWillUnmount: function () {
      bound.remove();
    }
  };
}

as per https://github.com/omniscientjs/omniscient/issues/42

[mikaelbr]

The current implementation is more like:

var clock = quiz.reference('clock');

let Clock = component('Clock', [ forceUpdateOn(clock) ], () =>
  div({},
    String(clock.cursor().deref())));

function forceUpdateOn (ref) {
  return {
    componentDidMount: function () {
      ref.onChange(_ => this.forceUpdate());
    },
    componentWillUnmount: function () {
      ref.unsubscribe();
    }
  };
}

(As it is unsubscribe on a reference level, not onChange-level)

[mikaelbr]

My main issue with this at this point is that it seems unnecessary to have many parallel equal listeners on "swap", instead of having one listener. One listener would make cleanup a more manual job, though.

I haven't tested the code extensively, nor have I written tests yet. This is just a concept / PoC, which we can discuss and possibly hack on. But initial run-through seems positive!

[dashed]

Agree with @mikaelbr that registering the structure once with a centralized function is sufficient. I do something similar with my own immstruct wrapper: https://gist.github.com/Dashed/e38d4184bc239268737e#file-listen-js-L84-L120

[torgeir]

Afaik, unsubscribing one listener and stopping a ref from any longer emitting changes are two separate things. One component should be able to unsubscribe its ref listeners without stopping other component's listeners

[mikaelbr]

@torgeir For now it is on reference level, not affecting anyone other then themself, but having one listener in common for every reference, would require some more logic for unsubscribing on reference level, yeah.

[torgeir]

That's my point excactly, you should be able to cancel subscritions as well as tear a whole ref

Edit: could probably be solved by using a list of functions that are looped and called if the key is the same on swap. unsubscribe() could remove the function instance from the list, while cancel could empty the list and remove the swap listener.

[dashed]

Why can't we make the function subscription a little bit smarter? We track by reference counting.

We subscribe changeListener to swap only when function passed to onChange is the first subscription. We unsubscribe changeListener from swap only when function removed from viaunsubscribe is the last listener.

changeListener subscription should be automatic. We, the users, shouldn't be handling this manually.

All we want and care about is adding and removing functions via onChange and unsubscribe.

[dashed]

I think Structure.prototype.reference should be renamed to something else. It may be misleading in that it refers to a static internal object associated with keyPath.

Imagine having var clock = quiz.reference('clock'); in two different components. Calling clock.unsubscribeAll() in one component doesn't affect clock object in the other component.

Maybe Structure.prototype.scope?

[dashed]

@mikaelbr looking good so far :+1:

[dashed]

Since Immutable is available. I'm wondering if the collections can be used instead of native object/array.

[mikaelbr]

Since Immutable is available. I'm wondering if the collections can be used instead of native object/array.

I started out with immutable structure, but it didn't really make the code easier or improve it much. Don't see any real gain at this point.

[mikaelbr]

Maybe Structure.prototype.scope?

We had a discussion on this, but we found that .reference still is more descriptive. You can have multiple references to a everlasting cursor, and I actually think this makes sense:

Imagine having var clock = quiz.reference('clock'); in two different components. Calling clock.unsubscribeAll() in one component doesn't affect clock object in the other component.

So, you are unsubscribing on all listeners for that reference, it shouldn't remove listeners on other references. They aren't the same references, just all references to the same data.

[dashed]

So, you are unsubscribing on all listeners for that reference, it shouldn't remove listeners on other references. They aren't the same references, just all references to the same data.

I need to think on this some more; maybe it'll grow on me :stuck_out_tongue:

/cc @skevy Thoughts?

[dashed]

onChange is pretty low-level. I feel that there should be sugar functions onAdd, onDelete, andonUpdate that wrap upon onChange.

// abstracted out
let analyze = function(newData, oldData, path) {

    var oldObject = oldData && oldData.getIn(path);
    var newObject = newData && newData.getIn(path);

    var inOld = oldData && hasIn(oldData, path);
    var inNew = newData && hasIn(newData, path);

    return {
        oldObject,
        newObject,
        inOld,
        inNew
    };
}

let onAdd = function(newFn) {

    return this.onChange(function(newData, oldData, path) {

        let ret = analyze(newData, oldData, path)
        if(!ret.inOld && ret.inNew) {
            newFn(path, ret.newObject)
        }
    })
}

// similar to onDelete and onUpdate

[dashed]

This may require to rename change event to update to be semantically consistent.

[dashed]

We'll need a way to completely destroy "everlasting cursors". It'll probably require a dead man's swtich variable, to make all methods of the reference cursor to be in noop mode.

We need this, because self._pathListeners[pathId] will become polluted with changeListener functions, even if the reference cursor is GC'd.

[dashed]

Throwing this idea in.

Since there is a cursor() method, I'm wondering if it's equally useful to have structure() method to get the structure object that's associated with the cursor/reference?

[dashed]

Putting here for posterity: Suggesting to rename to Structure.observe(keyPath) which returns an "observer" object.

See: https://gitter.im/omniscientjs/omniscient?at=54d621c1c4386fab2709bf4c

[skevy]

This is awesome that this is being worked on!

The only thing I would say with this is that the API for onChange is odd, simply because the structure is an EventEmitter, with .on. So I'm wondering if we also make the reference an EventEmitter, so that it the API between a structure and a reference is similar?

[dashed]

I'm throwing around this idea: https://gitter.im/omniscientjs/omniscient?at=54d6273cdb625d410e7c26bf

Usage story:

• Structure.reference(keyPath) would return an "everlasting cursor".
• And on this everlasting cursor, we can observer/unobserve on it.
• And destroy it.
• onChange and unsubscribeAll is renamed to observe and unobserveAll respectively.
• .observe(event, listener) and .observer(listener) where event is either add, delete, change, or swap.
• .observe(listener) is shorthand for .observe('swap', listener)

[skevy]

@Dashed you don't think using an EventEmitter and .on instead of .observe would be better? I was just thinking it'd be good to keep all the api's the same between the structure and the reference.

[dashed]

@skevy An EventEmitter is already used indirectly here: https://github.com/omniscientjs/immstruct/blob/8d7eba17e2ed19651f768ba58589b5915efdeeef/src/structure.js#L39-L44

To elaborate, Structure.reference(keyPath) is a more abstract concept wherein it creates and returns an everlasting cursor. An everlasting cursor holds a cursor of the same keyPath internally, and keeps that cursor fresh every time value at keyPath changes. Traditionally, cursors would become stale.

We add features to an everlasting cursor; such as observation. Through the everlasting cursor, we can 'observe' the internal cursor. That's why .on semantics (EventEmitter) isn't really used.

[dashed]

I was just thinking it'd be good to keep all the api's the same between the structure and the reference.

This may be a good argument against .observe(event, listener).

At the moment, onChange is way too low-level, in that it's pretty much equivalent to:

structure.on('swap', (newData, oldData ,path) => {
  if(!equalPath(path, keyPath))
    return;

  // ...
});

Thereby making the reference cursor dance a little silly.

[dashed]

Idea: Given a cursor, without an official public API to access the associated keyPath, we'd like to convert this into a reference cursor.

[mikaelbr]

I'm thinking this is getting mature enough to squash and merge? Any objections, folks?

[skevy]

Looks good to me!

[dashed]

Looks fine to me. I think later on, we should explore .on() or similar.

[mikaelbr]

At some point we can add specific evented listeners (add, change, delete). I have some stashed code for it, but for now it's not critical for this change. This is something we potentially can build on.

[skevy]

Awesome job @mikaelbr.

[mikaelbr]

Thanks! :cake:

[skevy]

Now we just need docs :)

[mikaelbr]

Indeed. Was just having the discussion with @torgeir . I'm going to open issues.

[skevy]

K.