Stack overflow on circular references

[weavejester]

The build-tree function has a (not (identical? obj child)) check in it, but it doesn't account for instances where a child links back to an earlier ancestor.

I've also noticed that some libraries (such as Pixi.js v4) occassionally hold references to DOM elements, which obviously we don't want to extern.

My hacky solution has been to update build-tree to hold a list of parents to check for circular references. This didn't seem to affect DOM elements, so I also added in an additional check for (.-nodeType child). You may well ask why I didn't use (instance? js/Node child)! Well, for some reason it didn't appear to work on the constructed canvas element, and I was unable to come up with a better check than to look for the existance of nodeType.

(defn build-tree
  ([obj name]
   (build-tree obj name ()))
  ([obj name parents]
   {:name      name
    :type      (get-type obj)
    :props     (for [k (js-keys obj)
                     :let [child (obj/get obj k)]]
                 (if (and (parent-type? child)
                          (not (or (identical? obj child)
                                   (.-nodeType child)
                                   (some (partial identical? child) parents))))
                   (build-tree child k (conj parents obj))
                   {:name k :type (get-type child)}))
    :prototype (for [k (js-keys (.-prototype obj))]
                 {:name k :type :function})}))

By the way, here's a link to the generated file I was using: http://www.booleanknot.com.s3.amazonaws.com/pixi.inc.js

If you try to extern "PIXI", the stack overflow error is generated.

[jmmk]

Thanks for reporting!

I have no access to a computer for a few days, but I will try to take a
look when I am back.
On Jun 13, 2016 5:14 PM, "James Reeves" notifications@github.com wrote:

The build-tree function has a (not (identical? obj child)) check in it,
but it doesn't account for instances where a child links back to an earlier
ancestor.

I've also noticed that some libraries (such as Pixi.js v4) occassionally
hold references to DOM elements, which obviously we don't want to extern.

My hacky solution has been to update build-tree to hold a list of parents
to check for circular references. This didn't seem to affect DOM elements,
so I also added in an additional check for (.-nodeType child). You may
well ask why I didn't use (instance? js/Node child)! Well, for some
reason it didn't appear to work on the constructed canvas element, and I
was unable to come up with a better check than to look for the existance of
nodeType.

(defn build-tree
([obj name](build-tree obj name %28%29))
([obj name parents]
{:name name
:type (get-type obj)
:props (for [k (js-keys obj)
:let [child (obj/get obj k)]](if %28and %28parent-type? child%29
%28not %28or %28identical? obj child%29
%28.-nodeType child%29
%28some %28partial identical? child%29 parents%29%29%29%29
%28build-tree child k %28conj parents obj%29%29
{:name k :type %28get-type child%29}))
:prototype (for [k (js-keys (.-prototype obj))]
{:name k :type :function})}))

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#19, or mute
the thread
https://github.com/notifications/unsubscribe/ABEfwjgns4oYextMJ5MtCsbP1rGeywfsks5qLcgdgaJpZM4I0vg6
.

[jmmk]

The reason the (instance? js/Node child) call does not work but nodeType does is because we are inside an iframe (see: http://stackoverflow.com/a/13895357). So child instanceof Node == false but child instanceof document.getElementById('sandbox').contentWindow.Node == true.

I am able to make these changes and see it working, but I'm unable to come up with a small test case for the circular reference.

[weavejester]

Maybe if you just create a DOM element and hold onto it? I believe DOM elements have a circular references around parents and siblings.

[jmmk]

Just the nodeType check handles the dom node reference: bc957ae

[weavejester]

Hm, what about something like...

var x = {foo: {}};
x.foo.bar = x;

The problem was with circular references that were multiple levels deep, e.g. an element linking back to its grandparent.

[jmmk]

That did it, thanks!

[weavejester]

Thanks for fixing this!

[jmmk]

@weavejester thanks for the help! Just deployed the change to the web app