Automattic / Automattic/mongoose

`clone()` not cloning objects with custom class names

Open
#12,574 0 comments 0 reactions 0 assignees View on GitHub
note to self
Dominant language
JavaScript
Stars
27.5k
Forks
4k
Avg merge
2d 7h
Merged PRs (30d)
35

Description

### Prerequisites

- [X] I have written a descriptive issue title
- [X] I have searched existing issues to ensure the bug has not already been reported

### Mongoose version

6.6.x

### Node.js version

Any

### MongoDB server version

Any

### Typescript version (if applicable)

_No response_

### Description

Take a look at this line: https://github.com/Automattic/mongoose/blob/1ef7bcda4d16a253b80181253b898fadf155e1f8/lib/helpers/clone.js#L107-L109

All objects have a `valueOf()` function that return `this` by default:

```
$ nodely
Welcome to Node.js v16.17.0.
Type ".help" for more information.
> {}.valueOf
[Function: valueOf]
>
```

So `cloneObject()` never gets called. This unfortunately means that `clone()` doesn't actually clone any non-POJO objects.

Fortunately, this bug doesn't affect much because we mostly use `clone()` for POJOs. Unfortunately, this bug is very old (dates back to this commit from Jan 2011 https://github.com/Automattic/mongoose/commit/c62bb1764905b24985a5c668b83251b1a006ff94), and fixing it blows up our test suite with a bunch of stack overflows. But this is definitely worth fixing because `clone(obj) === obj` is not very intuitive.

Popped up when working on #12549

### Steps to Reproduce

```javascript
const mongoose = require('mongoose');
const clone = require('mongoose/lib/helpers/clone');

class MyClass {}
const obj = new MyClass();

console.log(obj === clone(obj)); // true?
```

### Expected Behavior

_No response_

Contributor guide

Open the contributing guide

Research direction

Start in lib/helpers/clone.js at lines 107-109 and reproduce the issue with the MyClass example from the report. Inspect the existing clone-related test suite, including the stack overflows noted in the issue. Done means custom-class instances are cloned rather than returned unchanged, without introducing those recursion failures.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript, nodejs
Domain
backend
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.