Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Changed

- Replaced name-based index parameter tracking with scope-based resolution, so identifiers that shadow an index parameter are no longer misreported.

### Added

- Detect `Number(index)` coercion, in addition to `String(index)` and `index.toString()`.
- Detect index usage in `??`, `||`, and ternary fallback branches of a `key` value (e.g. `key={item.id ?? index}`).
- Detect string-literal `'key'` properties in `createElement`/`cloneElement` props objects.

## [5.6.0] - 2026-04-29

### Fixed
Expand Down
123 changes: 64 additions & 59 deletions plugins/eslint-plugin-react-x/src/rules/no-array-index-key/lib.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import { Extract, isOneOf } from "@eslint-react/ast";
import { Check, Extract, isOneOf } from "@eslint-react/ast";
import * as core from "@eslint-react/core";
import type { RuleContext } from "@eslint-react/eslint";
import { DefinitionType } from "@typescript-eslint/scope-manager";
import { AST_NODE_TYPES as AST, type TSESTree } from "@typescript-eslint/types";
import { findVariable } from "@typescript-eslint/utils/ast-utils";
import type { ReportDescriptor } from "@typescript-eslint/utils/ts-eslint";

/**
Expand All @@ -16,68 +18,71 @@ export function report(context: RuleContext) {
};
}

export function getIndexParamPosition(methodName: string) {
switch (methodName) {
case "every":
case "filter":
case "find":
case "findIndex":
case "findLast":
case "findLastIndex":
case "flatMap":
case "forEach":
case "map":
case "some":
return 1;
case "reduce":
case "reduceRight":
return 2;
default:
return -1;
}
}
/**
* Iterator-like methods that pass the item's index to their callback,
* mapped to the position of the index parameter in the callback's parameter list.
* `map` and `forEach` also cover `Children.map` and `Children.forEach`,
* whose callback is the second argument instead of the first.
*/
const INDEX_PARAM_POSITIONS = new Map<string, number>([
["every", 1],
["filter", 1],
["find", 1],
["findIndex", 1],
["findLast", 1],
["findLastIndex", 1],
["flatMap", 1],
["forEach", 1],
["map", 1],
["reduce", 2],
["reduceRight", 2],
["some", 1],
]);

