Cycle detected on latest version of mobx

[KatSick]

After updating to MobX 2.5.1 i encountered a problem with

mobx.js:2577 Uncaught Error: [mobx] Invariant failed: Cycle detected in computation [email protected] in 'function () {
                return this.$mobx.values[propName].get();
            }

How can i fix it? It not reproduces on previous versions...

[mweststrate]

Full stacktrace might be a good place to start. Best to set the max reported stacklines to infinite, and limit the stack depth, to keep it readable :). Or manually bail out in lastUpdate after a few iterations, and do a console.trace() inside of them.

[nsf]

Reproducable use case:

var mobx = require("mobx");

var objWrapper = mobx.observable({
        value: null,
});

var obj = {
        name: "Hello",
};

objWrapper.value = obj;
mobx.extendObservable(objWrapper, obj);

mobx.autorun(() => {
        console.log(objWrapper.name);
});

MobX 2.4.4 runs fine
MobX 2.5.2 fails with "cycle detected" exception

[nsf]

Oh and to add to the above. Is it something inherently wrong with doing what I do here? Extending an observable with another observable? And I see no cycles here.

[mweststrate]

@nsf that depends on which behavior you expect. extendObservable will one time copy the key / values from the source to the target (like object assign), so after extendObservable you will have two objects with different observables

[nsf]

But why does it reports cycles in this case then?

is it fair to say that extendObservable(a, b) is equivalent to extendObservable(a, toJS(b)) semantically?

[mweststrate]

@nsf yes. the cycle is probably a bug, will investigate. Is the cycle still reported if actually using the toJS?

[nsf]

@mweststrate runs fine with toJS.

[mweststrate]

@nsf / @ochervak proposal (see PR) is to fix this by explicitly forbidding this. Would this pose any problems for you?

[nsf]

I'm okay with that. It also gives an explicit hint that properties are not somehow magically reused, but a completely different observable object is created instead.

Also btw, I don't know if my issue matches originally reported issue here. So, can't speak for @ochervak. Not even sure if this fix fixes his problem. But it's a newly introduced cycle detection that wasn't here before that.