Error with adding a node to the dataset after setOptions

[AlexDM0]

You can see the issue here:
https://jsbin.com/hesexidaxi/edit?html,console,output

[rasmusblockzero]

To me it looks like the main oddity is this.nodeOptions = options; in nodesHandler.setOptions.
It's set to incoming options without any parsing, instead of reflecting nodeOptions it becomes a snapshot what ever was the latest option to be set. It beats me why this is important to pass to new nodes but I also must say that there is a fair bit in the options complex that beats me.

In either case when this becomes undefined this cause a raise condition in the labelHandler

It was added fairly recently maybe @eymiha or @mojoaxel has some input there as they added the parameter ?

[wimrijnders]

Analysis of @rasmusblockzero is valid. The node options get reset completely when NodesHandles.setOptions() is called.

This also happens in:

• EdgesHandler

This does not happen in:

• Canvas
• CanvasRenderer
• Groups
• InteractionHandler
• KamadaKawai
• LayoutEngine
• ManipulationSystem
• NavigationHandler
• PhysicsEngine
• SelectionHandler

...in other words, in all the other modules with options.

So there is a discrepancy between the handling of options for Nodes & Edges, and the rest of the code.

I admit this is confusing; while it is understandable that sometimes options need to get initialized from scratch, it is not possible to update just a subset of the options. IMHO this would be a useful feature. Will think about this.

[wimrijnders]

As a workaround, the easiest way to avoid the error in the example is to set options.nodes.font.multi: false. This will skip the code in which the error occurs (Label.propagateFonts()). The example doesn't use the multi-font options anyway.

[wimrijnders]

Added PR #3055 to fix the occurrence of the reported error. The example will now work as expected.

It should be noted that this is a partial fix; the issue with the resetting of the Nodes & Edges options is not addressed.

[AlexDM0]

Thanks for the quick fix! Should an issue be made for that?

…

On 21 May 2017, at 21:19, Alexander Wunschik ***@***.***> wrote: Closed #3025. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

[wimrijnders]

Reopening because the fix was partial; all it did was hide the error. This issue still needs some work.

[AlexDM0]

I might be able to explain some of the confusion around the handling of the options for the nodes and edges. When Jos and I designed that part of the system we thought that we should not change the data of the user. If you use a dataset and load the nodes into the network, you should now change the properties of those nodes by setting options on the network itself using setOptions. That's why the nodeOptions object is used to fuse the options of the node itself and the globals of the network. For all other modules (except edges) this is irrelevant since those do not come from the dataset. The node object (the class) needs to have access to all options at all time and we don't want to change de data of the nodes from the dataset. I'm not sure what extensions have been made on the system after handing it over a bit more than a year ago but maybe the initial idea helps to shed some light on the concept. Cheers

…

On 22 May 2017, at 10:01, wimrijnders ***@***.***> wrote: Reopened #3025. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

[wimrijnders]

@AlexDM0 Thanks for the explanation.

However, what you say does not conflict the current working in any way. The node options are copied into the Node instances, not used directly from the DataSet instance. I'm not sure if this functionality was added after your time, but it is how it works now.

Also, I think it is important to note the several levels of the options. The chain is as follows (from #3060):

node options -> group options -> nodesHandler options -> default options

... where node options are specific per node. Ultimately, these are options stored in the DataSet.

What is currently happening, is that the NodesHandler options are zapped whenever setOptions() is called. These are not options from the DataSet.

Finally, I'd like to mention that @rasmusblockzero and I opened #3060 to thoroughly review and overhaul the handling of options in network. It's confusing as it is now.

[rasmusblockzero]

Agree, I think the original architecture as I understand it from your explanation is still active and valid. The somewhat messy parts are things that as far as I can tell is going against the clean fusion and that is what we intend to straighten out.

[AlexDM0]

From what I remember, the core of the options propegation were the brige methods in the util class. They take two objects A and B, and all nested objects were bridged, de pointers of the objects are copied, no values are. This inheritance makes sure that all fields that do not exist in A are inherited from B.

…

On Mon, May 22, 2017 at 10:40 AM, Rasmus Hedin ***@***.***> wrote: Agree, I think the original architecture as I understand it from your explanation is still active and valid. The somewhat messy parts are things that as far as I can tell is going against the clean fusion and that is what we intend to straighten out. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#3025 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AFHWTRLyAwjxsVcRbEMEGs9wL8YPgIOiks5r8Un4gaJpZM4NN4yG> .

[wimrijnders]

the core of the options propegation were the brige methods in the util class.

Yep, they're still there. That was my main insight today. TIL.
On top of that, there's a lot of cruft which seriously complicates the understanding of options. My hope is to reduce it once again to judicious use of util.bridgeObject().

---

Update: maintaining the prototype inheritance is not feasible, see #3486. There are several reasons for that, groups is one of them and the order of font propagation is another. I'm going to slowly phase it out.