Skip to content
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.

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

BREAKING(react-conformance): update consistent-callback-args option to run for existing callbacks only #30376

Merged
merged 8 commits into from
Feb 7, 2024
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
@@ -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"
}
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,9 @@ describe('AvatarGroupPopover', () => {
getPortalElement: getPopoverSurfaceElement,
},
],
'consistent-callback-args': {
legacyCallbacks: ['onOpenChange'],
},
},
requiredProps: {
children: (
Expand Down
Original file line number Diff 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
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ describe('Checkbox', () => {
props: { label: 'Test Label' },
},
],
'consistent-callback-args': {
legacyCallbacks: ['onChange'],
},
},
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ describe('Combobox', () => {
},
},
],
'consistent-callback-args': {
legacyCallbacks: ['onOpenChange', 'onOptionSelect'],
},
},
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ describe('Dropdown', () => {
},
},
],
'consistent-callback-args': {
legacyCallbacks: ['onOpenChange', 'onOptionSelect'],
},
},
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,11 @@ describe('Listbox', () => {
isConformant({
Component: Listbox,
displayName: 'Listbox',
testOptions: {
'consistent-callback-args': {
legacyCallbacks: ['onOptionSelect'],
},
},
});

it('renders a default state', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ describe('Input', () => {
},
},
],
'consistent-callback-args': {
legacyCallbacks: ['onChange'],
},
},
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,11 @@ describe('MenuList', () => {
// MenuTrigger does not have own styles
'make-styles-overrides-win',
],
testOptions: {
'consistent-callback-args': {
legacyCallbacks: ['onCheckedValueChange'],
},
},
});

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@ describe('Popover', () => {
// Popover does not have own styles
'make-styles-overrides-win',
],
testOptions: {
'consistent-callback-args': {
legacyCallbacks: ['onOpenChange'],
},
},
});

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ describe('Radio', () => {
},
},
],
'consistent-callback-args': {
legacyCallbacks: ['onChange'],
},
},
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,11 @@ describe('RadioGroup', () => {
isConformant({
Component: RadioGroup,
displayName: 'RadioGroup',
testOptions: {
'consistent-callback-args': {
legacyCallbacks: ['onChange'],
},
},
});

it('renders a default state', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@ describe('SearchBox', () => {
},
},
],
'consistent-callback-args': {
legacyCallbacks: ['onChange'],
},
},
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,11 @@ describe('Slider', () => {
Component: Slider,
displayName: 'Slider',
primarySlot: 'input',
testOptions: {
'consistent-callback-args': {
legacyCallbacks: ['onChange'],
},
},
});

afterEach(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,11 @@ describe('SpinButton', () => {
Component: SpinButton,
displayName: 'SpinButton',
primarySlot: 'input',
testOptions: {
'consistent-callback-args': {
legacyCallbacks: ['onChange'],
},
},
});

it('renders a default uncontrolled state', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ describe('Switch', () => {
},
},
],
'consistent-callback-args': {
legacyCallbacks: ['onChange'],
},
},
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,11 @@ describe('TabList', () => {
isConformant({
Component: TabList,
displayName: 'TabList',
testOptions: {
'consistent-callback-args': {
legacyCallbacks: ['onTabSelect'],
},
},
});

it('renders with tabs', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ describe('TimePicker', () => {
},
},
],
'consistent-callback-args': {
legacyCallbacks: ['onOpenChange', 'onTimeChange'],
},
},
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,11 @@ describe('Tooltip', () => {
'component-has-root-ref',
'component-handles-classname',
],
testOptions: {
'consistent-callback-args': {
legacyCallbacks: ['onVisibleChange'],
},
},
});

afterEach(() => {
Expand Down
2 changes: 1 addition & 1 deletion packages/react-conformance/etc/react-conformance.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ export interface TestOptions {
};
// (undocumented)
'consistent-callback-args'?: {
ignoreProps?: string[];
legacyCallbacks?: string[];
};
// (undocumented)
'consistent-callback-names'?: {
Expand Down
15 changes: 13 additions & 2 deletions packages/react-conformance/src/defaultTests.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion packages/react-conformance/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ export interface TestOptions {
ignoreProps?: string[];
};
'consistent-callback-args'?: {
ignoreProps?: string[];
legacyCallbacks?: string[];
};
'has-static-classnames'?: {
props: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,6 @@ describe('FocusZone', () => {
elementRefName: 'elementRef',
testOptions: {
'consistent-callback-names': { ignoreProps: ['onActiveElementChanged'] },
'consistent-callback-args': {
ignoreProps: ['onActiveElementChanged', 'onBeforeFocus', 'onFocusNotification', 'onFocus'],
},
},
});

Expand Down
Loading