almende / vis

⚠️ This project is not maintained anymore! Please go to https://github.com/visjs
7.85k stars 1.48k forks source link

[util] extend() should be a deep-copy #2543

Open lewisjb opened 7 years ago

lewisjb commented 7 years ago

util.extend and util.selectiveExtend currently only copy over the first layer.

This can cause issues, seeing as the most common use for these functions is for options. Specifically: this.options = util.extend({}, this.defaultOptions); With a shallow-copy, any object-based options will be shared between options and defaultOptions (which defeats the purpose of them being separate).

The other common use is for applying custom options with selectiveExtend. With a shallow-copy, if you are only setting 1 option in an object, then the others won't be in the options (despite being in defaultOptions). e.g.

var options = {
  timeAxis: {
    scale: 'seconds'
  }
}

Once these options are copied over, the step option of timeAxis is no longer set.

This doesn't seem to have any impact on usability at the moment, as the settings have their default set in multiple places; but fixing this could stop an issue from happening in the future, and can remove the need to have the default in multiple places.

mojoaxel commented 7 years ago

:+1: