BREAKING(react-conformance): update consistent-callback-args option t…

…o run for existing callbacks only (#30376) Co-authored-by: Ben Howell <[email protected]>
microsoft · Feb 7, 2024 · 239dedf · 239dedf
1 parent b3c2fa6
commit 239dedf
Show file tree

Hide file tree

Showing 29 changed files with 110 additions and 18 deletions.
diff --git a/change/@fluentui-react-conformance-49d283fb-c0d3-4de1-9478-33cb86abe5ca.json b/change/@fluentui-react-conformance-49d283fb-c0d3-4de1-9478-33cb86abe5ca.json
@@ -0,0 +1,7 @@
+{
+  "type": "minor",
+  "comment": "BREAKING CHANGE: change `consistent-callback-args` test option to test selected props instead of ignoring props",
+  "packageName": "@fluentui/react-conformance",
+  "email": "[email protected]",
+  "dependentChangeType": "patch"
+}
diff --git a/...act-components/react-avatar/src/components/AvatarGroupPopover/AvatarGroupPopover.test.tsx b/...act-components/react-avatar/src/components/AvatarGroupPopover/AvatarGroupPopover.test.tsx
@@ -47,6 +47,9 @@ describe('AvatarGroupPopover', () => {
           getPortalElement: getPopoverSurfaceElement,
         },
       ],
+      'consistent-callback-args': {
+        legacyCallbacks: ['onOpenChange'],
+      },
     },
     requiredProps: {
       children: (

diff --git a/packages/react-components/react-card/src/components/Card/Card.test.tsx b/packages/react-components/react-card/src/components/Card/Card.test.tsx
@@ -30,6 +30,9 @@ describe('Card', () => {
           },
         },
       ],
+      'consistent-callback-args': {
+        legacyCallbacks: ['onSelectionChange'],
+      },
     },
     disabledTests: ['component-has-static-classname-exported'],
   });

diff --git a/packages/react-components/react-checkbox/src/components/Checkbox/Checkbox.test.tsx b/packages/react-components/react-checkbox/src/components/Checkbox/Checkbox.test.tsx
@@ -23,6 +23,9 @@ describe('Checkbox', () => {
           props: { label: 'Test Label' },
         },
       ],
+      'consistent-callback-args': {
+        legacyCallbacks: ['onChange'],
+      },
     },
   });
 

diff --git a/packages/react-components/react-combobox/src/components/Combobox/Combobox.test.tsx b/packages/react-components/react-combobox/src/components/Combobox/Combobox.test.tsx
@@ -34,6 +34,9 @@ describe('Combobox', () => {
           },
         },
       ],
+      'consistent-callback-args': {
+        legacyCallbacks: ['onOpenChange', 'onOptionSelect'],
+      },
     },
   });
 

diff --git a/packages/react-components/react-combobox/src/components/Dropdown/Dropdown.test.tsx b/packages/react-components/react-combobox/src/components/Dropdown/Dropdown.test.tsx
@@ -34,6 +34,9 @@ describe('Dropdown', () => {
           },
         },
       ],
+      'consistent-callback-args': {
+        legacyCallbacks: ['onOpenChange', 'onOptionSelect'],
+      },
     },
   });
 

diff --git a/packages/react-components/react-combobox/src/components/Listbox/Listbox.test.tsx b/packages/react-components/react-combobox/src/components/Listbox/Listbox.test.tsx
@@ -8,6 +8,11 @@ describe('Listbox', () => {
   isConformant({
     Component: Listbox,
     displayName: 'Listbox',
+    testOptions: {
+      'consistent-callback-args': {
+        legacyCallbacks: ['onOptionSelect'],
+      },
+    },
   });
 
   it('renders a default state', () => {

diff --git a/packages/react-components/react-input/src/components/Input/Input.test.tsx b/packages/react-components/react-input/src/components/Input/Input.test.tsx
@@ -31,6 +31,9 @@ describe('Input', () => {
           },
         },
       ],
+      'consistent-callback-args': {
+        legacyCallbacks: ['onChange'],
+      },
     },
   });
 

