NorthwoodsSoftware / gojs-angular

A set of Angular components to manage GoJS Diagrams, Palettes, and Overviews
Other
61 stars 16 forks source link

DiagramComponent - Stuck in an infinite recursive loop #12

Closed cdelgadob closed 3 years ago

cdelgadob commented 4 years ago

There is a case that makes the method mergeChanges(component, kvchanges, str) from the gojs-angular library get stuck in an infinite loop. More specifically, is a non-ending recursion call in the compareObjs(obj1, obj2) method.

I think it's caused by the circular structure discussed here.

My guess is that in the line if (!compareObjs(obj1[p], obj2[p])) it never ends, because of that circular structure mentioned before.

Here is the code, just imagine that the obj1 has a reference to itself in any property: the recursion would never end.

function compareObjs(obj1, obj2) {
            // Loop through properties in object 1
            for (const p in obj1) {
                // Check property exists on both objects
                if (obj1.hasOwnProperty(p) !== obj2.hasOwnProperty(p))
                    return false;
                switch (typeof (obj1[p])) {
                    // Deep compare objects
                    case 'object':
                        if (!compareObjs(obj1[p], obj2[p]))
                            return false;
                        break;
                    // Compare values
                    default:
                        if (obj1[p] !== obj2[p])
                            return false;
                }
            }
            // Check object 2 for any extra properties
            for (const p in obj2) {
                if (typeof (obj1[p]) === 'undefined')
                    return false;
            }
            return true;
        }
rjohnson465 commented 3 years ago

Interesting, did you run into this issue yourself? Can you provide some sample data that would reasonably be used in this function that triggers this issue?

rjohnson465 commented 3 years ago

Closing as stale (and perhaps no longer relevant, since 2.0 has been published and no longer uses this function)

Feel free to comment if this needs reopening