Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cloning a class that extends Map, e.g. a "DefaultMap", returns a Map instead #87

Open
laurence-myers opened this issue Nov 25, 2017 · 5 comments

Comments

@laurence-myers
Copy link

laurence-myers commented Nov 25, 2017

I have a custom class that extends from Map. This class is "DefaultMap", and provides a default value when an item is not found in the Map.

It looks like instances of "DefaultMap" are getting cloned as just "Map". It then loses the custom functionality provided by "DefaultMap".

Here's a test case in Mocha to reproduce the problem:

const assert = require('assert');
const clone = require('clone');

describe(`clone`, function () {
    it(`can deep copy a class that extends map`, function () {
        let overridingMethodWasCalled = false;
        class DefaultMap extends Map {
            constructor(defaultValueFactory, iterable) {
                super(iterable);
                this.defaultValueFactory = defaultValueFactory;
            }

            get(key) {
                overridingMethodWasCalled = true;
                if (!this.has(key)) {
                    const value = this.defaultValueFactory(key, this);
                    this.set(key, value);
                    return value;
                } else {
                    return super.get(key);
                }
            }
        }

        const defaultValue = 'placeholder';
        const map = new DefaultMap(() => defaultValue);
        const obj = {
            myMap: map
        };

        const copiedObj = clone(obj);
        console.log(obj, copiedObj);
        assert.notStrictEqual(obj.myMap, copiedObj.myMap, "Properties holding Maps should not reference the same object in memory");
        assert.strictEqual(obj.myMap.size, copiedObj.myMap.size, "The cloned Map should have the same size as the original Map");

        const value = copiedObj.myMap.get('nonExistentKey');
        assert.ok(overridingMethodWasCalled, "The overriding method on DefaultMap should be called");
        assert.strictEqual(value, defaultValue, "The cloned Map should be a DefaultMap, and should return the default value when given a non-existent key");
        assert.notStrictEqual(obj.myMap.size, copiedObj.myMap.size, "After implicitly inserting an element, the cloned Map should have a different size to the original Map");
    });
});

The test fails at the assertion "The overriding method on DefaultMap should be called".

Tested in clone v2.1.1, Node v6.9.1.

@Holger-Will
Copy link

i came across this issue just now. node-red uses clone to clone messages. I ran into this issue because I send custom message objects that extend Arrays.
Here is a more minimal testcase:

var clone = require("clone")

class MyArray extends Array{}
class MyThing {
    constructor(){
        this.arr = new MyArray()
    }
}
var thing = new MyThing()

var thing2 = clone(thing)

console.log(thing.arr.constructor.name)
console.log(thing2.arr.constructor.name)

@freedomhero
Copy link

Running into the same problem, any updates?

@zhfnjust
Copy link

same problem, can you fix it

@laurence-myers
Copy link
Author

If you don't need to use this specific library, I have found that lodash's cloneDeep() works best. It supports classes that inherit from Map.

https://lodash.com/docs/#cloneDeep

I wrote a comparison of some JS (NodeJS) cloning libraries back in 2018, here's one view of the results: https://laurence-myers.github.io/clone-comparison-nodejs/index.html#section-feature-table

@freedomhero
Copy link

@laurence-myers I will give it a try, thx any way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants