search

TehShrike / deepmerge

A library for deep (recursive) merging of Javascript objects
MIT License
2.75k stars 216 forks source link

v5 dependencies #214

Closed RebeccaStevens closed 3 years ago

RebeccaStevens commented 3 years ago

I noticed in the current v5 branch "is-plain-obj" is being used for is "isMergeable" by default. But "is-plain-obj" isn't listed as a dependency, only as a devDependency; and it's being bundled into the build.

I think it would be best to have this library as an actual dependency or even a peer dependency and not bundle it. Note: If it was included as a peer dependency, the install instruction for deepmerge would need to change to npm install deepmerge is-plain-obj.

But as "is-plain-obj" is such a small library that is unlikely to undergo many major changes, bundling it might be fine. Though I'd recommend using a tool like dependabot to help ensure it doesn't get out of date.

RebeccaStevens commented 3 years ago

Another question:

"is-mergeable-object" doesn't seem to be used anymore except by one of test files. I assume that used to be the main logic of "isMergeable". Should this be removed now as a dev dependency?

TehShrike commented 3 years ago

The bundling is mostly happening to support this feature:

deepmerge can be used directly in the browser without the use of package managers/bundlers as well: UMD version from unpkg.com.

I'm not exactly hardcore about always bundling every package, but for a broadly-used small package like this, it seems like the path of least annoyance.

is-mergeable-object can probably be removed – I would have guessed that it was referenced in one of the examples in the readme (which are evaluated using jsmd), but apparently not ¯_(ツ)_/¯