feat: improve conformance test to check argument types

change/@fluentui-react-conformance-618c8cde-a60c-4c41-be1e-d05081203b44.json

@@ -0,0 +1,7 @@
+{
+  "type": "minor",
+  "comment": "Implement additional tests in consistent-callback-args",
+  "packageName": "@fluentui/react-conformance",
+  "email": "olfedias@microsoft.com",
+  "dependentChangeType": "patch"
+}

change/@fluentui-react-focus-cfef290b-97f1-4b2f-a1a0-73149bf33f95.json

@@ -0,0 +1,7 @@
+{
+  "type": "none",
+  "comment": "Disable consistent-callback-args test",
+  "packageName": "@fluentui/react-focus",
+  "email": "olfedias@microsoft.com",
+  "dependentChangeType": "none"
+}

change/@fluentui-react-menu-b4635e5b-4210-4d0c-b478-370ef0c8d373.json

@@ -0,0 +1,7 @@
+{
+  "type": "prerelease",
+  "comment": "Simplify type for MenuOpenChangeData",
+  "packageName": "@fluentui/react-menu",
+  "email": "olfedias@microsoft.com",
+  "dependentChangeType": "patch"
+}

change/@fluentui-react-popover-c2b1b743-ef8b-4a73-8511-6f6abff38e2b.json

@@ -0,0 +1,7 @@
+{
+  "type": "prerelease",
+  "comment": "Simplify type for OnOpenChangeData",
+  "packageName": "@fluentui/react-popover",
+  "email": "olfedias@microsoft.com",
+  "dependentChangeType": "patch"
+}

change/@fluentui-react-tabs-0c8ac83c-97ab-45db-834d-67f42ff73b3c.json

@@ -0,0 +1,7 @@
+{
+  "type": "none",
+  "comment": "Disable consistent-callback-args test",
+  "packageName": "@fluentui/react-tabs",
+  "email": "olfedias@microsoft.com",
+  "dependentChangeType": "none"
+}

packages/react-conformance/src/defaultErrorMessages.tsx

@@ -1,10 +1,9 @@
 import { IsConformantOptions } from './types';
 
 import { EOL } from 'os';
-import * as _ from 'lodash';
 import * as path from 'path';
 
-import { errorMessageColors, formatArray, getErrorMessage } from './utils/errorMessages';
+import { errorMessageColors, formatArray, getErrorMessage, formatErrors } from './utils/errorMessages';
 
 /* eslint-disable @typescript-eslint/naming-convention */
 
@@ -488,16 +487,18 @@ export const defaultErrorMessages = {
     });
   },
 
