YuriGor / deepdash

eachDeep, filterDeep, findDeep, someDeep, omitDeep, pickDeep, keysDeep etc.. Tree traversal library written in Underscore/Lodash fashion
https://deepdash.io/
MIT License
274 stars 12 forks source link

filterDeep review / refactoring #47

Closed YuriGor closed 3 years ago

YuriGor commented 4 years ago

Same like in filterDeep we have cloneDeep option to let the user specify his own cloning method, we need also to review all the deepdash methods where flat _.clone method used and also let user replace it.

YuriGor commented 4 years ago

Also check what's wrong with Lodash cloneDeep, as found by @GaborTorma in #56 It keeps deep function values instead of turning them into empty objects as per docs.

YuriGor commented 4 years ago

Opened yet another issue about this in Lodash

YuriGor commented 4 years ago

Currently, we have these issues with filterDeep: it pretends to return a filtered clone of a given object, but the quality of this clone is far from one provided by Lodash's clone/cloneDeep. filterDeep assumes the data consists of objects, arrays, and primitives. we need to carefully handle such cases as

console.clear();

class Test {
  constructor(x) {
    console.log('constructor',x); // called once
    if(!x)throw 'XXX!!!'; // no error, clone doesn't call it
    this.a = "a";
    this.b = "b";
    this.c = "c";
    this.x = x;
  }
  test(){
    console.log('test');
  }
  get z(){
    return 'z';
  }
}

const t = new Test('x');
t.d='d';
delete t.b;
t.hi = ()=>{console.log('hi')};
const tt = _.cloneDeep(t);

console.log(t); // Test {a: "a", c: "c", x: "x", d: "d"}
console.log(tt); // Test {a: "a", c: "c", x: "x", d: "d"}
t.test(); // works
tt.test(); // works
tt.hi(); // works, but I bieleve it's a Lodash's bug, according to docs tt.hi should by {} after cloning
console.log(t.test===tt.test); // equal because of this method comes from prototype
console.log(t.hi===tt.hi); // equal, but I bieleve it's a Lodash's bug, according to docs tt.hi should by {} after cloning
console.log(tt.z); // getters are functions in prototype, so it works
muuvmuuv commented 4 years ago

I would also like to add a new option to exclude the children from the output when it is empty. Example: https://codepen.io/yurigor/pen/GaKvNm?editors=0010 (parent 1.2 should not have children because it is empty)

YuriGor commented 4 years ago

you probably posted a wrong link - your example is just template without filtering done. But the option you are asking about is already there - read about "keepIfEmpty" ones https://deepdash.io/#filterdeep

muuvmuuv commented 4 years ago

Wow... I forgot to fork it, sorry. Yes, but only if I do not use childrenPath: https://codepen.io/muuvmuuv/pen/VweJLPe?editors=0011

YuriGor commented 4 years ago

Ok got it, will think about this. It's not clear what to do if childrenPath is deep. not just next level property. like here if this option is true - then we need to remove only property by the last key of children's path, not the whole path.