fastify / deepmerge

Merges the enumerable properties of two or more objects deeply. Fastest implementation of deepmerge
Other
80 stars 11 forks source link

`Buffer` is merged like `Array` #10

Closed mikaelkaron closed 1 year ago

mikaelkaron commented 2 years ago

Prerequisites

Fastify version

4

Plugin version

1.1.0

Node.js version

16.17.0

Operating system

Linux

Operating system version (i.e. 20.04, 11.3, 10)

20.04

Description

Since Buffer is in essence just a Uint8Array it gets mangled by deepmerge.

Steps to Reproduce

import { deepmerge } from '@fastify/deepmerge'
const merge = deepmerge()
const merged = JSON.stringify(merge({}, { buffer: Buffer.of(1, 2, 3) })) // {"buffer":{"0":1,"1":2,"2":3}}

Expected Behavior

import { deepmerge } from '@fastify/deepmerge'
const merge = deepmerge()
const merged = JSON.stringify(merge({}, { buffer: Buffer.of(1, 2, 3) })) // {"buffer":{"type":"Buffer","data":[1,2,3]}}
Uzlopak commented 2 years ago

Good catch. But the thing is, that we should add an option how buffers should be merged.

I mean, you could actually provide a custom array merger function which checks for buffer and returns a ref or a new Buffer. But actually not merging buffers but replacing buffers makes more sense.

mikaelkaron commented 2 years ago

All for that. One could also be allowed to provide isMergeableObject and similar functions as arguments to deepmerge to control this on a case by case basis.

Uzlopak commented 2 years ago

Adding an option for a custom isMergeableObject should be small patch.

But still, you are right regarding the Buffer. Buffer is a builtIn, so I would assume that we should take special care of it.