diff --git a/packages/react-components/react-menu/src/components/MenuList/MenuList.test.tsx b/packages/react-components/react-menu/src/components/MenuList/MenuList.test.tsx
@@ -14,6 +14,11 @@ describe('MenuList', () => {
       // MenuTrigger does not have own styles
       'make-styles-overrides-win',
     ],
+    testOptions: {
+      'consistent-callback-args': {
+        legacyCallbacks: ['onCheckedValueChange'],
+      },
+    },
   });
 
   /**

diff --git a/packages/react-components/react-migration-v0-v9/src/components/List/List/List.test.tsx b/packages/react-components/react-migration-v0-v9/src/components/List/List/List.test.tsx
@@ -12,12 +12,6 @@ describe('List', () => {
     // Disabled because this should be ItemLayout's responsibility, but it doesn't render those.
     // Adding them there now might not be safe.
     disabledTests: ['component-has-static-classnames-object', 'has-docblock', 'has-top-level-file'],
-    testOptions: {
-      'consistent-callback-args': {
-        // onSelectionChange has an eventArgument which is React.SyntheticEvent. This throws an error during testing
-        ignoreProps: ['onSelectionChange'],
-      },
-    },
   });
 
   // TODO add more tests here, and create visual regression tests in /apps/vr-tests

diff --git a/packages/react-components/react-popover/src/components/Popover/Popover.test.tsx b/packages/react-components/react-popover/src/components/Popover/Popover.test.tsx
@@ -18,6 +18,11 @@ describe('Popover', () => {
       // Popover does not have own styles
       'make-styles-overrides-win',
     ],
+    testOptions: {
+      'consistent-callback-args': {
+        legacyCallbacks: ['onOpenChange'],
+      },
+    },
   });
 
   /**

diff --git a/packages/react-components/react-radio/src/components/Radio/Radio.test.tsx b/packages/react-components/react-radio/src/components/Radio/Radio.test.tsx
@@ -19,6 +19,9 @@ describe('Radio', () => {
           },
         },
       ],
+      'consistent-callback-args': {
+        legacyCallbacks: ['onChange'],
+      },
     },
   });
 

diff --git a/packages/react-components/react-radio/src/components/RadioGroup/RadioGroup.test.tsx b/packages/react-components/react-radio/src/components/RadioGroup/RadioGroup.test.tsx
@@ -10,6 +10,11 @@ describe('RadioGroup', () => {
   isConformant({
     Component: RadioGroup,
     displayName: 'RadioGroup',
+    testOptions: {
+      'consistent-callback-args': {
+        legacyCallbacks: ['onChange'],
+      },
+    },
   });
 
   it('renders a default state', () => {

diff --git a/packages/react-components/react-search-preview/src/components/SearchBox/SearchBox.test.tsx b/packages/react-components/react-search-preview/src/components/SearchBox/SearchBox.test.tsx
@@ -43,6 +43,9 @@ describe('SearchBox', () => {
           },
         },
       ],
+      'consistent-callback-args': {
+        legacyCallbacks: ['onChange'],
+      },
     },
   });
 

diff --git a/packages/react-components/react-select/src/components/Select/Select.test.tsx b/packages/react-components/react-select/src/components/Select/Select.test.tsx
@@ -9,6 +9,11 @@ describe('Select', () => {
     Component: Select,
     displayName: 'Select',
     primarySlot: 'select',
+    testOptions: {
+      'consistent-callback-args': {
+        legacyCallbacks: ['onChange'],
+      },
+    },
   });
 
   // Note for Select tests: avoid using getByRole;

diff --git a/packages/react-components/react-slider/src/components/Slider/Slider.test.tsx b/packages/react-components/react-slider/src/components/Slider/Slider.test.tsx
@@ -10,6 +10,11 @@ describe('Slider', () => {
     Component: Slider,
     displayName: 'Slider',
     primarySlot: 'input',
+    testOptions: {
+      'consistent-callback-args': {
+        legacyCallbacks: ['onChange'],
+      },
+    },
   });
 
   afterEach(() => {

diff --git a/packages/react-components/react-spinbutton/src/components/SpinButton/SpinButton.test.tsx b/packages/react-components/react-spinbutton/src/components/SpinButton/SpinButton.test.tsx
@@ -15,6 +15,11 @@ describe('SpinButton', () => {
     Component: SpinButton,
     displayName: 'SpinButton',
     primarySlot: 'input',
+    testOptions: {
+      'consistent-callback-args': {
+        legacyCallbacks: ['onChange'],
+      },
+    },
   });
 
   it('renders a default uncontrolled state', () => {

diff --git a/packages/react-components/react-switch/src/components/Switch/Switch.test.tsx b/packages/react-components/react-switch/src/components/Switch/Switch.test.tsx
@@ -19,6 +19,9 @@ describe('Switch', () => {
           },
         },
       ],
+      'consistent-callback-args': {
+        legacyCallbacks: ['onChange'],
+      },
     },
   });
 

diff --git a/packages/react-components/react-table/src/components/DataGrid/DataGrid.test.tsx b/packages/react-components/react-table/src/components/DataGrid/DataGrid.test.tsx
@@ -43,6 +43,9 @@ describe('DataGrid', () => {
       'make-styles-overrides-win': {
         callCount: 2,
       },
+      'consistent-callback-args': {
+        legacyCallbacks: ['onSortChange', 'onSelectionChange', 'onColumnResize'],
+      },
     },
     // TODO: https://github.com/microsoft/fluentui/issues/19618
     // eslint-disable-next-line @typescript-eslint/no-explicit-any

diff --git a/packages/react-components/react-tabs/src/components/TabList/TabList.test.tsx b/packages/react-components/react-tabs/src/components/TabList/TabList.test.tsx
@@ -8,6 +8,11 @@ describe('TabList', () => {
   isConformant({
     Component: TabList,
     displayName: 'TabList',
+    testOptions: {
+      'consistent-callback-args': {
+        legacyCallbacks: ['onTabSelect'],
+      },
+    },
   });
 
   it('renders with tabs', () => {

diff --git a/packages/react-components/react-tags/src/components/TagGroup/TagGroup.test.tsx b/packages/react-components/react-tags/src/components/TagGroup/TagGroup.test.tsx
@@ -8,11 +8,6 @@ describe('TagGroup', () => {
   isConformant({
     Component: TagGroup,
     displayName: 'TagGroup',
-    testOptions: {
-      'consistent-callback-args': {
-        ignoreProps: ['onDismiss'], // onDismiss uses generics, this test does not support that
-      },
-    },
   });
 
   it('should invoke onDismiss when clicking on children Tag', () => {

diff --git a/packages/react-components/react-textarea/src/components/Textarea/Textarea.test.tsx b/packages/react-components/react-textarea/src/components/Textarea/Textarea.test.tsx
@@ -22,6 +22,11 @@ describe('Textarea', () => {
     Component: Textarea,
     displayName: 'Textarea',
     primarySlot: 'textarea',
+    testOptions: {
+      'consistent-callback-args': {
+        legacyCallbacks: ['onChange'],
+      },
+    },
   });
 
   // TODO create visual regression tests in /apps/vr-tests

diff --git a/...es/react-components/react-timepicker-compat/src/components/TimePicker/TimePicker.test.tsx b/...es/react-components/react-timepicker-compat/src/components/TimePicker/TimePicker.test.tsx
@@ -36,6 +36,9 @@ describe('TimePicker', () => {
           },
         },
       ],
+      'consistent-callback-args': {
+        legacyCallbacks: ['onOpenChange', 'onTimeChange'],
+      },
     },
   });
 

diff --git a/packages/react-components/react-toolbar/src/components/Toolbar/Toolbar.test.tsx b/packages/react-components/react-toolbar/src/components/Toolbar/Toolbar.test.tsx
@@ -7,6 +7,11 @@ describe('Toolbar', () => {
   isConformant({
     Component: Toolbar,
     displayName: 'Toolbar',
+    testOptions: {
+      'consistent-callback-args': {
+        legacyCallbacks: ['onCheckedValueChange'],
+      },
+    },
   });
 
   // TODO add more tests here, and create visual regression tests in /apps/vr-tests

diff --git a/packages/react-components/react-tooltip/src/components/Tooltip/Tooltip.test.tsx b/packages/react-components/react-tooltip/src/components/Tooltip/Tooltip.test.tsx
@@ -39,6 +39,11 @@ describe('Tooltip', () => {
       'component-has-root-ref',
       'component-handles-classname',
     ],
+    testOptions: {
+      'consistent-callback-args': {
+        legacyCallbacks: ['onVisibleChange'],
+      },
+    },
   });
 
   afterEach(() => {

diff --git a/packages/react-conformance/etc/react-conformance.api.md b/packages/react-conformance/etc/react-conformance.api.md
@@ -56,7 +56,7 @@ export interface TestOptions {
     };
     // (undocumented)
     'consistent-callback-args'?: {
-        ignoreProps?: string[];
+        legacyCallbacks?: string[];
     };
     // (undocumented)
     'consistent-callback-names'?: {

diff --git a/packages/react-conformance/src/defaultTests.tsx b/packages/react-conformance/src/defaultTests.tsx
@@ -453,10 +453,21 @@ export const defaultTests: DefaultTestObject = {
       const { testOptions = {} } = testInfo;
 
       const propNames = Object.keys(componentInfo.props);
-      const ignoreProps = testOptions['consistent-callback-args']?.ignoreProps || [];
+      const legacyCallbacks = testOptions['consistent-callback-args']?.legacyCallbacks || [];
+
+      // verify that legacyCallbacks option contains real props:
+      const legacyCallbacksNotInProp = legacyCallbacks.filter(legacyCallback => !propNames.includes(legacyCallback));
+      if (legacyCallbacksNotInProp.length) {
+        throw new Error(
+          [
+            `Option "consistent-callback-args.legacyCallbacks" contains "${legacyCallbacksNotInProp.join(', ')}" prop,`,
+            'which is not present in component props.',
+          ].join(' '),
+        );
+      }
 
       const invalidProps = propNames.reduce<Record<string, Error>>((errors, propName) => {
-        if (!ignoreProps.includes(propName) && CALLBACK_REGEX.test(propName)) {
+        if (legacyCallbacks.includes(propName)) {
           const propInfo = componentInfo.props[propName];
 
           if (!propInfo.declarations) {

diff --git a/packages/react-conformance/src/types.ts b/packages/react-conformance/src/types.ts
@@ -12,7 +12,7 @@ export interface TestOptions {
     ignoreProps?: string[];
   };
   'consistent-callback-args'?: {
-    ignoreProps?: string[];
+    legacyCallbacks?: string[];
   };
   'has-static-classnames'?: {
     props: {

diff --git a/packages/react-focus/src/components/FocusZone/FocusZone.test.tsx b/packages/react-focus/src/components/FocusZone/FocusZone.test.tsx
@@ -91,9 +91,6 @@ describe('FocusZone', () => {
     elementRefName: 'elementRef',
     testOptions: {
       'consistent-callback-names': { ignoreProps: ['onActiveElementChanged'] },
-      'consistent-callback-args': {
-        ignoreProps: ['onActiveElementChanged', 'onBeforeFocus', 'onFocusNotification', 'onFocus'],
-      },
     },
   });
-Original file line number
+Diff line change
@@ Expand Up / @@ -30,6 +30,9 @@ describe('Card', () => { @@
               },
             },
           ],
+          'consistent-callback-args': {
+            legacyCallbacks: ['onSelectionChange'],
+          },
         },
         disabledTests: ['component-has-static-classname-exported'],
       });
@@ Expand Down @@