fix: replace merge() with Object.assign()

[KATT]

Background

In my tests - I often spin up servers as part of beforeEach. In the particular area where I encountered problems is that my test server kept track of incoming requests by a mutable array. Couldn't for the life of me understand why the array was constantly empty, but I think this was the culprit.

To be even more specific, I was testing a websocket trigger/receiver - so I in my tests I dynamically use http.createServer() & later I want to assert that the server received events as after doing specific actions in my app.

TODO

• [ ] docs behaviour was not docced before as far as I can tell
• [ ] tests seem to have been unchanged as well

[jasonkuhrt]

This will do a shallow merge, we should actually add tests and docs about how merging works.

We definitely want a deep merge.

But maybe we do have something not desirable about the lodash algo. Can you investigate an isolated example of how the merge is not doing what you want/expect?

[KATT]

What is the benefit of a deep merge? Will reference what I am doing as soon as the PR working in general. Will probably send it over tomorrow.

I want some of my values in my context mutable from within itself. But maybe there's an error with my code.

[jasonkuhrt]

It is pretty case dependent. Happy to look at your case though. Maybe we need a toggle to enable or disable deep merge e.g.:

import { konn } from 'konn'

konn.settings({ deepMerge: false })

Note that this has an impact on the static types. The static types currently do a deep merge. The only way to to get the static typing change from this runtime setting change would be either some pretty complex typegen stuff or, more simply, via keeping the settings change in the chain. Reuse can be achieved like so:

import { konn } from 'konn'

export const mykonn = konn.settings({...})

Now all code needs to import and use mykonn instead of konn.

Providers API doesn't need to worry about this I think.

Making static types not do a deep merge will take some additional effort in this PR.

[KATT]

I would expect it to simply blow up if the keys collide as there's no good way of approaching it - both merging and overriding have drawbacks - I'd treat it as an implementation error.

In tRPC I have a bunch of similar cases and the way I deal with it is that I make the server explode on bootup if you accidentally achieve "impossible" states (duplicate queries/multiple data transformers/[..]).

[jasonkuhrt]

@KATT Strict mode sounds nice, I see a union instead of boolean then:

konn.settings({ merge: 'error' | 'deep' | 'shallow' })

[KATT]

If you don't actually utilize the behavior of being able to deep merge props in any of your apps already, I think you should just delete it. Always great to opt for keeping the API surface area slim & keeping deps to a minimum when making libs.

[jasonkuhrt]

FYI: I [re-]discovered that Lodash merge does not merge arrays.

[KATT]

Here is the thing I can't do within the context of the current behavior

1. Create a server dynamically <- I want this in a beforeEach
2. Wait for it to receive a response

webhookReceiver.requestList.length was always 0 when I had this in a beforeEach which was super unintuitive and I thought it was my own code that was broken for half an hour or so. Something in the _.merge() seem to duplicate the array instead of referencing it, which makes this broken.

I might be wrong though - will do another test.

[KATT]

https://github.com/calendso/calendso/pull/975

Here you can see a clear example of this failing in the first commit and works when I revert it

[jasonkuhrt]

@KATT you might be interested in the child process provider https://github.com/prisma-labs/konn#standard-providers