Watch.JS icon indicating copy to clipboard operation
Watch.JS copied to clipboard

recursion + infinite loops

Open timoxley opened this issue 12 years ago • 5 comments

Consider this case:

given a watched object, "dad", dad's "child" property is set to an object, which contains a value "parent" that refers back to "dad".

I think this creates two problems:

  1. whenever you set a watched properties value to an object Watch.JS very brazenly assumes I want all of it's properties watched as well… and if it tries to watch a property that is already a watched object, then it calls watch on it again, which then watches the property that contains a reference to itself which then it tries to watch… etc, etc.
  2. JSON.stringify does not support circular structures, and JSON.stringify is how you are currently determining if two values are the same… thinking about this, it's probably also causing "2", 2 and {toString: function() {return 2}} to be equal.

I've got a 'componentized' fork in which I've applied fixes for this to. My fork fixes this test for timoxley/react. Feel free to adopt something similar.

Also, +1 on #22.

timoxley avatar Jan 22 '13 09:01 timoxley

can you make a pull request?

melanke avatar Feb 21 '13 13:02 melanke

Apart from the JSON.stringify thing (that leads to issues like https://github.com/melanke/Watch.JS/issues/37) I think the problem is that the setter defined in the defineWatcher function doesn't consider the level property, in fact on line 270:

if (obj[prop]){
    watchAll(obj[prop], watcher);
}

i.e. the watchAll function is always called, also if the user has specified a fixed recursion level, such as 0 or 1. This can be fixed by passing the level property to the defineWatcher function and changing the above code to:

if (level !== 0 && obj[prop]){
    // watch sub properties
    watchAll(obj[prop], watcher, (level===undefined)?level:level-1);
}

In this case the infinite recursion happens only with an undefined recursion level and circular objects, but this should be the intended working.

I'm currently testing this fix, if all goes well, I could make a pull request.

Maluen avatar Apr 09 '13 13:04 Maluen

@Maluen I would appreciate that. Thank you

melanke avatar Apr 09 '13 13:04 melanke

@melanke would love to issue PR, just busy, you can see the fixes for it in my code though.

timoxley avatar Apr 12 '13 09:04 timoxley

Yeah, I will check it later. I am so busy these times too hahahaha. There is a few things I want to change in this and other projects but a lot of payed job to do.

melanke avatar Apr 12 '13 12:04 melanke