From 941ceab3b4696fcc13df79f4c7967655fab817cd Mon Sep 17 00:00:00 2001 From: Liu Liu Date: Wed, 25 Jun 2025 11:10:05 -0700 Subject: [PATCH 1/2] error->warn --- .../react/src/ActionList/ActionList.test.tsx | 31 ++++++++++--------- packages/react/src/ActionList/Selection.tsx | 10 +++--- 2 files changed, 21 insertions(+), 20 deletions(-) diff --git a/packages/react/src/ActionList/ActionList.test.tsx b/packages/react/src/ActionList/ActionList.test.tsx index b65aa6ee803..2bbdea2ff1d 100644 --- a/packages/react/src/ActionList/ActionList.test.tsx +++ b/packages/react/src/ActionList/ActionList.test.tsx @@ -30,21 +30,24 @@ describe('ActionList', () => { expect(results).toHaveNoViolations() }) - it('should throw when selected is provided without a selectionVariant on parent', async () => { - // we expect console.error to be called, so we suppress that in the test - const mockError = vi.spyOn(console, 'error').mockImplementation(() => vi.fn()) - - expect(() => { - HTMLRender( - - - Primer React - - , - ) - }).toThrow('For Item to be selected, ActionList or ActionList.Group needs to have a selectionVariant defined') + it('should warn when selected is provided without a selectionVariant on parent', async () => { + // we expect console.warn to be called, so we spy on that in the test + const spy = vi.spyOn(console, 'warn').mockImplementation(() => vi.fn()) + + HTMLRender( + + + Primer React + + , + ) + + expect(spy).toHaveBeenCalledWith( + 'Warning:', + 'For Item to be selected, ActionList or ActionList.Group should have a selectionVariant defined.', + ) - mockError.mockRestore() + spy.mockRestore() }) it('should be navigatable with arrow keys for certain roles', async () => { diff --git a/packages/react/src/ActionList/Selection.tsx b/packages/react/src/ActionList/Selection.tsx index 42d1933164f..6fbab8c20ee 100644 --- a/packages/react/src/ActionList/Selection.tsx +++ b/packages/react/src/ActionList/Selection.tsx @@ -6,6 +6,7 @@ import {type ActionListProps, type ActionListItemProps, ListContext} from './sha import {VisualContainer} from './Visuals' import classes from './ActionList.module.css' import Radio from '../Radio' +import {warning} from '../utils/warning' type SelectionProps = Pick export const Selection: React.FC> = ({selected, className}) => { @@ -20,14 +21,11 @@ export const Selection: React.FC> = ({se if (!selectionVariant) { // if selectionVariant is not set on List, but Item is selected - // fail loudly instead of silently ignoring + // warn in development if (selected) { - throw new Error( - 'For Item to be selected, ActionList or ActionList.Group needs to have a selectionVariant defined', - ) - } else { - return null + warning(true, 'For Item to be selected, ActionList or ActionList.Group should have a selectionVariant defined.') } + return null } if (selectionVariant === 'radio') { From f0d06e29a9ea4f6974b8c43985b5a795327e273e Mon Sep 17 00:00:00 2001 From: Liu Liu Date: Wed, 25 Jun 2025 11:20:16 -0700 Subject: [PATCH 2/2] changeset --- .changeset/true-lines-find.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/true-lines-find.md diff --git a/.changeset/true-lines-find.md b/.changeset/true-lines-find.md new file mode 100644 index 00000000000..4e1c4e42a9e --- /dev/null +++ b/.changeset/true-lines-find.md @@ -0,0 +1,5 @@ +--- +'@primer/react': patch +--- + +Prevent ActionList crash when selected prop is true without selectionVariant.