// Gets the name of the index parameter from a map-like function's callback
// e.g., in `data.map((item, index) => ...)` it returns 'index'
export function getMapIndexParamName(context: RuleContext, node: TSESTree.CallExpression): string | null {
const callee = Extract.unwrap(node.callee);
if (callee.type !== AST.MemberExpression) {
return null;
}
if (callee.property.type !== AST.Identifier) {
return null;
}
const { name } = callee.property;
// Determines the position of the index parameter for array methods like 'map', 'forEach', etc
const indexPosition = getIndexParamPosition(name);
if (indexPosition === -1) {
return null;
}
// The callback function is the first argument, or the second for `React.Children` methods
const callbackArg = node.arguments[
core.isChildrenMap(context, callee) || core.isChildrenForEach(context, callee)
? 1
: 0
];
if (callbackArg == null) {
return null;
}
if (!isOneOf([AST.ArrowFunctionExpression, AST.FunctionExpression])(callbackArg)) {
return null;
}
const { params } = callbackArg;
if (params.length < indexPosition + 1) {
return null;
/**
* Checks whether an identifier is a reference to the index parameter of an
* iterator-like method's callback (e.g. `i` in `items.map((item, i) => ...)`).
* @param context The ESLint rule context.
* @param node The identifier to check.
* @returns `true` if the identifier resolves to an array index parameter.
*/
export function isArrayIndexReference(context: RuleContext, node: TSESTree.Identifier): boolean {
const variable = findVariable(context.sourceCode.getScope(node), node);
const def = variable?.defs.at(0);
if (def == null || def.type !== DefinitionType.Parameter) return false;
const callback = def.node;
if (!isOneOf([AST.ArrowFunctionExpression, AST.FunctionExpression])(callback)) return false;
// The argument node as it appears in the call, including any TS type wrappers around the callback
let argument: TSESTree.Node = callback;
while (Check.isTypeExpression(argument.parent)) {
argument = argument.parent;
}
const param = params.at(indexPosition);

return param != null && "name" in param
? param.name
: null;
const call = argument.parent;
if (call.type !== AST.CallExpression) return false;
const callee = Extract.unwrap(call.callee);
if (callee.type !== AST.MemberExpression) return false;
if (callee.property.type !== AST.Identifier) return false;
const indexPosition = INDEX_PARAM_POSITIONS.get(callee.property.name);
if (indexPosition == null) return false;
// The callback is the first argument, or the second for `Children.map`/`Children.forEach`
const callbackPosition = core.isChildrenMap(context, callee) || core.isChildrenForEach(context, callee)
? 1
: 0;
if (call.arguments[callbackPosition] !== argument) return false;
// The resolved binding must be the parameter at the index position itself,
// possibly wrapped in a default value assignment (e.g. `(item, i = 0) => ...`)
const param = callback.params[indexPosition];
if (param == null) return false;
return param === def.name
|| (param.type === AST.AssignmentPattern && param.left === def.name);
}

// Recursively collects all identifiers from a binary expression
// e.g., for `a + b + c`, it returns identifiers for a, b, and c
/**
* Recursively collects all identifiers from a binary expression.
* e.g., for `a + b + c`, it returns identifiers for a, b, and c.
* @param side The binary expression (or one of its sides) to collect from.
* @returns The identifiers found in the expression.
*/
export function getIdentifiersFromBinaryExpression(
side: TSESTree.BinaryExpression["left"],
): readonly TSESTree.Identifier[] {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,88 @@ function MyComponent({ items }: MyComponentProps) {
}
```

### Deriving the key from the index

Wrapping the index in a string or a conversion does not make it a stable identity — the key still changes whenever the list is reordered. The rule also reports indexes used in template literals, string concatenation, and `String()`, `Number()`, or `.toString()` conversions.

```tsx
// Problem: The key is still derived from the array index
interface MyComponentProps {
items: { id: string; name: string }[];
}

function MyComponent({ items }: MyComponentProps) {
return (
<ul>
{items.map((item, index) => (
// ^^^ Do not use an item's index in the array as its key.
<li key={`item-${index}`}>{item.name}</li>
))}
{items.map((item, index) => (
// ^^^ Do not use an item's index in the array as its key.
<li key={String(index)}>{item.name}</li>
))}
</ul>
);
}
```

```tsx
// Recommended: Derive the key from the data itself
interface MyComponentProps {
items: { id: string; name: string }[];
}

function MyComponent({ items }: MyComponentProps) {
return (
<ul>
{items.map((item) => <li key={`item-${item.id}`}>{item.name}</li>)}
</ul>
);
}
```

### Falling back to the index

Using the index as a fallback when an identifier is missing still gives some items an unstable identity. The rule checks every branch a key may evaluate to, including `??`, `||`, and ternary expressions.

```tsx
// Problem: Items without an id silently fall back to the index
interface MyComponentProps {
items: { id?: string; name: string }[];
}

function MyComponent({ items }: MyComponentProps) {
return (
<ul>
{items.map((item, index) => (
// ^^^ Do not use an item's index in the array as its key.
<li key={item.id ?? index}>{item.name}</li>
))}
</ul>
);
}
```

```tsx
// Recommended: Assign a stable id when the item is created, not at render time
interface MyComponentProps {
items: { id: string; name: string }[];
}

function createItem(name: string) {
return { id: crypto.randomUUID(), name };
}

function MyComponent({ items }: MyComponentProps) {
return (
<ul>
{items.map((item) => <li key={item.id}>{item.name}</li>)}
</ul>
);
}
```

### Rendering nested lists

When rendering nested lists, both the outer and inner items need stable keys. Using the item's own identifier ensures React can correctly track elements at every level.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,41 @@ ruleTester.run(RULE_NAME, rule, {
code: tsx`foo.map((value, index) => <Foo key={(String as any)(index)} />)`,
errors: [{ messageId: "default" }],
},
{
code: tsx`foo.map((bar, i) => <Foo key={Number(i)} />)`,
errors: [{ messageId: "default" }],
},
{
code: tsx`foo.map((bar, i) => <Foo key={bar.id ?? i} />)`,
errors: [{ messageId: "default" }],
},
{
code: tsx`foo.map((bar, i) => <Foo key={bar.id != null ? bar.id : i} />)`,
errors: [{ messageId: "default" }],
},
{
code: tsx`foo.map((bar, i = 0) => <Foo key={i} />)`,
errors: [{ messageId: "default" }],
},
{
code: tsx`foo.map((bar, i) => bar.items.map((baz) => <Foo key={i} />))`,
errors: [{ messageId: "default" }],
},
{
// The identifier resolves to the nearest binding: the inner map's index
code: tsx`foo.map((bar, i) => bar.items.map((item, i) => <Foo key={i} />))`,
errors: [{ messageId: "default" }],
},
{
// Recursion reaches the index through chained logical branches
code: tsx`foo.map((bar, i) => <Foo key={bar.id ?? bar.name ?? i} />)`,
errors: [{ messageId: "default" }],
},
{
// String-literal 'key' property is treated like the identifier form
code: tsx`foo.map((bar, i) => React.createElement('Foo', { 'key': i }))`,
errors: [{ messageId: "default" }],
},
],
valid: [
// https://github.com/oxc-project/oxc/issues/21110
Expand Down Expand Up @@ -303,5 +338,43 @@ ruleTester.run(RULE_NAME, rule, {
);
}
`,
// The identifier shadows the index parameter with a different value
tsx`
foo.map((bar, i) => {
return bar.items.map((item) => {
const i = item.id;
return <Foo key={i} />;
});
})
`,
// The identifier is not an index parameter of an iterator-like callback
tsx`
const i = getId();
foo.map((bar) => <Foo key={i} />);
`,
tsx`foo.customMap((bar, i) => <Foo key={i} />)`,
// The 'key' is derived from item data, not the array index
tsx`foo.map((bar, i) => <Foo key={bar.id} />)`,
tsx`foo.map((bar, i) => <Foo key={bar.id || bar.name} />)`,
// Only the parameter at the index position counts, not other callback params
tsx`foo.map((bar, i) => <Foo key={bar} />)`,
tsx`foo.reduce((acc, bar, i) => acc.concat(<Foo key={bar} />), [])`,
// The function must be at the callback position of the call
tsx`foo.map(notCallback, (bar, i) => <Foo key={i} />)`,
// Only inline callbacks are tracked, not function references
tsx`
function renderItem(bar, i) {
return <Foo key={i} />;
}
foo.map(renderItem);
`,
// The index in a ternary test is not part of the key's value
tsx`foo.map((bar, i) => <Foo key={i > 0 ? bar.id : bar.name} />)`,
// Only direct template interpolations are checked, not nested expressions
tsx`foo.map((bar, i) => <Foo key={\`\${'foo' + i}\`} />)`,
// Only 'toString', 'String' and 'Number' conversions are tracked
tsx`foo.map((bar, i) => <Foo key={i.toFixed()} />)`,
// Computed 'key' properties are not checked
tsx`foo.map((bar, i) => React.createElement('Foo', { ['key']: i }))`,
],
});
Loading
Loading