Skip to content

[feat] immutability: detect direct property mutation of props/state (including via shallow copies), not just closures reaching freeze sinks #1941

Description

@rakleed

Problem Description

@eslint-react/immutability's label and description ("Validates against mutating props, state, and other values that are immutable") read as if it covers general "don't mutate props/state" violations — the same territory as eslint-plugin-react-hooks's react-hooks/immutability rule it's modeled on. In practice, the implementation only detects one specific shape: a function that mutates a captured value, where that function itself is passed to a "freeze sink" (a JSX prop, a hook argument, or a hook return). I confirmed this by running the rule directly (@eslint-react/eslint-plugin@5.18.6, rule force-enabled since it's all-only):

// ✅ correctly flagged — mutating function passed to useEffect (a freeze sink)
function Example({ items }) {
  const mutate = () => { items.push(1); };
  useEffect(mutate, []);
}
5:5   error  This modifies 'items'
8:13  error  This function may (indirectly) reassign or modify 'items' after render...

But the much more common real-world violation — directly mutating a prop/state value, with no closure or sink involved — is never flagged, whether the mutation is direct or reached through a shallow copy:

// ❌ NOT flagged — direct mutation, no closure/sink involved
function Example({ initial }) {
  const [values, setValues] = useState(initial);
  const handleChange = (itemId, diff) => {
    values[itemId].confirmedQuantity = diff; // mutates state directly
    setValues(values);
  };
}

// ❌ NOT flagged either — mutation reached through a shallow copy
function Example({ initial }) {
  const [values, setValues] = useState(initial);
  const handleChange = (itemId, diff) => {
    const copyValues = { ...values };            // shallow copy — nested objects still shared
    copyValues[itemId].confirmedQuantity = diff;  // copyValues[itemId] === values[itemId]
    setValues(copyValues);
  };
}

This is exactly the shape that got a colleague and me tripped up in a real component: we had values[itemId] = ... correctly flagged (by the upstream react-hooks/immutability rule, not this one — @eslint-react/immutability isn't even in recommended/strict, only in all), "fixed" it with const copy = { ...values }, and the fix looked idiomatic — but the mutation, and the underlying rendering bug, were still there because the copy is shallow.

Alternative Solutions

  • The upstream eslint-plugin-react-hooks's react-hooks/immutability rule does catch the direct-mutation case above, but has the identical blind spot for the shallow-copy case — reported upstream as facebook/react#37316 (well, filed against react/react). That fix has to land in the compiler's shared mutation-validation pass (ValidateNoFreezingKnownMutableFunctions/HIR-level analysis) that both the Babel plugin and the ESLint rule consume, so realistically it'll take a while to land and reach a release even if accepted — it's core, heavily-reviewed infrastructure with a much wider blast radius than a single ESLint rule. eslint-react maintains its own independent AST-based approximation of the same check, so it isn't blocked on that upstream process and could close this specific gap on its own timeline — which is the main reason I'm proposing it here rather than just waiting on the upstream report.
  • There's no other rule in @eslint-react/eslint-plugin that covers plain state.foo = x / props.foo = x mutation outside of a freeze-sink context, as far as I could find searching the rule list and existing issues.
  • Enabling only all isn't a real workaround, since (per my testing) @eslint-react/immutability still doesn't fire on either mutation shape above even when force-enabled — the gap isn't about the preset, it's the detection model itself.

Feature Details

Feature name: extend @eslint-react/immutability's mutation-recognition to also flag direct mutation of a value that provably originates from props/state, independent of whether that mutation happens inside a function that later reaches a freeze sink. Concretely:

  1. Treat a member-assignment/update/delete (x.foo = v, x[0]++, delete x.foo) as reportable when x's root identifier resolves (via the same initializer-alias tracing the rule already does) to a prop, a useState/useReducer value, or similar.
  2. Extend that same root-identifier resolution one step further through object/array literals built from a spread of a props/state value (const copy = { ...state } / const copy = [...state]) — copy's own top-level slots are new, but anything reached by indexing/property access into copy and then mutating a nested member is still the original reference, and should be treated the same as mutating state directly.

If bundling this with immutability would muddy its documented scope (it's explicitly modeled on the compiler's ValidateNoFreezingKnownMutableFunctions, a narrower check), a separate rule (e.g. no-direct-state-mutation / no-direct-props-mutation) covering just points 1–2 above would be just as useful, and arguably clearer to reason about than overloading immutability.

Examples

function Example({ initial }) {
  const [values, setValues] = useState(initial);

  const handleChange = (itemId, diff) => {
    values[itemId].confirmedQuantity = diff; // (1) should be flagged: direct state mutation
    setValues(values);
  };

  const handleChangeViaCopy = (itemId, diff) => {
    const copyValues = { ...values };
    copyValues[itemId].confirmedQuantity = diff; // (2) should be flagged: copyValues[itemId] === values[itemId]
    setValues(copyValues);
  };
}

Evaluation Checklist

  • I have had problems that this feature would solve
  • I could not find a way to solve the problem with existing features or workarounds
  • I have thought very hard about potential edge cases and downsides, and they are acceptable
  • I think the feature is well-defined and would provide clear value to users

Metadata

Metadata

Assignees

No one assigned

    Labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions