Skip to content

Controls: fix option label display when name matches Array built-in method #33908

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.

Sign up for GitHub

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

Closed
mixelburg wants to merge 4 commits into from
Closed

Controls: fix option label display when name matches Array built-in method #33908

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d147c16
Controls: fix option display when name matches Array built-in method
Feb 23, 2026
483692c
fix: guard against empty-string labels and add regression story for a…
mixelburg Feb 27, 2026
6ed1806
ci: trigger recheck
mixelburg Mar 4, 2026
adcae64
ci: trigger CI recheck
Mar 5, 2026
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
7 changes: 6 additions & 1 deletion code/addons/docs/src/blocks/controls/options/Options.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,12 @@ import { SelectControl } from './Select';
const normalizeOptions = (options: Options, labels?: Record<any, string>) => {
if (Array.isArray(options)) {
return options.reduce((acc, item) => {
acc[labels?.[item] || String(item)] = item;
const label = labels?.[item];
// Ensure the label is a string to avoid using non-string values (e.g., Array prototype
// methods) as object keys. This can happen when an option's name matches a built-in array
// method (e.g. 'reverse') and `labels` is inadvertently an array instead of a Record.
// See: https://github.com/storybookjs/storybook/issues/30142
acc[typeof label === 'string' && label !== '' ? label : String(item)] = item;
return acc;
}, {});
}
Expand Down
25 changes: 25 additions & 0 deletions code/addons/docs/src/blocks/controls/options/RadioOptions.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,3 +115,28 @@ export const ArrayReadonly: Story = {
},
},
};

// Regression test for https://github.com/storybookjs/storybook/issues/30142
// Options whose names match Array prototype methods (e.g. 'reverse') must be
// displayed as plain text, not as the stringified native function.
const optionsWithArrayMethodNames = ['normal', 'reverse', 'filter', 'map'];
export const ArrayWithBuiltinNames: Story = {
name: 'Array with option names matching built-in Array methods',
args: {
value: optionsWithArrayMethodNames[0],
argType: { options: optionsWithArrayMethodNames },
},
};
Comment on lines +119 to +129

@coderabbitai coderabbitai Bot Feb 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Story doesn't reproduce the actual bug condition

The bug is triggered only when labels is inadvertently an array — labels?.['reverse'] then resolves to Array.prototype.reverse (a truthy function). When labels is omitted (as it is here), both old and new code take the identical String(item) path, so this story would have passed even before the fix.

To make it a true regression guard, pass labels as an empty array (casting past TypeScript since the buggy scenario is a runtime type mismatch):

🧪 Suggested improvement to cover the bug path 
 export const ArrayWithBuiltinNames: Story = {
   name: 'Array with option names matching built-in Array methods',
   args: {
     value: optionsWithArrayMethodNames[0],
     argType: { options: optionsWithArrayMethodNames },
+    // Simulates the bug: labels accidentally being an Array object causes
+    // labels?.['reverse'] to return Array.prototype.reverse (a function).
+    // The fix guards against this by checking typeof label === 'string'.
+    // eslint-disable-next-line `@typescript-eslint/no-explicit-any`
+    labels: [] as any,
   },
 };

Alternatively, consider adding a unit test that directly calls normalizeOptions(['normal', 'reverse', 'filter', 'map'], [] as any) and asserts the keys equal the plain strings.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Regression test for https://github.com/storybookjs/storybook/issues/30142
// Options whose names match Array prototype methods (e.g. 'reverse') must be
// displayed as plain text, not as the stringified native function.
const optionsWithArrayMethodNames = ['normal', 'reverse', 'filter', 'map'];
export const ArrayWithBuiltinNames: Story = {
name: 'Array with option names matching built-in Array methods',
args: {
value: optionsWithArrayMethodNames[0],
argType: { options: optionsWithArrayMethodNames },
},
};
// Regression test for https://github.com/storybookjs/storybook/issues/30142
// Options whose names match Array prototype methods (e.g. 'reverse') must be
// displayed as plain text, not as the stringified native function.
const optionsWithArrayMethodNames = ['normal', 'reverse', 'filter', 'map'];
export const ArrayWithBuiltinNames: Story = {
name: 'Array with option names matching built-in Array methods',
args: {
value: optionsWithArrayMethodNames[0],
argType: { options: optionsWithArrayMethodNames },
// Simulates the bug: labels accidentally being an Array object causes
// labels?.['reverse'] to return Array.prototype.reverse (a function).
// The fix guards against this by checking typeof label === 'string'.
// eslint-disable-next-line `@typescript-eslint/no-explicit-any`
labels: [] as any,
},
};
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@code/addons/docs/src/blocks/controls/options/RadioOptions.stories.tsx` around
lines 119 - 129, The story isn't exercising the bug because `labels` is omitted;
update the regression story (ArrayWithBuiltinNames) to pass an explicit
empty-array `labels` (cast to any) so `labels?.['reverse']` will resolve to the
Array.prototype function at runtime — e.g. add `labels: [] as any` alongside
`options: optionsWithArrayMethodNames` in the story args/argType to reproduce
the bug path; alternatively (or additionally) add a unit test that calls
normalizeOptions(optionsWithArrayMethodNames, [] as any) and asserts the
returned keys are the plain strings to guard the fix.


// Reproduces the exact bug from issue #30142: when `labels` is inadvertently an array
// (instead of a Record), `labels?.['reverse']` resolves to `Array.prototype.reverse` — a
// truthy native function — causing the label to render as "function reverse() { [native code] }".
export const ArrayWithBuiltinNamesAndArrayLabels: Story = {
name: 'Array with built-in names and labels accidentally passed as array (bug #30142)',
args: {
value: optionsWithArrayMethodNames[0],
argType: { options: optionsWithArrayMethodNames },
// eslint-disable-next-line @typescript-eslint/no-explicit-any
labels: optionsWithArrayMethodNames as any,
},
};
Loading