Skip to content

[minor] Relax the no-param-reassign warning #964

@monfera

Description

@monfera

Currently, a stringent version is active. Ie. it doesn't just complain on parameter reassignment. It also complains if you want to set some inner property of what gets passed as a parameter. Which is the intent, and often used, eg. when building tree or generally recursive structures, indices etc. Example

The no-param-reassign is a clear misnomer in this case. The word parameter has precise meaning. In this case, it's the binding of the argument object to the name mapNode. The parameter is mapNode. That line does not reassign the parameter.

Would be great to deactivate it for the property update case and leave it in place for the reassignment of the binding eg. const foo = x => { x++ } though even such use isn't a huge deal in a library (popular libraries sometimes have if(!Array.isArray(arg2)) arg2 = [arg2] or some such)

It's shades of gray and subjective: my feel is that leaning on the stricter side with warnings and errors is great practice for web apps but feels less useful for library code. Read support for this kind of lib vs app distinction, maybe I'll run into it again. Eg. is the above referred code line in group_by_rollup problematic in data processing code that warrants a noise line? I'm OK if we are explicit about our goals and state that we're inching toward immutability; in this case we could chat about the goal, then put the rule in to help move toward the goal. The linter rule alone won't help much if this is the goal. Also, immutability has a price.

It also feels off to see a linter warning and not do something about it. It's nice to see the gutter with no discolorations, so I'll prefix that line with // eslint-disable-next-line no-param-reassign which is better than leaving in a warning (even if I currently disagree with the necessity for the warning for the stricter interpretation).

Some of the most popular libraries that do nontrivial things don't enforce a large fraction of the rules just because they are around. Often for some good reason, and they don't typically excuse them with noisy // eslint-disable-next-line no-param-reassign lines. Would be interested in how y'all see the webapp vs library use case, and why there should, or should not be a distinction.

Metadata

Metadata

Assignees

No one assigned

    Labels

    discussTo be discussed

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions