[Network] Uncaught TypeError: Cannot read property 'shape' of undefined

[strblr]

Hello,

This is the first time I have to post here, so before I introduce my issue, let me thank you for your amazing work.

Here is my case : Long story short, I have a network entirely controlled through the API. For instance, if I wanna delete a node, I select it and click "Delete" somewhere on my app. This sends a request to my server, which basically responds something like "You can / You cannot". When I can, I use the remove() method from the node DataSet. Works fine.

But then, when I press on my "Fit" button, which calls the Network fit() method, I get this error :

Uncaught TypeError: Cannot read property 'shape' of undefined
    at Function.getRange (vis.js:44421)
    at View.fit (vis.js:48582)
    at Network.fit (vis.js:33745)

The fit() function works however fine before removing something. And magically, sometimes I won't get that error. I tried but it's hard to figure out what are the exact conditions for getting it.
I fixed getRange() like this line 44419 of the webpacked vis.js (cf the not-undefined and not-null check) and it seems to make things work :

if (specificNodes.length > 0) {
   for (var i = 0; i < specificNodes.length; i++) {
      node = allNodes[specificNodes[i]];
      if(node === undefined || node === null)
	 continue;
      if (minX > node.shape.boundingBox.left) {
         minX = node.shape.boundingBox.left;
      }
      if (maxX < node.shape.boundingBox.right) {
         maxX = node.shape.boundingBox.right;
      }
      // .......
  }

Hope that will help.

Note :

• I'm using React 16, if that may change anything
• Browser : Chromium 61.0.3163.79

[wimrijnders]

Hi there. Your fix is fine for preventing the error. However, I'm really wondering why there is an undefined node there in the first place. The internal state of the Network instance should mirror the associated nodes DataSet.

I'll see if I can simply recreate this issue. If not, I may ask you to supply me with a demo of this happening.

---

How do you call network.fit()? Do you pass any parameters to it?

[wimrijnders]

Right. In the code snippet above, specificNodes is passed in the options parameter of method fit(). If it contains node id's of defunct nodes, node will be undefined.

Can you confirm that you are passing node id's into fit()? In that case, your fix is completely valid. Some extra commenting and a mention in the documentation might be required.

[strblr]

Thanks for responding that quickly.
Actually no, here are the options I pass to fit() :

{
    animation: {
        offset: { x: 0, y: 0 },
        duration: 1000,
        easingFunction: 'easeInOutQuad'
    }
}

[wimrijnders]

In that case, there is a discrepancy between the nodes defined and the nodes displayed. There is a node id in the display list which does not exist any more in the define list.

Which is a situation that should never ever occur, of course. Will need to go a bit deeper in the code for this.

[wimrijnders]

Have you set option edges.smooth.type by any chance? The default is dynamic, it might be that you set it to that value yourself.

[strblr]

Indeed, I set smooth: { type: 'continuous' }.

[strblr]

Here is my whole configuration generator, if that can help

export function graphOptions(edgeCreator, rightToEdit) {
    return {
        physics: false,
        interaction: {
            dragNodes: rightToEdit,
            multiselect: true,
            hover: true
        },
        manipulation: {
            enabled: false,
            addEdge: ({ from, to }, callback) => {
                if(from != to) edgeCreator(from, to)
            }
        },
        nodes: {
            size: 500,
            borderWidth: 0,
            shape: 'box',
            shadow: { size: 9, x: 1, y: 1 },
            font: {
                color: 'rgba(255, 255, 255, 0.7)',
                size: 17,
                face: "'Roboto Condensed', sans-serif"
            },
            labelHighlightBold: true
        },
        edges: {
            arrows: { to: { enabled: true, scaleFactor: 0.60 }},
            color: 'rgba(255, 255, 255, 0.5)',
            hoverWidth: 1,
            smooth: { type: 'continuous' }
        }
    }
}

[wimrijnders]

OK thanks, that should help. Need to stop now, real life is calling. Will pick it up again later when I'm free again.

[strblr]

Even with my fix, I get this. Here are two screenshots of what I get after spreading some nodes and calling fit() :

After some tests, I found this pattern :

• I refresh the page, the graph is fully re-rendered
• fit() works fine, no matter how far I drag the nodes
• I programmatically add a node (send a request to the server and the server actually makes me create a new node)
• The node is added to the graph (we can see, manipulate and focus() it) through the DataSet
• fit() just ignores that new node. Everything is scaled and dragged according to how the graph was before insertion.
• I refresh the page, fit() works fine again with all nodes.

In the two previous screenshots, the nodes that appear out of box were created after the first graph rendering.
This seems obviously related to my first issue, it seems like fit() just deals with the initial state.
Maybe the initial state reference is kept somehow ?

[wimrijnders]

Looking at the code of fit(), there's only one way that can happen, and that is if the node id is entered incorrectly in the visible nodes list. I guess that's a lead of some kind.

[wimrijnders]

I need a way to reproduce this. I'll try something myself by duplicating the steps you posted.

[strblr]

I just found what was wrong. I exported my fit() options from a separate file as an object. But after checking https://github.com/almende/vis/blob/master/lib/network/modules/View.js, I realized you actually mutate the option object line 43 :

if (options.nodes === undefined || options.nodes.length === 0) {
   options.nodes = this.body.nodeIndices;
}

Since I never rebuild the options, it won't match that condition again and keeps using the options.nodes set the first time. All my issues are related to this. I now export my options as a function returning a new object for each call, and it works. I would suggest clarifying in the documentation that the options are mutated, or just copy the parameters in fit() :

fit(options = {nodes:[]}, initialZoom = false) {
   let range;
   let zoomLevel;
   options = Object.assign({}, options);
   if (options.nodes === undefined || options.nodes.length === 0) {
     options.nodes = this.body.nodeIndices;
   }
   // ...

[wimrijnders]

A sincere thank you for looking into the problem yourself. I truly appreciate all external effort to fix issues.

I had zoomed into that particular statement myself, only I hadn't figured out what to do about it. I think your second option is the best approach here. Any options passed in should not be changed.

I need a regression test here; do you have a suggestion?

[strblr]

You're welcome, glad I helped. For the regression test, well I don't know, is there any internal call in Vis where the options mutation is actually relevant in further calls ? I cannot think of any reason that would be the case, but I don't know how it works in the inside.

Some options contain nested objects. Since assign() just makes shallow copies, the problem would remain the same if something is mutated in a nested option object. I would recommand using a deep copy algorithm like https://lodash.com/docs/4.17.4#cloneDeep

[wimrijnders]

I'm going to change tack here; the root of the problem is not that fit() leaves out nodes, but that the options have changed. I'm just going to unit test the latter.

In fact, I'm going to scan the whole code for similar problems, see #3548, and make unit tests for them all.

---

The vis.js analogue for cloneDeep() is deepExtend(), and yes, I may be grafting that in a bit more where appropriate. In any case, option mutation should be buried.

[wimrijnders]

Are you good for the moment? Or are you waiting for this fix to arrive? This for determining the urgency for next release, which officially is in a code freeze.

[strblr]

That's ok, everything works fine in my app now. Thank you