Merge function does not overwrite if new value is falsy

[jcheroske]

Given the following code:

{{ dict "foo" "bar" | merge (dict "foo" "abc") | toPrettyJson }}

The output is as expected:

{
  "foo": "abc"
}

But if the second dict's key is falsy, the value is not overwritten:

{{ dict "foo" "bar" | merge (dict "foo" 0) | toPrettyJson }}

Results in:

{
  "foo": "bar"
}

Any falsy value will be skipped:

{{ dict "foo" "bar" | merge (dict "foo" 0) | toPrettyJson }}
{{ dict "foo" "bar" | merge (dict "foo" "") | toPrettyJson }}
{{ dict "foo" "bar" | merge (dict "foo" false) | toPrettyJson }}
...etc

This is not the behavior of Javascript's Object.assign:

Object.assign({}, {foo: "bar"}, {foo: 0}) # <-- {foo: 0}

Sprig's behavior feels extremely counter-intuitive. When I'm merging dicts, if a key has a value assigned, that's the value I care about. It's language-specific falsyness feels irrelevant in this context.

[Overbryd]

I found the same.
Triggered in my helm templating. Both are of type map[string]interface{}.
Helm uses Masterminds/sprig to provide merge functionality for their template values.

This breaks the idea and "contract" implemented here: https://github.com/Masterminds/sprig/blob/master/dict.go#L80
I will also create an issue on their side.

I whipped up an example that shows the unexpected behaviour:

package main

import (
  "fmt"
  "github.com/imdario/mergo"
)

func main() {
  map1 := map[string]interface{}{
    "deep": map[string]interface{}{
      "enabled": true,
    },
    "key": "default",
  }
  map2 := map[string]interface{}{
    "deep": map[string]interface{}{
      "enabled": false,
    },
    "key": "overwritten",
  }

  mergo.Merge(&map1, map2)

  fmt.Println(map1)
}

Expected output

map[deep:map[enabled:false] key:overwritten]

Actual output

$ go get -u github.com/imdario/mergo
map[deep:map[enabled:true] key:default]

---

Cross referenced from darccio/mergo#91

[Overbryd]

It seems like the correct function invocation should be:

mergo.Merge(&map1, map2, mergo.WithOverride)

Then it overwrites/merges those values correctly.
See darccio/mergo#91 (comment)
And darccio/mergo#91 (comment)

[Overbryd]

https://github.com/Masterminds/sprig/blob/master/dict.go#L82

[Dean-Coakley]

Looks like this had been resolved/can be closed @jcheroske ?

[Overbryd]

Yes, with the addition of mergeOverwrite this is solved. Now even available in helm. Have fun deep merging to you all :)

…

On Fri 15. Mar 2019 at 01:34, Dean Coakley ***@***.***> wrote: Looks like this had been resolved/can be closed @Overbryd <https://github.com/Overbryd> ? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#134 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABSdzHLE6_bwO0MFnsWvOw5nQ_mnMsSks5vWuqhgaJpZM4ZkKev> .

[mattfarina]

With mergeOverwrite this is now supported.

[sshishov]

The issue is still happening on latest HELM3:

helm version
version.BuildInfo{Version:"v3.2.4", GitCommit:"0ad800ef43d3b826f31a5ad8dfbb4fe05d143688", GitTreeState:"dirty", GoVersion:"go1.14.3"}

Both merge and mergeOverwrite does not consider "falsy" value.

This example:

{{ dict "foo" "bar" | merge (dict "foo" 0) | toPrettyJson }}

Produces:

{
  "foo": "bar"
}

[olfway]

It seems that order of args should be reversed for mergeOverwrite:

merge0: "{{ merge (dict "merge" 0) (dict "merge" "default") | toString }}"
merge1: "{{ merge (dict "merge" 1) (dict "merge" "default") | toString }}"
mergeOverwrite: "{{ mergeOverwrite (dict "mergeOverwrite" "default") (dict "mergeOverwrite" 0) | toString }}"

Produces:

merge0: map[merge:default]
merge1: map[merge:1]
mergeOverwrite: map[mergeOverwrite:0]

helm version
version.BuildInfo{Version:"v3.3.0", GitCommit:"8a4aeec08d67a7b84472007529e8097ec3742105", GitTreeState:"dirty", GoVersion:"go1.14.7"}