From 44b2a1d94e7b0d97dc1d0d5f481c25ceb651f815 Mon Sep 17 00:00:00 2001 From: Wim Rijnders Date: Sat, 14 Oct 2017 06:12:11 +0200 Subject: [PATCH 1/2] Network: preload images in options for all shape types Fixes #3532 If the images option is set, preload it, even if the node shape is not `image` or `circularImage`. This needs to be done because the user can switch to an image shape later, as is happening in the issue. Option `image` is only mandatory for the image shapes. There is no unit test for this, because it would need a mock object for Image and I didn't succeed in creating/finding one. --- lib/network/modules/components/Node.js | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/lib/network/modules/components/Node.js b/lib/network/modules/components/Node.js index f4b70e11a..828d5c3f5 100644 --- a/lib/network/modules/components/Node.js +++ b/lib/network/modules/components/Node.js @@ -146,19 +146,20 @@ class Node { /** * Load the images from the options, for the nodes that need them. * - * TODO: The imageObj members should be moved to CircularImageBase. - * It's the only place where they are required. + * Images are always loaded, even if they are not used in the current shape. + * The user may switch to an image shape later on. * * @private */ _load_images() { - // Don't bother loading for nodes without images - if (this.options.shape !== 'circularImage' && this.options.shape !== 'image') { - return; + if (this.options.shape === 'circularImage' || this.options.shape === 'image') { + if (this.options.image === undefined) { + throw new Error("Option image must be defined for node type '" + this.options.shape + "'"); + } } if (this.options.image === undefined) { - throw new Error("Option image must be defined for node type '" + this.options.shape + "'"); + return; } if (this.imagelist === undefined) { From 8e8e1768a50cc1ee20b5712f6f1a4697e0149ede Mon Sep 17 00:00:00 2001 From: Wim Rijnders Date: Sat, 14 Oct 2017 06:50:47 +0200 Subject: [PATCH 2/2] Network: Handle null data gracefully During testing I discovered that passing `null` for network data leads to a thrown error. This adds a guard to prevent it, plus unit tests for regression. --- lib/network/Network.js | 4 ++++ test/Network.test.js | 19 +++++++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/lib/network/Network.js b/lib/network/Network.js index f5980b205..486a8a215 100644 --- a/lib/network/Network.js +++ b/lib/network/Network.js @@ -154,6 +154,10 @@ Emitter(Network.prototype); * @param {Object} options */ Network.prototype.setOptions = function (options) { + if (options === null) { + options = undefined; // This ensures that options handling doesn't crash in the handling + } + if (options !== undefined) { let errorFound = Validator.validate(options, allOptions); if (errorFound === true) { diff --git a/test/Network.test.js b/test/Network.test.js index 19ea7428f..b9f191c4c 100644 --- a/test/Network.test.js +++ b/test/Network.test.js @@ -395,6 +395,25 @@ describe('Network', function () { createSampleNetwork(options); }); + + it('can deal with null data', function() { + // While we're at it, try out other silly values as well + // All the following are wrong, but none should lead to a crash + var awkwardData = [ + null, + [1,2,3], + 42, + 'meow' + ]; + + var container = document.getElementById('mynetwork'); + + for (var n = 0; n < awkwardData.length; ++n) { + var network = new vis.Network(container, awkwardData[n], {}); // Should not throw + } + }); + + describe('Node', function () { it('has known font options', function () {