stampit-org / stamp

Stamps - better OOP model
https://stampit.js.org
MIT License
25 stars 3 forks source link

stampit doesn't clone array props #81

Closed vivien-adnot-dr closed 3 years ago

vivien-adnot-dr commented 3 years ago

Hello, consider this code:

const stampit = require('@stamp/it');

const Operations = stampit({
  props: { operations: [] }
});

// the test
it('stampit does not clone array props', () => {
  const ops = new Operations();
  ops.operations.push('op 1');
  expect(ops.operations.length).toBe(1);

  const ops2 = new Operations();
  // this test fails, we receive ['op 1'] instead of []
  expect(ops2.operations).toEqual([]);
});

As you see in the code example, Stampit didn't clone the array when the second instance was created. Instead, it shared the reference from the first instance.

This is an unusual behaviour that should be fixed in my opinions.

PS: This little hack greens the test above:

const Operations = stampit({
  init() { this.operations = [] }
});

rerun the unit test above, now it passes.

koresar commented 3 years ago

Hello. Props are not deep cloned, that's by design. They are simply copied by reference.

If you want your array copied your might want to use "deepProps". https://stampit.js.org/api/deep-properties Anyhow. You've found the right "hack"! Initialising data in an initialiser is exactly the purpose of the initialisers. 😁

I welcome you to give more read to this page stampit.js.org

Cheers

vivien-adnot-dr commented 3 years ago

Hello @koresar, many thanks for your fast answer :) Indeed, i didn't know about deepProps, it sounds like a very good tool. However in my usecase, it's probably not a good solution, because it concatenates arrays. And I don't want that, I just want my arrays 100% isolated from each other.

So, I will continue to use init() for my usecase and I will definitely keep in mind deepProps for the future !

Cheers