no-mutating-assign and functions

[jdiamond]

This doesn't pass the no-mutating-assign rule:

Object.assign(() => {}, {foo:1});

Should it be allowed?

[jfmengels]

Hi @jdiamond!

From what I see in the Object.assign documentation, the first parameter must be an object. Since functions are objects, this is valid, and I guess I could see some use-cases for it. We could allow (arrow) function expressions, since no variable is mutated.

Did you happen to notice it or do you have a use-case where this rule bothered you?

[jdiamond]

Hi @jfmengels,

Thanks for looking into it. I did discover this while working on "real" code so I think it's a valid use-case, but my problem is definitely self-imposed.

I'm working on a React app and am defining modules containing stateless functional components (just functions). Those normally look like this:

const MyComponent = ({ myProp }) => {
    return (
        <div>{myProp}</div>
    );
};

MyComponent.propTypes = {
    myProp: React.PropTypes.string,
};

export default MyComponent;

I have the "no-unused-expression" rule turned on which prevents me from mutating MyComponent with that assignment statement so I changed my code to this:

const propTypes = {
    myProp: React.PropTypes.string,
};

// eslint-disable-next-line fp/no-mutating-assign
const MyComponent = Object.assign(({ myProp }) => {
    return (
        <div>{myProp}</div>
    );
}, {
    propTypes
});

export default MyComponent;

This works in my app, but now I have to disable the "no-mutating-assign" rule to avoid the lint errors.

[jfmengels]

I just published v2.3.0 with this change. Let me know if that solves it for you :)

By the way, I don't see how the no-unused-expression rule prevents you from mutating MyComponent. Did you mean no-mutation? If so, you can configure the rule to allow the mutation of anything when setting the propTypes.

/* eslint fp/no-mutation: ["error", {"exceptions": [{"property": "propTypes"}]}] */
function Component(props) {
  // ...
}

Component.propTypes = {
  // ...
};

Your way is safer, but I'm mentioning in case you end up finding your current method too confusing for newcomers or hard to read. By the way, I had not thought of that method, that's a good one to try :)

[jdiamond]

Hey thanks! I upgraded and was able to remove my comments disabling the rule.

You're right that no-mutation is the error I get when trying to assign propTypes the old-fashioned way. I think I assumed no-unused-expression because that's the error I've been seeing the most lately as I try to write in a more functional style.

I didn't know about the exceptions property so thanks for that. I agree that passing an arrow function to Object.assign looks very funny. Just trying it out for now.

Thanks again for the awesome plugin!

[graingert]

I think this is probably acceptable for function literals as long as they are only accessible after mutation.

[graingert]

I can see:

const foo = Object.assign(function foo(ham) { }, { $inject: ['ham'] });

being preferred to:

const foo = function foo(ham) { };
foo.$inject = ['ham'];

[jdiamond]

IIRC, @jfmengels did make the change for your first example to work without triggering this error.