Yomguithereal / baobab

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

Monkey in an array #483

Open basicdays opened 7 years ago

basicdays commented 7 years ago

Hey all,

Just ran into this bug. If I create a monkey that contains an array within its own path on the tree, it doesn't seem to be able to act as a getter. I'm currently on version 2.4.1. Here's some code to reproduce.

#!/usr/bin/env node
'use strict';
const Baobab = require('baobab');
const monkey = Baobab.monkey;

function alias(item) {
    return item;
}

let tree = new Baobab({
    stuff: 'things',
    alias: monkey({
        cursors: {data: ['stuff']},
        get: alias,
    }),
});
console.dir(tree.get());
console.dir(tree.get(['alias']));
console.log();

tree = new Baobab({
    stuff: 'things',
    aliases: [
        monkey({
            cursors: {data: ['stuff']},
            get: alias,
        }),
    ],
});
console.dir(tree.get());
console.dir(tree.get(['aliases', 0]));
console.log();

tree = new Baobab({
    stuff: 'things',
    aliases: [
        {
            item: monkey({
                cursors: {data: ['stuff']},
                get: alias,
            }),
        },
    ],
});
console.dir(tree.get(), {depth: 5});

And its output:

{ stuff: 'things', alias: [Getter] }
{ data: 'things' }

{ stuff: 'things',
  aliases:
   [ MonkeyDefinition {
       type: 'object',
       getter: [Function: alias],
       projection: [Object],
       paths: [Object],
       options: {},
       hasDynamicPaths: false } ] }
MonkeyDefinition {
  type: 'object',
  getter: [Function: alias],
  projection: { data: [ 'stuff' ] },
  paths: [ [ 'stuff' ] ],
  options: {},
  hasDynamicPaths: false }

{ stuff: 'things',
  aliases:
   [ { item:
        MonkeyDefinition {
          type: 'object',
          getter: [Function: alias],
          projection: { data: [ 'stuff' ] },
          paths: [ [ 'stuff' ] ],
          options: {},
          hasDynamicPaths: false } } ] }

Will probably rearchitect how I'm approaching the problem to work around this as I don't know the performance impact anyways.

Yomguithereal commented 7 years ago

I'll try to check that soon but yes, having monkeys in an array is probably a bad idea performance-wise.

basicdays commented 7 years ago

Just started poking around in the source about it and found this in baobab.js line 166:

  /**
   * Internal method used to refresh the tree's monkey register on every
   * update.
   * Note 1) For the time being, placing monkeys beneath array nodes is not
   * allowed for performance reasons.
   *
   * @param  {mixed}   node      - The starting node.
   * @param  {array}   path      - The starting node's path.
   * @param  {string}  operation - The operation that lead to a refreshment.
   * @return {Baobab}            - The tree instance for chaining purposes.
   */
  _refreshMonkeys(node, path, operation) {

If you'd like, I just forked the repo. I could just add a notice in the readme that monkeys that are on a path with an array are not supported at this time for a PR. Wouldn't mind getting a bit more involved in this project. :)

Yomguithereal commented 7 years ago

I recall writing something about this in the README concerning monkeys and arrays but I guess I forgot. My bad. Don't hesitate to open a PR :).