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

no-param-reassign with props #626

Closed
trshafer opened this issue Dec 16, 2015 · 5 comments
Closed

no-param-reassign with props #626

trshafer opened this issue Dec 16, 2015 · 5 comments

Comments

@trshafer
Copy link
Contributor

Hey AirBnB,
What are your thoughts about preventing object manipulation on params as well:
http://eslint.org/docs/rules/no-param-reassign.html. I know I'm surprised when I pass an object to a function and the function manipulates the object.

The current value is set to 2. What do you think about changing it to [2, { "props": true }]?

Thanks!

@ljharb
Copy link
Collaborator

ljharb commented Dec 16, 2015

I think absolutely this should be turned on - I wasn't aware that was even an option.

A PR that updates the README, and updates the lint rule (including linking to the rule docs), would be very welcome.

@trshafer
Copy link
Contributor Author

Sweet. Will do it.

@trshafer
Copy link
Contributor Author

closed via #627

@EvanCarroll
Copy link

@ljharb @thomasjshafer This is silly. At least for the reason given because you're not preventing object manipulation with this. In fact, you're just obstructing it to silence the linter.

var o = {f:1};
var f = (o)=> {let a=o; a.g=2};
f(o);
console.log(o.g); // logs 2

This is going to print 2, and not generate any errors in the linter. If you're doing it for this reason, I'd highly suggest you reverse this patch.

@ljharb
Copy link
Collaborator

ljharb commented Feb 16, 2016

@EvanCarroll You are welcome to disable the rule in your own codebase if you don't like it. I commented a bit more on your eslint issue about why this is important.

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

3 participants