Skip to content
This repository has been archived by the owner on Mar 8, 2023. It is now read-only.

fix(loader): use isPlainObject when merging. #68

Merged
merged 1 commit into from
Apr 29, 2020

Conversation

lholmquist
Copy link
Contributor

Earlier this week, one of my modules, the openshift-rest-client, which is built on top of the kubernetes-client, started to break when uploading a binary file. Basically, we create a readStream from a local file using fs.createReadStream which gets added to the request body

The error was happening in the Request module, at the point where it looks to set the ContentLength, which is should skip over if the body is a stream. Code link for reference: https://github.com/request/request/blob/3c0cddc7c8eb60b470e9519da85896ed7ee0081e/request.js#L441

The problem started happening when we upgraded to [email protected], and after some digging i noticed that the merge libraries where swapped. Since deepmerge by default clones every object property, it was "stripping" the fact that the object i passed in was a ReadStream

according to the docs, https://www.npmjs.com/package/deepmerge#ismergeableobject , that can be fixed using the "isPlainObject" module when doing merges.

This PR adds the isPlainObject module and also adds that third parameter to the merge function calls.

I create a repo where i was basically simulating what was happening to show the difference: https://github.com/lholmquist/merge-deep-merge-fu

Copy link
Owner

@silasbw silasbw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix. Sorry about that.

@silasbw silasbw merged commit cc2c177 into silasbw:master Apr 29, 2020
@lholmquist lholmquist deleted the deepmerge-is-plain-object branch April 29, 2020 17:13
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants