CODEWITHSUNDEEP
javascript
yqlim
yqlim

Reputation: 7080

Javascript pass element as value

I am learning to build a Javascript constructor and met the following problem:

var x = new Constructor({
        B: '1000',
        child: document.getElementById('id'), // assume there is a div#id
    });

function Constructor(options) {
    var settings;

    this.defaults = {
        A: 'value A',
        B: 2,
        parent: document.body,
        child: document.getElementById('anotherId'), // assume there is a div#anotherId
    }

    settings = extend({}, this.defaults, options);

    console.log(this.defaults.A)      // outputs 'value A'
    console.log(this.defaults.B)      // outputs 2
    console.log(this.defaults.parent) // outputs <body>...</body>
    console.log(this.defaults.child)  // outputs <div id="id">...</div>

    console.log(settings.A)           // outputs 'value A'
    console.log(settings.B)           // outputs '1000'
    console.log(settings.parent)      // outputs empty object
    console.log(settings.child)       // outputs empty object
}

function extend(out) {
    out = out || {};
    for (var i = 1; i < arguments.length; i++) {
        var obj = arguments[i];

        if (!obj) continue;

        for (var key in obj) {
            if (obj.hasOwnProperty(key)) {
                out[key] = (typeof obj[key] === 'object') ? extend(out[key], obj[key])
                                                          : obj[key];
            }
        }
    }
    return out;
}

The problem is settings.parent doesn't have the same value as this.defaults.parent, but the other properties are just fine.

What could be the problem here?

Upvotes: 2

Views: 1042

Answers (1)

Matt Browne
Matt Browne

Reputation: 12419

The problem is the extend() function - it apparently wasn't written to support copying DOM nodes like document.body. I recommend using Object.assign() instead - it's a native function available in ES6 and there are shims available for older browsers (such as this one on MDN).

Usage:

settings = Object.assign({}, this.defaults, options);

Note that this only makes a shallow copy, so might not work in some situations. If you are copying nested objects and you want to be sure that you're not modifying the original object in this.defaults or options, then you might need to do some deep copying / cloning.

I'm not sure where you got that extend() function, but it does do some basic deep copying. The problem is that DOM nodes can't be cloned in the same way as regular objects. There is a way to clone them - you call the clone() method - but the purpose of that is to create an actual copy of the DOM node that you can then add somewhere else in the document, which isn't what you want to do here.

Are you using any DOM library such as jQuery? If so you could do this and avoid the whole problem:

this.defaults = {
    A: 'value A',
    B: 2,
    parent: $('body'),
    child: $('#anotherId'), // assume there is a div#anotherId
}

This would work because jQuery objects are regular JS object that wrap one or more DOM nodes.

UPDATE: Actually for this to work you'd still need to modify the extend() function or use a different cloning function, because there's another bug in extend(). See "UPDATE" below.

Alternatively you could alter your extend() method so it only copies references to DOM nodes but deep-copies all other kinds of objects. Here's how you could do that:

function extend(out){
    out = out || {};
    for (var i = 1; i < arguments.length; i++){
        var obj = arguments[i];
        if (!obj) continue;
        for (var key in obj){
            if (obj.hasOwnProperty(key)) {
                if (typeof obj[key] === 'object') {
                    //don't deep-copy DOM nodes
                    if (obj.nodeType && obj.nodeName && obj.cloneNode) {
                        out[key] = obj[key];
                    }
                    else out[key] = extend(out[key], obj[key]);
                }
                else out[key] = obj[key];
            }
        }
    }
    return out;
}

However, I think that's a somewhat unusual solution; personally if I encountered a function called extend() I'd expect it to either deep-copy or shallow-copy without special exceptions. So I'd recommend using jQuery objects or some similar wrapper object rather than the DOM nodes themselves.

UPDATE

As mentioned above, the extend() method has another issue that causes it not to clone jQuery objects properly. The issue is that it doesn't preserve the object's prototype, but rather just starts with an empty object {}. You could fix that by using Object.create(), but it still would take extra work to fix some other issues with it (see comments below). So I recommend just using an existing 3rd party cloning function instead.

For example, you could use the cloneDeepWith function from lodash:

settings = _.cloneDeepWith(this.defaults, options);

(Note: Apparently lodash just copies a reference for DOM nodes and deep-clones everything else, so in that respect it works like the modified version of extend() I wrote above. So it might be a convenient option for you.)

Or jQuery's extend() method using the deep option:

settings = $.extend(true /* setting the first argument to true makes it deep copy */,
    {}, this.defaults, options);

I also wrote my own deepCopy() function which is quite robust...it's party of my simpleOO library which will become irrelevant over time now that Javascript has classes, but here's the link to the documentation on the deepCopy method if you're interested:

https://github.com/mbrowne/simpleoo.js/wiki/Additional-Examples#deep-copying-cloning-objects

(My deepCopy function can also be used on its own; see the second example in my answer here: https://stackoverflow.com/a/13333781/560114.)

Upvotes: 2

Related Questions