Add support for non-enumerable properties

[BrendanAnnable]

Currently the object cloner does not support cloning non-enumerable properties. This has prevented me from using Baobab on tree structures which include non-enumerable properties.

e.g. The AST provided by recast has a non-enumerable original member for each node. Manipulating the tree with Baobab results in this property being lost on all cloned nodes.

This PR opts to use Object.getOwnPropertyNames instead of the for in loop, allowing non-enumerable properties to be cloned with their descriptors intact. This also mitigates the need for the use of hasOwnProp.

[Yomguithereal]

Hello @BrendanAnnable. Thanks for your PR. However, while I see the appeal of your addition, I have some concerns about performance. The fact that .getOwnPropertyNames creates an array & that .defineProperty is probably less efficient than a standard set operation makes me wonder whether adding this feature might slow down some uses cases. So, do you think the impact would be too much or do you think this is negligible?

[BrendanAnnable]

I understand your concern. Are there any performance tests which could be run to verify performance regressions? I'd be guessing without one.

[Yomguithereal]

You can try to run npm run benchmark to run a very naive benchmark and see if your changes affect it. This benchmark is not really clever but this would be start.

[BrendanAnnable]

Results

original master branch

# 1

Baobab#get x 468,103 ops/sec ±0.44% (99 runs sampled)
Baobab#set x 86,547 ops/sec ±1.20% (96 runs sampled)
Baobab#unset x 688,980 ops/sec ±0.67% (99 runs sampled)
Baobab.Cursor#set x 20,777 ops/sec ±2.67% (89 runs sampled)
Baobab.Cursor#unset x 720,595 ops/sec ±0.68% (98 runs sampled)
Baobab.Cursor#push x 986 ops/sec ±45.32% (9 runs sampled)
Baobab.Cursor#unshift x 338 ops/sec ±4.49% (66 runs sampled)
Baobab.Cursor#splice x 382 ops/sec ±4.22% (85 runs sampled)

# 2

Baobab#get x 436,471 ops/sec ±0.72% (98 runs sampled)
Baobab#set x 77,831 ops/sec ±2.87% (93 runs sampled)
Baobab#unset x 533,692 ops/sec ±1.09% (95 runs sampled)
Baobab.Cursor#set x 15,143 ops/sec ±5.35% (85 runs sampled)
Baobab.Cursor#unset x 668,186 ops/sec ±0.69% (96 runs sampled)
Baobab.Cursor#push x 1,009 ops/sec ±40.46% (10 runs sampled)
Baobab.Cursor#unshift x 379 ops/sec ±6.70% (69 runs sampled)
Baobab.Cursor#splice x 382 ops/sec ±4.25% (85 runs sampled)

# 3

Baobab#get x 345,590 ops/sec ±0.66% (95 runs sampled)
Baobab#set x 65,825 ops/sec ±0.72% (98 runs sampled)
Baobab#unset x 493,662 ops/sec ±0.73% (97 runs sampled)
Baobab.Cursor#set x 15,844 ops/sec ±2.72% (88 runs sampled)
Baobab.Cursor#unset x 535,177 ops/sec ±4.22% (91 runs sampled)
Baobab.Cursor#push x 937 ops/sec ±24.05% (20 runs sampled)
Baobab.Cursor#unshift x 436 ops/sec ±1.75% (81 runs sampled)
Baobab.Cursor#splice x 456 ops/sec ±0.54% (95 runs sampled)

copy-descriptors branch

# 1

Baobab#get x 428,240 ops/sec ±2.90% (92 runs sampled)
Baobab#set x 83,724 ops/sec ±1.32% (94 runs sampled)
Baobab#unset x 605,589 ops/sec ±1.65% (92 runs sampled)
Baobab.Cursor#set x 19,689 ops/sec ±2.44% (90 runs sampled)
Baobab.Cursor#unset x 700,189 ops/sec ±0.97% (93 runs sampled)
Baobab.Cursor#push x 1,018 ops/sec ±43.83% (9 runs sampled)
Baobab.Cursor#unshift x 376 ops/sec ±5.77% (68 runs sampled)
Baobab.Cursor#splice x 397 ops/sec ±3.89% (87 runs sampled)

# 2

Baobab#get x 428,240 ops/sec ±2.90% (92 runs sampled)
Baobab#set x 83,724 ops/sec ±1.32% (94 runs sampled)
Baobab#unset x 605,589 ops/sec ±1.65% (92 runs sampled)
Baobab.Cursor#set x 19,689 ops/sec ±2.44% (90 runs sampled)
Baobab.Cursor#unset x 700,189 ops/sec ±0.97% (93 runs sampled)
Baobab.Cursor#push x 1,018 ops/sec ±43.83% (9 runs sampled)
Baobab.Cursor#unshift x 376 ops/sec ±5.77% (68 runs sampled)
Baobab.Cursor#splice x 397 ops/sec ±3.89% (87 runs sampled)

# 3

Baobab#get x 363,735 ops/sec ±0.69% (94 runs sampled)
Baobab#set x 56,443 ops/sec ±5.89% (85 runs sampled)
Baobab#unset x 372,329 ops/sec ±3.25% (86 runs sampled)
Baobab.Cursor#set x 12,068 ops/sec ±2.99% (83 runs sampled)
Baobab.Cursor#unset x 510,168 ops/sec ±1.76% (97 runs sampled)
Baobab.Cursor#push x 891 ops/sec ±39.67% (11 runs sampled)
Baobab.Cursor#unshift x 392 ops/sec ±1.97% (82 runs sampled)
Baobab.Cursor#splice x 412 ops/sec ±1.35% (89 runs sampled)

I'm not sure how much we can conclude...

[Yomguithereal]

I guess we can conclude that the differnce is not incredible & that I can probably merge your PR :) If you are really zealous you could write a test using a very huge index, say an object with ~5000 keys & see if this slows things down.

[Yomguithereal]

And, out of curiosity, how are you using Baobab with an AST tree? Is this just for traversal or do you patch & listen to updates?

[BrendanAnnable]

I'll leave the zealous decision to you! :)

I'm building a library for easily traversing and manipulating ASTs (similar to jQuery). I do the traversal myself, but currently use Baobab internally for manipulating the AST with cursors. I currently do not have a need for listening to updates.

[Yomguithereal]

However, can I just ask you to add a unit test with a non-enumerable property so I can track regressions in the future please?

[BrendanAnnable]

Done 🙂

[Yomguithereal]

Thanks @BrendanAnnable. Merging right now.

[Yomguithereal]

Your update has been published through v2.4.0.