Cleanup mergeOptions()

[wimrijnders]

Fixing and testing of method util.mergeOptions().

I wasn't planning on doing this, but in the course of handling another issue, I discovered a bug in
util.mergeOptions() and then the more I looked at it, the more problems I found.
I can not unsee this, and therefore I hereby give it a torture test and proper fixing.

To be honest, I'm not sure if this is useful, since ES6 has Object.assign(), but I don't want
to screw up the expected functionality too much. However, mergeOptions() has changed a lot.

Changes:

• Removed parameter allowDeletion, this is not used within the method.
  • The impact of this removal for all modules except network is zero, because there allowDeletion is never used
  • For network, the impact is small
• Added unit tests for module util, in particular for mergeOptions(). Goals:
  • validate all paths within method mergeOptions()
  • ensure that incorrect input is handled properly
• The actual initial fix: missing option value should be handled properly, even when missing in
  globalOptions and when globalOptions not present.

[bradh]

+1 for the tests.

[bradh]

Looks like you're changing the signature. If this was exported, doesn't that make it public API? Perhaps keep the old method (deprecated) and remove later.

[wimrijnders]

This is not a documented API call and is only used internally between the vis components. I therefore have no qualms about changing the signature. I scanned all of the code under lib/, this adjustment is safe.

You get bonus points for your attention, though.

[bradh]

So we do need the export, but only for intra-vis module sharing?

[wimrijnders]

Yes, indeed. The sharing between vis modules also goes for the other source files in the same directory, except DataSet.js and DataView.js. When I squint at them, I actually think that lib/shared would be a better place to put them.