remove deep-assign dependency

[Simek]

Summary:

Fixes #374

This PR removes deprecated deep-assign package dependency and replaces its functionality with the simple native ES code.

Since there is only one place where this method is used and it is a simple usage I have decided to not implement target as spread argument.

I'm happy to improve/put more work into this PR if it's needed.

Test Plan:

From the several tests I have conducted for all of the cases the result of deep-assign and new merge function were exactly the same.

Snack playground:

• https://snack.expo.io/@simek/courageous-bubblegum

[krizzu]

Since this is our own implementation, could you export this function (and write a comment that it's being exported for tests only) and write some tests? To make sure this works as expected

[Simek]

I had started working on the requested changes but it looks like there is already an own, native implementation of deep merge in the code:

• https://github.com/react-native-community/async-storage/blob/master/jest/async-storage-mock.js#L99

Can this code be moved into main file? What is the best approach in this situation?

[krizzu]

Good catch, I forgot about that code there 😊 But this should not be a problem in that case - the _deepMerge function is used in Mock function. What I meant in my request was to write tests for merge function only, to check if it properly merges objects together.

I guess we could try to reuse merge in mock itself, but I'm not sure if that's good approach - the mock has no external (or even internal) dependencies, and I'd like to keep it that way

[Simek]

I'm a little bit afraid that I don't fully understand your test plan or your testing approach. I happy to help with fixing the initial issue and the basic tests but don't want to spend a lot of time working on the full test suite for this change.

So there are three options I consider:

• ditching both native implementations (in code and in mock) in favor of one of recommended replace packages (this would not require creating test suite, but this still will require avoidable dependency),
• leaving native merge as is and include/copy tests from the deep-merge package,
• closing this PR as is right now with an offer to help when someone decide to tackle this issue again in the future.

Let me know what's your opinion about that.

[krizzu]

I'm okay with changing the dependency to something else (more tested and used), than having own implementation, so that we save time on writing proper implementation and tests.

[krizzu]

Working with @Simek, we came to conclusion it's better to leave it as a dependency for now, due possible breaking changes (and how library is created).

[nicolasscard]

Any notice with this deprecated dependency ?

[tido64]

There's a new attempt here: #601