Nested null is not removed when merging into an empty value

[neil-marcellini]

Problem

The problem is described by @paultsimura here.

Solution

Investigate and fix it.

[paultsimura]

While working closer on the implementation and exploring the past issues, I think we're making a full circle here.

Seems like the deep null removal was introduced here: #353
But later, it was removed in #411 (see #411 (comment)).

Specifically the mergeCollection expects deep null values not to be removed, otherwise, we'll encounter this issue again: #408 (comment)

@janicduplessis @chrispader could you please take a look and share your thoughts here as you were involved in the current implementation of the null processing?

[neil-marcellini]

❗❗ Heads up, I'm going to be OOO working from Spain 🇪🇸 part time until 3/28. Most days I will be working 50%, some days 100%. Please DM me if something needs urgent attention.❗❗

[chrispader]

@paultsimura you're completely right, all of these tests (2 Onyx, 1 fastMerge) shouldn't be failing:

#301 (comment)
#301 (comment)

The nested nulls should be removed already at the level of the fastMerge function, because the shouldRemoveNullObjectValues parameter is true by default. I've created a PR to fix, simplify and further document the fastMerge implementation: #518

As far as i see it, the changes by @janicduplessis in #411 to not remove nested null values in cache don't affect this though.

[neil-marcellini]

Great thanks so much @chrispader! Now we just need an App PR to update the Onyx version right? What's the plan for that?

[paultsimura]

@neil-marcellini I believe the merged PR is a part of the solution – it shouldn't have affected the App yet. We should wait for #519 to bump the version.

Here's the tracking issue: #516

[chrispader]

Agree, i think we'll want to wait for #519 and potentially also #515 before we create a bump PR in E/App. Right now, these PRs are only affecting @paultsimura 's work on his PR, if i understand correctly...

[neil-marcellini]

Ok then I think we can call this done. Please lmk if it needs to be re-opened for some reason.