Update strip-nils to only strip nils, not empty strings, maps, etc

[dominicfreeston]

I'm not sure if this is considered a bug, but it has recently caught us by surprise. If it's not a bug then:strip-nils option seems misnamed as it is more of a strip-empties, removing empty strings, maps, vectors, etc.

This change obviously potentially breaks backwards compatibility so I'm not sure if you'd want to include it as is (and whether you'd still want to support some strip-empties variant), but it does currently feel like a bug as the behaviour doesn't match what's implied by the name of the option, it's description or the intent of the original PR.

Thanks for your time and for all the wonderful metosin libraries 😄

[opqdonut]

I'm in favour of merging this. Needs a clear note in the changelog though.

[ikitommi]

Hi. Agree that the change is good as it doesn't break the existing contract, e.g. the old test:

(let [data-with-nils {:hello "world" :goodbye nil}]
  (is (= "{\"hello\":\"world\"}" (j/write-value-as-string data-with-nils (j/object-mapper {:strip-nils true})))))

still holds. The tests define expectations for the library. If the old test would have failed after this change, this would be a MAJOR breaking change (e.g. silent change of behavior) and it would have warranted 1.0.0.

my proposal:

1. keep the old assertion too to show it didn't change
2. release it (maybe with new :strip-emptys if someone wants that?)

[ikitommi]

btw, Jsonista doesn't conform to BREAKVER like other libs do, maybe it should like other libs? https://github.com/metosin/malli/blob/master/CHANGELOG.md#malli-changelog

[dominicfreeston]

keep the old assertion too to show it didn't change

That makes sense, I've brought back the original assertion.

[dominicfreeston]

Thanks for getting this in!