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

feat(eslint-rules): add consistent-callback-type rule for event callback typing for react-components and enable it in all v9 packages #30293

Merged
merged 36 commits into from
Feb 7, 2024
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
2faffc9
wip lint rule
YuanboXue-Amber Jan 11, 2024
8836d62
add eslint disable for existing callbacks
YuanboXue-Amber Jan 12, 2024
69dc503
remove testing code in input.type.ts
YuanboXue-Amber Jan 12, 2024
135cce9
changelog
YuanboXue-Amber Jan 12, 2024
5301285
add unit test
YuanboXue-Amber Jan 12, 2024
89cb618
remove react-utilities changelog
YuanboXue-Amber Jan 16, 2024
c35948a
add comments for lint disable and use multi line pragma
YuanboXue-Amber Jan 16, 2024
78c479f
Update packages/eslint-plugin/src/rules/consistent-callback-type/inde…
YuanboXue-Amber Jan 16, 2024
fcce254
Update packages/eslint-plugin/src/rules/consistent-callback-type/inde…
YuanboXue-Amber Jan 16, 2024
2fe4251
Update packages/eslint-plugin/src/rules/consistent-callback-type/inde…
YuanboXue-Amber Jan 16, 2024
29b34cc
Update packages/eslint-plugin/src/rules/consistent-callback-type/inde…
YuanboXue-Amber Jan 16, 2024
5bf9121
prettier after apply suggestion
YuanboXue-Amber Jan 16, 2024
caba8b5
update UT according to comments
YuanboXue-Amber Jan 16, 2024
10c5632
enhance rule
YuanboXue-Amber Jan 16, 2024
e941176
update rule
YuanboXue-Amber Jan 16, 2024
460b012
change matching glob to src instead of matching just *.types.ts
YuanboXue-Amber Jan 16, 2024
2e5d96d
add lint disable after changing scope
YuanboXue-Amber Jan 17, 2024
ad5dbe2
changelog
YuanboXue-Amber Jan 17, 2024
91f6a20
add lint disable
YuanboXue-Amber Jan 17, 2024
c66507f
changelog
YuanboXue-Amber Jan 17, 2024
720bd99
put comment together with disable
YuanboXue-Amber Jan 17, 2024
b98d9f8
update comments
YuanboXue-Amber Jan 18, 2024
9b8fbc0
update lint rule
YuanboXue-Amber Jan 23, 2024
2c1632a
Merge branch 'master' into event-test-lint
YuanboXue-Amber Jan 23, 2024
04a9131
move rule to internal
YuanboXue-Amber Jan 23, 2024
02b6d75
update lint disable
YuanboXue-Amber Jan 23, 2024
84209ac
update change log for eslint-plugin
YuanboXue-Amber Jan 23, 2024
51bc1ba
add internal lint rule in the proper way
YuanboXue-Amber Jan 23, 2024
e1c4b0a
Update packages/eslint-plugin/src/configs/react.js
YuanboXue-Amber Jan 23, 2024
8b671d5
Merge branch 'master' into event-test-lint
YuanboXue-Amber Jan 25, 2024
8c7e484
remove react-rating-preview change log after merge
YuanboXue-Amber Jan 25, 2024
cc028b1
Merge branch 'master' into event-test-lint
YuanboXue-Amber Feb 6, 2024
5d12e30
Merge branch 'master' into event-test-lint
YuanboXue-Amber Feb 7, 2024
99871ce
add lint disable after merge
YuanboXue-Amber Feb 7, 2024
13f7c3e
add changelog
YuanboXue-Amber Feb 7, 2024
a704db8
Merge branch 'master' into event-test-lint
YuanboXue-Amber Feb 7, 2024
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": "feat: add new rule consistent-callback-type",
"packageName": "@fluentui/eslint-plugin",
"email": "[email protected]",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "none",
"comment": "chore: disable consistent-callback-type lint rule for existing callbacks",
"packageName": "@fluentui/react-accordion",
"email": "[email protected]",
"dependentChangeType": "none"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "none",
"comment": "chore: disable consistent-callback-type lint rule for existing callbacks",
"packageName": "@fluentui/react-card",
"email": "[email protected]",
"dependentChangeType": "none"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "none",
"comment": "chore: disable consistent-callback-type lint rule for existing callbacks",
"packageName": "@fluentui/react-checkbox",
"email": "[email protected]",
"dependentChangeType": "none"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "chore: disable consistent-callback-type lint rule for existing callbacks",
"packageName": "@fluentui/react-combobox",
"email": "[email protected]",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "none",
"comment": "chore: disable consistent-callback-type lint rule for existing callbacks",
"packageName": "@fluentui/react-datepicker-compat",
"email": "[email protected]",
"dependentChangeType": "none"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "none",
"comment": "chore: disable consistent-callback-type lint rule for existing callbacks",
"packageName": "@fluentui/react-dialog",
"email": "[email protected]",
"dependentChangeType": "none"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "none",
"comment": "chore: disable consistent-callback-type lint rule for existing callbacks",
"packageName": "@fluentui/react-input",
"email": "[email protected]",
"dependentChangeType": "none"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "none",
"comment": "chore: disable consistent-callback-type lint rule for existing callbacks",
"packageName": "@fluentui/react-menu",
"email": "[email protected]",
"dependentChangeType": "none"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "none",
"comment": "chore: disable consistent-callback-type lint rule for existing callbacks",
"packageName": "@fluentui/react-migration-v0-v9",
"email": "[email protected]",
"dependentChangeType": "none"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "none",
"comment": "chore: disable consistent-callback-type lint rule for existing callbacks",
"packageName": "@fluentui/react-popover",
"email": "[email protected]",
"dependentChangeType": "none"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "none",
"comment": "chore: disable consistent-callback-type lint rule for existing callbacks",
"packageName": "@fluentui/react-radio",
"email": "[email protected]",
"dependentChangeType": "none"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "none",
"comment": "chore: disable consistent-callback-type lint rule for existing callbacks",
"packageName": "@fluentui/react-rating-preview",
"email": "[email protected]",
"dependentChangeType": "none"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "none",
"comment": "chore: disable consistent-callback-type lint rule for existing callbacks",
"packageName": "@fluentui/react-select",
"email": "[email protected]",
"dependentChangeType": "none"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "none",
"comment": "chore: disable consistent-callback-type lint rule for existing callbacks",
"packageName": "@fluentui/react-slider",
"email": "[email protected]",
"dependentChangeType": "none"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "none",
"comment": "chore: disable consistent-callback-type lint rule for existing callbacks",
"packageName": "@fluentui/react-spinbutton",
"email": "[email protected]",
"dependentChangeType": "none"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "none",
"comment": "chore: disable consistent-callback-type lint rule for existing callbacks",
"packageName": "@fluentui/react-switch",
"email": "[email protected]",
"dependentChangeType": "none"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "none",
"comment": "chore: disable consistent-callback-type lint rule for existing callbacks",
"packageName": "@fluentui/react-table",
"email": "[email protected]",
"dependentChangeType": "none"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "none",
"comment": "chore: disable consistent-callback-type lint rule for existing callbacks",
"packageName": "@fluentui/react-tabs",
"email": "[email protected]",
"dependentChangeType": "none"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "none",
"comment": "chore: disable consistent-callback-type lint rule for existing callbacks",
"packageName": "@fluentui/react-tags",
"email": "[email protected]",
"dependentChangeType": "none"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "none",
"comment": "chore: disable consistent-callback-type lint rule for existing callbacks",
"packageName": "@fluentui/react-teaching-popover-preview",
"email": "[email protected]",
"dependentChangeType": "none"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "none",
"comment": "chore: disable consistent-callback-type lint rule for existing callbacks",
"packageName": "@fluentui/react-textarea",
"email": "[email protected]",
"dependentChangeType": "none"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "none",
"comment": "chore: disable consistent-callback-type lint rule for existing callbacks",
"packageName": "@fluentui/react-timepicker-compat",
"email": "[email protected]",
"dependentChangeType": "none"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "chore: disable consistent-callback-type lint rule for existing callbacks",
"packageName": "@fluentui/react-toast",
"email": "[email protected]",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "none",
"comment": "chore: disable consistent-callback-type lint rule for existing callbacks",
"packageName": "@fluentui/react-toolbar",
"email": "[email protected]",
"dependentChangeType": "none"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "none",
"comment": "chore: disable consistent-callback-type lint rule for existing callbacks",
"packageName": "@fluentui/react-tooltip",
"email": "[email protected]",
"dependentChangeType": "none"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "none",
"comment": "chore: disable consistent-callback-type lint rule for existing callbacks",
"packageName": "@fluentui/react-tree",
"email": "[email protected]",
"dependentChangeType": "none"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "none",
"comment": "chore: disable consistent-callback-type lint rule for existing callbacks",
"packageName": "@fluentui/react-virtualizer",
"email": "[email protected]",
"dependentChangeType": "none"
}
6 changes: 6 additions & 0 deletions packages/eslint-plugin/src/configs/react.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,5 +58,11 @@ module.exports = {
'react/jsx-no-bind': 'off',
},
},
{
files: ['**/src/**/*.{ts,tsx}'],
rules: {
'@fluentui/consistent-callback-type': 'error',
},
},
],
};
1 change: 1 addition & 0 deletions packages/eslint-plugin/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ module.exports = {
'no-visibility-modifiers': require('./rules/no-visibility-modifiers'),
'no-restricted-imports': require('./rules/no-restricted-imports'),
'no-context-default-value': require('./rules/no-context-default-value'),
'consistent-callback-type': require('./rules/consistent-callback-type'),
},

