Object.assign instead of manipulating input objects

[SamVerschueren]

I myself did it wrong in the early days, and I definitely come across this issue quite a lot.

module.exports = opts => {
    opts.foo = opts.foo || 'bar';
};

While it should've been

module.exports = opts => {
    opts = Object.assign({
        foo: 'bar'
    }, opts);
};

Not sure how easy it would be to detect this though.

[sindresorhus]

Slightly relevant issue: https://github.com/sindresorhus/eslint-config-xo/issues/18

[sindresorhus]

Seems like it should be possible to detect the arguments and to find the cases where something is assigning to properties of those arguments. Could maybe use code path analysis.

Should use object spread and not Object.assign().

[MrHen]

Seems like the better fix is to make sure that objects in parameters are treated as immutable. The implementation for immutable/no-mutation might work as a starting point.

// objects
function foo(bar) {
    delete bar.zaz; // fail
    bar.zaz = bar.zaz || 'zaz'; // fail
    bar = {
        zaz: 'zaz',
        ...bar
    }; // pass
}

// arrays
function foo(bar) {
    bar.push('zaz'); // fail
    bar.pop() // fail
    bar.unshift('zaz'); // fail
    bar.shift(); // fail
    bar.length = 0; // fail
    delete bar[0]; // fail
    bar = {
        ...bar,
        'zaz'
    }; // pass
}

I'd argue that overshadowing parameter is also bad but should be a different rule.

[MrHen]

Turns out what I was mentioning is already a rule: https://eslint.org/docs/rules/no-param-reassign

There is also a functional difference between foo || bar and Object.assign. If you set a property to null and want a default then you can use || but not Object.assign.

[sindresorhus]

There is also a functional difference between foo || bar and Object.assign. If you set a property to null and want a default then you can use || but not Object.assign.

I've been using Object.assign for options arguments for years and that has never been a problem in practice.

[MrHen]

It's not really a problem. It's just a slight functional difference. In my opinion, I think the Object.assign pattern is more clear / less surprising anyway.

In any case, I was just pondering whether I wanted to help out with this feature -- but I'd rather have immutable parameters and/or use destructuring directly in the function signature. I'll go dig up something else to poke at.

[fregante]

Duplicate of #29

It's being fixed #632, but using default parameters instead of Object.assign