prisma-labs / konn

MIT License
89 stars 4 forks source link

fix: replace `merge()` with `Object.assign()` #24

Open KATT opened 3 years ago

KATT commented 3 years ago

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

jasonkuhrt commented 3 years ago

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

KATT commented 3 years ago

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 commented 3 years ago

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 commented 2 years ago

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