Yomguithereal / baobab

JavaScript & TypeScript persistent and optionally immutable data tree with cursors.
MIT License
3.15k stars 117 forks source link

Monkeys don't emit update events? #448

Closed oortlieb closed 6 years ago

oortlieb commented 8 years ago

I'm using monkeys to join a bunch of separate paths into an array with all of my data. My dependencies look like this:

A(cursor)              B(cursor)    C(cursor)
          \            /            /
            D(monkey)              /          
                      \          /
                        E(monkey)

When any of A, B, or C update, the appropriate monkeys are updated. However, the monkeys themselves do not appear to emit update events, which means that I don't get the updated monkey values unless I periodically re-check them.

I could watch the cursors for updates and re-evaluate the monkeys, but then I have to declare the monkeys' dependencies everywhere it's used. #358 says that monkeys aren't cursors -- am I experiencing the intended monkey behavior? What is the suggested way to get an update event from a monkey?

dumconstantin commented 8 years ago

+1 Experiencing the same issue, any chance this will be fixed?

dumconstantin commented 8 years ago

Here's a jsbin with this issue: http://jsbin.com/ziyopuqohe/edit?html,js,console

@oortlieb is this somewhat your use case?

Zache commented 8 years ago

Monkeys are lazy by default so you need to add a watcher.get() or similar to get the monkey to actually evaluate. Den 30 maj 2016 15:25 skrev "Constantin Dumitrescu" < notifications@github.com>:

Here's a jsbin with this issue: http://jsbin.com/ziyopuqohe/edit?html,js,console

@oortlieb https://github.com/oortlieb is this somewhat your use case?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/Yomguithereal/baobab/issues/448#issuecomment-222492184, or mute the thread https://github.com/notifications/unsubscribe/ADYLwAGjdW98Yev2JHTSTDUkbacBT_j2ks5qGuUvgaJpZM4IVZsW .

dumconstantin commented 8 years ago

Indeed, the only way to know if a monkey's value has changed is to evaluate it (regardless if its dependencies have changed, the monkey's might not)

But, how about an EAGER UPDATER flag on the cursor/watcher so that you get updates for anything that occurs on the dependency chain?

watcher.on('eager-update', x => {
   console.log('getting every update here!!! and I can decide! if I do trigger')
   if (x.theChangedData !== "something I know will not need evaluation") {
      x.currentData()
   }
})

Otherwise, we would have to do it like this:

tree.on('update', x => {
// very complex logic in order to know if a monkey does indeed need to evaluate
})

The fact is that monkeys (dynamic nodes) are meant to be linked in hierarchies. In my view at least, you can encapsulate all of the application logic in dynamic nodes that define derived data based on the essential state your application runs on.

oortlieb commented 8 years ago

Sorry for disappearing!

Yes, @dumconstantin, that's similar to my use case. I'm piling dynamic nodes on dynamic nodes, and my current method of watching paths and manually replacing cursors is getting unwieldy.

@Zache I thought the laziness could be the issue, but setting the lazyMonkeys option to false in my baobab didn't fix the issue. It's possible I did something incorrectly -- should that have fixed my problem?

Nimelrian commented 7 years ago

Hello! Is there any update on this? I tried fixing this issue myself last week, but ultimately failed, because I'm not very familiar with the inner workings of Baobab.

I have written the following test case which you can drop into the monkey suite:

describe('Issue #448 - Monkeys don\'t emit update events?', function () {
  it('update events emitted by a monkey should also reach the parents of the monkey.', function (done) {
    const tree = new Baobab({
      value: 5,
      monkeys: {
        valueSquared: monkey(['value'], value => value * value)
      }
    }, {
      lazyMonkeys: false
    });

    const newValue = 10;

    const parentCursor = tree.select('monkeys');
    parentCursor.on('update', () => {
      assert.strictEqual(tree.get(['monkeys', 'valueSquared']), newValue * newValue);
      done();
    });

    tree.set(['value'], newValue);
  });
});
dlio commented 6 years ago

+1 experiencing this issue myself as well and I am also personally unable to provide a solution. Thanks in advance to anyone who can take a closer look.

Yomguithereal commented 6 years ago

Hum... That's interesting. I wonder what's going on in the code here. If someone has some time to investigate the issue I'd be glad. I won't have time to check this very soon.

roark commented 6 years ago

@Yomguithereal -- I took a short dive into the code and noticed the following...

It appears that updated monkey path is not added to the tree._affectedPathsIndex and thereafter tree.commit() emits an update message without the monkey path in paths and therefore the parent cursor doesn't solveUpdate and emit an update event.

The tree._data does have the correct values for the monkey. But the event isn't fired for cursors that should update based on the monkey path.

It also appears that the difference is that monkey.update() uses the update.js function directly as opposed to the tree.update() function (which cursor.set() uses) that appends to the tree._affectedPathsIndex

How would you recommend the monkey, when evaluated , add its path to the tree._affectedPathsIndex?

Yomguithereal commented 6 years ago

I must admit my memory is a bit blurry on the subject. Let me check that at the beginning of next week. Don’t hesitate to ping me if I forget.

roark commented 6 years ago

@Yomguithereal -- Thanks for avid engagement, truly appreciated it. I put in a Pull Request that fixes the issue. Let me know if it needs any refactoring.

Yomguithereal commented 6 years ago

Merged the PR and released 2.5.1 on npm. Thanks @roark.