Leaflet: L.Util.setOptions does not merge objects in config

Checklist

  • I’ve looked at the documentation to make sure the behavior isn’t documented and expected.
  • I’m sure this is an issue with Leaflet, not with my app or other dependencies (Angular, Cordova, React, etc.).
  • I’ve searched through the current issues to make sure this hasn’t been reported yet.
  • I agree to follow the Code of Conduct that this project adheres to.

Steps to reproduce

When extending a LeafletControl (maybe other classes as well), nested options are not merged, as I expect it. If I have an object within my options object, the merging only adds the explicitely newly given attributes and does not keep the old ones.

In the minimal example, I add a control to the map. The control can be configured through the options passed in the factory function L.control.simpleControl(options); The nested options object in the class L.Control-SimpleControl is overwritten by the given options, without considering already stored default values: this leads to undefined values in the control to the bottom right. In my oppinion, leaflet should store these values to allow both separation of variables and providing default values.

Expected behavior

Leaflet should add the newly given properties to the nested configuration like this:

If adding the object to the map with these options…

L.control.simpleControl({
            position: "bottomleft",
            configurationObject: {
                currency: "euro"
            }
        }).addTo(map);

… leaflet should merge the configuration object, so explicitely given values through the factory function are changed, but others should be kept in default.

    options: {
        heading: "Checkout",
        configurationObject: {
            amount: 20,
            currency: "euro",
            sign: "$"
        }
    }

Current behavior

When adding the object to the leaflet map, keys in options are overwritten and no default can be described in the options:

L.control.simpleControl({
            position: "bottomleft",
            configurationObject: {
                currency: "euro"
            }
        }).addTo(map);

Leaflet overwrites the keys in options.configurationObject:

        {
            position: "bottomleft",
            configurationObject: {
                currency: "euro"
            }
        }

overwrites the properties options.configurationObject.amount and options.configurationObject.sign.

I have condensed the issue in the following image:

image

Minimal example reproducing the issue

https://plnkr.co/edit/FffSux4MRKmllYyo

Environment

  • Leaflet version: 1.7.1, 1.8

About this issue

  • Original URL
  • State: open
  • Created 2 years ago
  • Comments: 20 (8 by maintainers)

Commits related to this issue

Most upvoted comments

I’m not against this adding this as an option. But perhaps this can also easily be done with existing JS features or a small lib?

We do not want to add dependencies when we can avoid this. So if this were to be implemented, then it would have to be in Leaflet itself.

However I think we should avoid complicating this functionality in Leaflet. One can even question if we still need to have this functionality at all since Object.assign and spread syntax are basically a more familiar and standardized way to accomplish this.

Perhaps this is more API that we should be stripping out for the 2.0 release?

Just be aware that Lodash is not tree-shakable. So if you are bundling it you are very likely to end up with all of the code in Lodash rather than just the utility you are importing. For that reason you are better of using lodash-es.

The Lodash team is aware of this, and it looks like it will be resolved in some future version (see https://github.com/lodash/lodash/issues/5107).

Instead of making a breaking change let us implement in L.Util.setOptions a flag for deep merges