// Not a standard eslint thing, just exported for convenience
Expand Down
82 changes: 82 additions & 0 deletions packages/eslint-plugin/src/rules/consistent-callback-type/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
// @ts-check
const { AST_NODE_TYPES } = require('@typescript-eslint/experimental-utils');
const createRule = require('../../utils/createRule');

module.exports = createRule({
name: 'consistent-callback-type',
meta: {
type: 'problem',
docs: {
description: 'Enforce callback props to be typed with `EventHandler`',
category: 'Best Practices',
recommended: 'error',
},
messages: {
invalidType: 'callback props should be typed with @fluentui/react-utilities#EventHandler<T>',
},
schema: [],
},
defaultOptions: [],
create(context) {
/**
* @type {{
* node: import('@typescript-eslint/experimental-utils').TSESTree.TSTypeAliasDeclaration,
* hasInnerTypeLiteral: boolean
* }[]}
*/
const typeAliasStack = []; // use a stack to match the first TSTypeLiteral node within TSTypeAliasDeclaration node

return {
// eslint-disable-next-line @typescript-eslint/naming-convention
'TSTypeAliasDeclaration[id.name=/.*Props$/]': function (node) {
typeAliasStack.push({ node, hasInnerTypeLiteral: false });
},

// eslint-disable-next-line @typescript-eslint/naming-convention
TSTypeLiteral: (/** @type {import('@typescript-eslint/experimental-utils').TSESTree.TSTypeLiteral}*/ node) => {
if (typeAliasStack.length > 0) {
const top = typeAliasStack[typeAliasStack.length - 1];
if (!top.hasInnerTypeLiteral) {
/**
* This is the first TSTypeLiteral node within the current TSTypeAliasDeclaration
* For this example:
* `type SomeProps3 = SomeArgBag & { someFunction?: (args: SomeArgBag) => void; };`
* it matches `{ someFunction?: (args: SomeArgBag) => void; }`
*/
top.hasInnerTypeLiteral = true;

node.members.forEach(member => {
if (
member.type === AST_NODE_TYPES.TSPropertySignature &&
member.key.type === AST_NODE_TYPES.Identifier &&
/^on[A-Z]/.test(member.key.name)
) {
const typeAnnotation = member.typeAnnotation?.typeAnnotation;
// Check if typeAnnotation is of type EventHandler
if (
!(
typeAnnotation &&
typeAnnotation.type === AST_NODE_TYPES.TSTypeReference &&
typeAnnotation.typeName.type === AST_NODE_TYPES.Identifier &&
typeAnnotation.typeName.name === 'EventHandler' &&
typeAnnotation.typeParameters
)
) {
context.report({
node: member,
messageId: 'invalidType',
});
}
}
});
}
}
},

// eslint-disable-next-line @typescript-eslint/naming-convention
'TSTypeAliasDeclaration:exit': function () {
typeAliasStack.pop();
},
};
},
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
// @ts-nocheck
const { ESLintUtils } = require('@typescript-eslint/experimental-utils');
const rule = require('./index');

const ruleTester = new ESLintUtils.RuleTester({
parser: '@typescript-eslint/parser',
});

ruleTester.run('consistent-callback-type', rule, {
valid: [
// Valid when prop is TSTypeAliasDeclaration and the callback uses EventHandler
{
code: `
import { EventHandler, EventData } from '@fluentui/react-utilities';
export type OnSomeEventData = EventData<'focus', React.FocusEvent<HTMLElement>> & {
open: boolean,
};
export type SomeProps = {
onSomeEvent?: EventHandler<OnSomeEventData>,
};
`,
filename: 'src/components/SomeComponent/SomeComponent.type.ts',
},
// Valid when prop is TSIntersectionType and the callback uses EventHandler
{
code: `
import { EventHandler, EventData } from '@fluentui/react-utilities';
export type OnSomeEventData = EventData<'focus', React.FocusEvent<HTMLElement>> & {
open: boolean,
};
export type SomeProps = SomeType & {
onSomeEvent?: EventHandler<OnSomeEventData>,
};
`,
filename: 'src/components/SomeComponent/SomeComponent.type.ts',
},
// Valid when prop has a function that is not callback, and not using EventHandler type
{
code: `
type SomeArgBag = { one: number };
export type SomeProps = {
someFunction?: (args: SomeArgBag) => void;
someFunction2?: (args: { onSomething: string }) => void; // test nested TSTypeLiteral node that starts with 'on'
};
`,
filename: 'src/components/SomeComponent/SomeComponent.type.ts',
},
],
invalid: [
// Invalid when callback in props is not using EventHandler
{
code: `
export type SomeProps = {
onChange?: (ev: React.ChangeEvent<HTMLInputElement>, data: {}) => void;
};
`,
errors: [{ messageId: 'invalidType' }],
},
// Invalid when callback in props is not using EventHandler, and the props is TSIntersectionType
{
code: `
export type SomeProps = SomeType & {
onChange?: (ev: React.ChangeEvent<HTMLInputElement>, data: {}) => void;
};
`,
errors: [{ messageId: 'invalidType' }],
},
],
});
Loading
Loading