-  'consistent-callback-args': (testInfo: IsConformantOptions, invalidProps: string[]) => {
+  'consistent-callback-args': (testInfo: IsConformantOptions, invalidProps: Record<string, Error>) => {
     const { displayName } = testInfo;
     const { testErrorInfo, resolveInfo } = errorMessageColors;
 
     return getErrorMessage({
       displayName,
       overview: 'uses non-standard callback arguments.',
-      details: ['These callback(s) need have two params:', testErrorInfo(formatArray(invalidProps))],
+      details: ['These callback(s) have issues:', testErrorInfo(formatErrors(invalidProps))],
       suggestions: [
-        `Ensure that ${resolveInfo(displayName + `'s`)} callbacks have two params (an event and data object).`,
+        `Ensure that ${resolveInfo(
+          displayName + `'s`,
+        )} callbacks have two params (an event and data object) and types of arguments are correct.`,
         `If a callback is intended to have a different signature, add the prop to isConformant ${resolveInfo(
           "testOptions['consistent-callback-args'].ignoreProps",
         )}.`,

packages/react-conformance/src/defaultTests.tsx

@@ -6,6 +6,7 @@ import { getCallbackArguments } from './utils/getCallbackArguments';
 import { mount, ReactWrapper } from 'enzyme';
 import { act } from 'react-dom/test-utils';
 import parseDocblock from './utils/parseDocblock';
+import { validateCallbackArguments } from './utils/validateCallbackArguments';
 
 import * as React from 'react';
 import * as _ from 'lodash';
@@ -344,7 +345,7 @@ export const defaultTests: TestObject = {
       const propNames = Object.keys(componentInfo.props);
       const ignoreProps = testOptions['consistent-callback-args']?.ignoreProps || [];
 
-      const invalidProps = propNames.filter(propName => {
+      const invalidProps = propNames.reduce<Record<string, Error>>((errors, propName) => {
         if (!ignoreProps.includes(propName) && CALLBACK_REGEX.test(propName)) {
           const propInfo = componentInfo.props[propName];
 
@@ -370,20 +371,22 @@ export const defaultTests: TestObject = {
           }
 
           const rootFileName = propInfo.declarations[0].fileName;
-          const typeName = propInfo.declarations[0].name;
+          const propsTypeName = propInfo.declarations[0].name;
 
-          const handlerArguments = getCallbackArguments(tsProgram, rootFileName, typeName, propName);
+          try {
+            validateCallbackArguments(getCallbackArguments(tsProgram, rootFileName, propsTypeName, propName));
+          } catch (err) {
+            console.log('err', err);
 
-          if (Object.keys(handlerArguments).length !== 2) {
-            return true;
+            return { ...errors, [propName]: err };
           }
         }
 
-        return false;
-      });
+        return errors;
+      }, {});
 
       try {
-        expect(invalidProps).toEqual([]);
+        expect(invalidProps).toEqual({});
       } catch (e) {
         throw new Error(defaultErrorMessages['consistent-callback-args'](testInfo, invalidProps));
       }

packages/react-conformance/src/utils/errorMessages.ts

@@ -62,3 +62,14 @@ export function getErrorMessage(params: {
 export function formatArray(arr: string[] | undefined) {
   return arr ? arr.map(value => `    ${value}`).join(EOL) : 'received undefined';
 }
+
+/**
+ * Formats an object with props & errors strings to be displayed in the console.
+ */
+export function formatErrors(value: Record<string, Error> | undefined) {
+  return value
+    ? Object.entries(value)
+        .map(([propName, error]) => `    ${propName}: ${error.message}`)
+        .join(EOL)
+    : 'received undefined';
+}

packages/react-conformance/src/utils/getCallbackArguments.test.ts

@@ -66,7 +66,7 @@ describe('getCallbackArguments', () => {
       });
       const type = getCallbackArguments(program, 'Accordion.types.ts', 'AccordionProps', 'onToggle');
 
-      expect(type).toMatchObject({});
+      expect(type).toEqual([]);
     });
 
     it('handles "null" as a param', async () => {
@@ -75,7 +75,7 @@ describe('getCallbackArguments', () => {
       });
       const type = getCallbackArguments(program, 'Accordion.types.ts', 'AccordionProps', 'onToggle');
 
-      expect(type).toMatchObject({ e: null });
+      expect(type).toEqual([['e', null]]);
     });
 
     it('handles primitives as a param', async () => {
@@ -86,7 +86,12 @@ describe('getCallbackArguments', () => {
       });
       const type = getCallbackArguments(program, 'Accordion.types.ts', 'AccordionProps', 'onToggle');
 
-      expect(type).toMatchObject({ a: 'string', b: 'number', c: 'boolean', d: undefined });
+      expect(type).toEqual([
+        ['a', 'string'],
+        ['b', 'number'],
+        ['c', 'boolean'],
+        ['d', undefined],
+      ]);
     });
 
     it('handles arrays', async () => {
@@ -95,7 +100,7 @@ describe('getCallbackArguments', () => {
       });
       const type = getCallbackArguments(program, 'Accordion.types.ts', 'AccordionProps', 'onToggle');
 
-      expect(type).toMatchObject({ data: 'Array' });
+      expect(type).toEqual([['data', 'Array']]);
     });
 
     it('handles simple type reference as a param', async () => {
@@ -104,7 +109,7 @@ describe('getCallbackArguments', () => {
       });
       const type = getCallbackArguments(program, 'Accordion.types.ts', 'AccordionProps', 'onToggle');
 
-      expect(type).toMatchObject({ e: 'Event' });
+      expect(type).toEqual([['e', 'Event']]);
     });
 
     it('handles complex type reference as a param', async () => {
@@ -115,7 +120,7 @@ describe('getCallbackArguments', () => {
       });
       const type = getCallbackArguments(program, 'Accordion.types.ts', 'AccordionProps', 'onToggle');
 
-      expect(type).toMatchObject({ e: 'React.MouseEvent' });
+      expect(type).toEqual([['e', 'React.MouseEvent']]);
     });
 
     it('handles alias type as a param', async () => {
@@ -127,7 +132,7 @@ describe('getCallbackArguments', () => {
       });
       const type = getCallbackArguments(program, 'Accordion.types.ts', 'AccordionProps', 'onToggle');
 
-      expect(type).toMatchObject({ e: 'React.MouseEvent' });
+      expect(type).toEqual([['e', 'React.MouseEvent']]);
     });
 
     it('handles imported alias type as a param', async () => {
@@ -140,29 +145,67 @@ describe('getCallbackArguments', () => {
       });
       const type = getCallbackArguments(program, 'Accordion.types.ts', 'AccordionProps', 'onToggle');
 
-      expect(type).toMatchObject({ e: 'React.MouseEvent' });
-    });
-
-    // TODO: to write better assertions we will need to resolve some of our custom types
-    // it.todo('revolves references', async () => {
-    //   const program = await setupProgram(['Accordion.types.ts'], {
-    //     './Accordion.types.ts': `import * as React from 'react';
-    //
-    //     export interface AccordionToggleData {
-    //       value: AccordionItemValue;
-    //     }
-    //
-    //     export type AccordionToggleEvent<E = HTMLElement> = React.MouseEvent<E> | React.KeyboardEvent<E>;
-    //     export type AccordionToggleEventHandler = (event: AccordionToggleEvent, data: AccordionToggleData) => void;
-    //
-    //     export interface AccordionProps {
-    //       onToggle: AccordionToggleEventHandler;
-    //     }`,
-    //   });
-    //   const type = getCallbackArguments(program, 'Accordion.types.ts', 'AccordionProps', 'onToggle');
-    //
-    //   expect(type).toMatchObject({ event: ['React.MouseEvent', 'React.KeyboardEvent'], data: { value: 'String' } });
-    // });
+      expect(type).toEqual([['e', 'React.MouseEvent']]);
+    });
+
+    it('revolves references', async () => {
+      const program = await setupProgram(['Accordion.types.ts'], {
+        './Accordion.types.ts': `import * as React from 'react';
+
+        export type TypeA = React.MouseEvent;
+        export type TypeString = string;
+        export type TypeB = React.MouseEvent | MouseEvent | TypeString | number;
+
+        export type AccordionToggleEventHandler = (a: TypeA, b: TypeB, c: React.MouseEvent) => void;
+
+        export interface AccordionProps {
+          onToggle: AccordionToggleEventHandler;
+        }`,
+      });
+      const type = getCallbackArguments(program, 'Accordion.types.ts', 'AccordionProps', 'onToggle');
+
+      expect(type).toEqual([
+        ['a', 'React.MouseEvent'],
+        ['b', ['string', 'number', 'MouseEvent', 'React.MouseEvent']],
+        ['c', 'React.MouseEvent'],
+      ]);
+    });
+
+    it('revolves references to classes', async () => {
+      const program = await setupProgram(['Accordion.types.ts'], {
+        './Accordion.types.ts': `import * as React from 'react';
+
+        export class TabItem {}
+        export interface AccordionProps {
+          onToggle: (item: TabItem) => void;
+        }`,
+      });
+      const type = getCallbackArguments(program, 'Accordion.types.ts', 'AccordionProps', 'onToggle');
+
+      expect(type).toEqual([['item', 'TabItem']]);
+    });
+
+    it('revolves references to interfaces', async () => {
+      const program = await setupProgram(['Accordion.types.ts'], {
+        './Accordion.types.ts': `import * as React from 'react';
+
+        type AccordionItemValue = string
+
+        export interface Data {
+          key: string
+          value: AccordionItemValue;
+        }
+
+        export type AccordionToggleEventHandler = (data: Data) => void;
+
+        export interface AccordionProps {
+          onToggle: AccordionToggleEventHandler;
+        }`,
+      });
+      const type = getCallbackArguments(program, 'Accordion.types.ts', 'AccordionProps', 'onToggle');
+
+      expect(type).toEqual([['data', { key: 'string', value: 'string' }]]);
+    });
 
     it('handles generics', async () => {
       const program = await setupProgram(['Accordion.types.ts'], {
@@ -172,7 +215,7 @@ describe('getCallbackArguments', () => {
       });
       const type = getCallbackArguments(program, 'Accordion.types.ts', 'AccordionProps', 'onToggle');
 
-      expect(type).toMatchObject({ e: 'React.MouseEvent' });
+      expect(type).toEqual([['e', 'React.MouseEvent']]);
     });
 
     it('handles unions', async () => {
@@ -183,7 +226,7 @@ describe('getCallbackArguments', () => {
       });
       const type = getCallbackArguments(program, 'Accordion.types.ts', 'AccordionProps', 'onToggle');
 
-      expect(type).toMatchObject({ e: ['MouseEvent', 'React.MouseEvent'] });
+      expect(type).toEqual([['e', ['MouseEvent', 'React.MouseEvent']]]);
     });
 
     it('handles multiple params', async () => {
@@ -194,7 +237,10 @@ describe('getCallbackArguments', () => {
       });
       const type = getCallbackArguments(program, 'Accordion.types.ts', 'AccordionProps', 'onToggle');
 
-      expect(type).toMatchObject({ e: null, data: { value: 'string' } });
+      expect(type).toEqual([
+        ['e', null],
+        ['data', { value: 'string' }],
+      ]);
     });
   });
 
@@ -235,5 +281,23 @@ describe('getCallbackArguments', () => {
         `"A file (Accordion.types.ts) does not contain definition for type \\"AccordionProps.onClick\\"."`,
       );
     });
+
+    it('throws on complex types', async () => {
+      const program = await setupProgram(['Accordion.types.ts'], {
+        './Accordion.types.ts': `import * as React from 'react';
+
+        type TypeA = { open: boolean }
+        type TypeB = Pick<TypeA, 'open'>
+        export interface AccordionProps { onToggle: (a: TypeB) => void; }`,
+      });
+
+      /* eslint-disable @fluentui/max-len */
+      expect(() =>
+        getCallbackArguments(program, 'Accordion.types.ts', 'AccordionProps', 'onToggle'),
+      ).toThrowErrorMatchingInlineSnapshot(
+        `"We received a type \\"Pick<TypeA, \\"open\\">\\" that is too complex to resolve. Please simply it, for example remove usage of \\"Pick\\"."`,
+      );
+      /* eslint-enable @fluentui/max-len */
+    });
   });
 });