stampit-org / stampit

OOP is better with stamps: Composable object factories.
https://stampit.js.org
MIT License
3.02k stars 103 forks source link

[Bug] RangeError: 'Maximum call stack size exceeded' when use setters #337

Closed GeorgeGkas closed 6 years ago

GeorgeGkas commented 6 years ago

I post a potential bug.

Package: @stamp/it Version: 1.0.3 Node: v10.5.0 Description: When try to set a property value using set property Descriptor the following error occurs: RangeError: Maximum call stack size exceeded.

Example:

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

const Stamp = stampit({
  props: {
    parent: undefined, // This property must be initialized in init().
  },
  propertyDescriptors: {
    parent: {
      configurable: false,
      enumerable: true,
      set(uuid) {
        this.parent = uuid;
      },
    },
  },
  init() {
    this.parent = null; // RangeError: Maximum call stack size exceeded
  },
});

export default Stamp;

The RangeError is thrown whenever try to assign a value in parent property, not just on init().

Test case:

import test from 'ava';
import { expect } from 'chai';
import Stamp from './Stamp'; // The above stamp.

test('should not throw when assign to parent property', () => {
  const obj = Stamp();
  expect(() => obj.parent = 'some-uuid').not.to.throw();
});
danielkcz commented 6 years ago

This is unrelated to stamps, it's a basic behavior of JS engine. You are causing infinite loop there. By setting a value to this.parent it calls set which again sets this.parent and then it loops till stack is full. I believe you should avoid defining that setter there, it's an implicit behavior to set the value of that property. Otherwise, you would need to add a check there for equality and not to set a value that's already there to break the loop.

GeorgeGkas commented 6 years ago

Oh yes just find out what cause it. I came to close this issue, but you already replied. Thank you that took time to answer my question. :)

koresar commented 6 years ago

Frankly speaking this issue freaked me out. :) You know, when you spend days testing the code adding hundreds of unit tests (I even moved the unit tests to its own module), and then someone finds a bug...

Much relief having you finding the cause! :) Thanks @FredyC ! Spot on 👍