diff --git a/packages/eslint-plugin/src/internal.js b/packages/eslint-plugin/src/internal.js index 02c31f5aa97d30..19afbe17a62174 100644 --- a/packages/eslint-plugin/src/internal.js +++ b/packages/eslint-plugin/src/internal.js @@ -35,6 +35,7 @@ const __internal = { /** @type {import('eslint').Linter.RulesRecord} */ rules: { '@nx/workspace-consistent-callback-type': 'error', + '@nx/workspace-consistent-base-hook': 'error', '@nx/workspace-no-restricted-globals': restrictedGlobals.react, '@nx/workspace-no-missing-jsx-pragma': ['error', { runtime: 'automatic' }], }, diff --git a/packages/react-components/react-menu/library/src/components/MenuTrigger/useMenuTrigger.ts b/packages/react-components/react-menu/library/src/components/MenuTrigger/useMenuTrigger.ts index 9ee9a91e217b73..90c99f3a49ad95 100644 --- a/packages/react-components/react-menu/library/src/components/MenuTrigger/useMenuTrigger.ts +++ b/packages/react-components/react-menu/library/src/components/MenuTrigger/useMenuTrigger.ts @@ -75,6 +75,7 @@ export type UseMenuTriggerBaseOptions = { * * @public */ +// eslint-disable-next-line @nx/workspace-consistent-base-hook -- legacy: second param is `options` instead of `ref`; refactor pending export const useMenuTriggerBase_unstable = ( props: MenuTriggerProps, options?: UseMenuTriggerBaseOptions, diff --git a/packages/react-components/react-tags/library/src/components/TagGroup/useTagGroup.ts b/packages/react-components/react-tags/library/src/components/TagGroup/useTagGroup.ts index 21dd23eb1b8c52..fc23db87cd0dbe 100644 --- a/packages/react-components/react-tags/library/src/components/TagGroup/useTagGroup.ts +++ b/packages/react-components/react-tags/library/src/components/TagGroup/useTagGroup.ts @@ -38,6 +38,7 @@ export const useTagGroupBase_unstable = ( const innerRef = React.useRef(undefined); const { targetDocument } = useFluent(); + // eslint-disable-next-line @nx/workspace-consistent-base-hook -- legacy: tabster usage should be moved out of base hook const { findNextFocusable, findPrevFocusable } = useFocusFinders(); const [items, setItems] = useControllableState>({ @@ -80,6 +81,7 @@ export const useTagGroupBase_unstable = ( }), ); + // eslint-disable-next-line @nx/workspace-consistent-base-hook -- legacy: tabster usage should be moved out of base hook const arrowNavigationProps = useArrowNavigationGroup({ circular: true, axis: 'both', diff --git a/packages/react-components/react-tooltip/library/src/components/Tooltip/useTooltipBase.tsx b/packages/react-components/react-tooltip/library/src/components/Tooltip/useTooltipBase.tsx index 4919ea58156ac3..30d586b5e79b43 100644 --- a/packages/react-components/react-tooltip/library/src/components/Tooltip/useTooltipBase.tsx +++ b/packages/react-components/react-tooltip/library/src/components/Tooltip/useTooltipBase.tsx @@ -35,7 +35,7 @@ import { Escape } from '@fluentui/keyboard-keys'; * @param props - props from this instance of Tooltip */ export const useTooltipBase_unstable = (props: TooltipBaseProps): TooltipBaseState => { - 'use no memo'; + ('use no memo'); const context = useTooltipVisibility(); const isServerSideRender = useIsSSR(); diff --git a/tools/eslint-rules/index.ts b/tools/eslint-rules/index.ts index 6aeaae6c06da8e..8e6f5eca37fd1f 100644 --- a/tools/eslint-rules/index.ts +++ b/tools/eslint-rules/index.ts @@ -4,6 +4,7 @@ import { RULE_NAME as consistentCallbackTypeName, rule as consistentCallbackType, } from './rules/consistent-callback-type'; +import { RULE_NAME as consistentBaseHookName, rule as consistentBaseHook } from './rules/consistent-base-hook'; /** * Import your custom workspace rules at the top of this file. @@ -32,6 +33,7 @@ module.exports = { */ rules: { [consistentCallbackTypeName]: consistentCallbackType, + [consistentBaseHookName]: consistentBaseHook, [noRestrictedGlobalsName]: noRestrictedGlobals, [noMissingJsxPragmaName]: noMissingJsxPragma, }, diff --git a/tools/eslint-rules/rules/consistent-base-hook.spec.ts b/tools/eslint-rules/rules/consistent-base-hook.spec.ts new file mode 100644 index 00000000000000..c15112c95fb9fb --- /dev/null +++ b/tools/eslint-rules/rules/consistent-base-hook.spec.ts @@ -0,0 +1,248 @@ +import { RuleTester } from '@typescript-eslint/rule-tester'; +import { rule, RULE_NAME } from './consistent-base-hook'; + +const ruleTester = new RuleTester(); + +ruleTester.run(RULE_NAME, rule, { + valid: [ + // Valid base hook: 2 Identifier params, no forbidden imports. + { + code: ` + import * as React from 'react'; + export const useThingBase_unstable = (props, ref: React.Ref) => { + return { props, ref }; + }; + `, + }, + // Valid base hook declared as FunctionDeclaration. + { + code: ` + import { Ref } from 'react'; + export function useThingBase_unstable(props, ref: Ref) { + return { props, ref }; + } + `, + }, + // Valid base hook with only `props` (ref is optional). + { + code: ` + export const useThingBase_unstable = (props) => { + return { props }; + }; + `, + }, + // Forbidden import exists but is only used by a non-base hook in the same file. + { + code: ` + import { useArrowNavigationGroup } from '@fluentui/react-tabster'; + export const useThingBase_unstable = (props, ref: React.Ref) => { + return { props, ref }; + }; + export const useThing_unstable = (props, ref) => { + const nav = useArrowNavigationGroup({}); + return { ...useThingBase_unstable(props, ref), nav }; + }; + `, + }, + // Non-base hook is not subject to the param-shape constraint. + { + code: ` + export const useThing_unstable = (props, ref, extra) => { + return { props, ref, extra }; + }; + `, + }, + // Allowlist opt-out: a specific imported name is permitted inside base hooks. + { + code: ` + import { useFocusFinders } from '@fluentui/react-tabster'; + export const useThingBase_unstable = (props, ref: React.Ref) => { + const finders = useFocusFinders(); + return { props, ref, finders }; + }; + `, + options: [ + { + forbiddenPackages: [{ name: '@fluentui/react-tabster', allow: ['useFocusFinders'] }], + }, + ], + }, + // Default allowlist: `useFocusWithin` and `useFocusVisible` from `@fluentui/react-tabster` are permitted inside base hooks. + { + code: ` + import { useFocusWithin, useFocusVisible } from '@fluentui/react-tabster'; + export const useThingBase_unstable = (props, ref: React.Ref) => { + useFocusVisible(); + return { props, ref: useFocusWithin() }; + }; + `, + }, + // `keyborg` is not forbidden by default — bindings imported from it are allowed inside base hooks. + { + code: ` + import { createKeyborg, KEYBORG_FOCUSIN } from 'keyborg'; + export const useThingBase_unstable = (props, ref: React.Ref) => { + return { kb: createKeyborg(window), evt: KEYBORG_FOCUSIN }; + }; + `, + }, + // Identifier with the same local name as a forbidden import alias does not collide via scope analysis. + { + code: ` + import { useArrowNavigationGroup } from '@fluentui/react-tabster'; + export const useThing_unstable = (props, ref) => { + return useArrowNavigationGroup({}); + }; + export const useThingBase_unstable = (props, ref: React.Ref) => { + const useArrowNavigationGroup = () => 1; + return { value: useArrowNavigationGroup() }; + }; + `, + }, + ], + invalid: [ + // Too few params (0). + { + code: ` + export const useThingBase_unstable = () => ({}); + `, + errors: [{ messageId: 'invalidParamCount', data: { hookName: 'useThingBase_unstable', actual: 0 } }], + }, + // Too many params. + { + code: ` + export const useThingBase_unstable = (props, ref, extra) => ({ props, ref, extra }); + `, + errors: [{ messageId: 'invalidParamCount', data: { hookName: 'useThingBase_unstable', actual: 3 } }], + }, + // Wrong param names. + { + code: ` + export const useThingBase_unstable = (p, r) => ({ p, r }); + `, + errors: [ + { + messageId: 'invalidParamName', + data: { hookName: 'useThingBase_unstable', index: 1, expected: 'props', actual: 'p' }, + }, + { + messageId: 'invalidParamName', + data: { hookName: 'useThingBase_unstable', index: 2, expected: 'ref', actual: 'r' }, + }, + ], + }, + // ObjectPattern for `props` is not allowed. + { + code: ` + export const useThingBase_unstable = ({ a }, ref: React.Ref) => ({ a, ref }); + `, + errors: [ + { + messageId: 'invalidParamName', + data: { hookName: 'useThingBase_unstable', index: 1, expected: 'props', actual: '{ ... }' }, + }, + ], + }, + // `ref` parameter without a type annotation. + { + code: ` + export const useThingBase_unstable = (props, ref) => ({ props, ref }); + `, + errors: [ + { + messageId: 'invalidRefType', + data: { hookName: 'useThingBase_unstable', actual: '' }, + }, + ], + }, + // `ref` parameter typed as something other than React.Ref. + { + code: ` + export const useThingBase_unstable = (props, ref: HTMLElement) => ({ props, ref }); + `, + errors: [ + { + messageId: 'invalidRefType', + data: { hookName: 'useThingBase_unstable', actual: 'HTMLElement' }, + }, + ], + }, + // `ref` parameter typed as React.ForwardedRef (must be React.Ref). + { + code: ` + export const useThingBase_unstable = (props, ref: React.ForwardedRef) => ({ props, ref }); + `, + errors: [ + { + messageId: 'invalidRefType', + data: { hookName: 'useThingBase_unstable', actual: 'React.ForwardedRef' }, + }, + ], + }, + // Body uses a binding imported from @fluentui/react-tabster. + { + code: ` + import { useArrowNavigationGroup } from '@fluentui/react-tabster'; + export const useThingBase_unstable = (props, ref: React.Ref) => { + const nav = useArrowNavigationGroup({}); + return { props, ref, nav }; + }; + `, + errors: [ + { + messageId: 'forbiddenPackageUsage', + data: { + hookName: 'useThingBase_unstable', + importedName: 'useArrowNavigationGroup', + package: '@fluentui/react-tabster', + }, + }, + ], + }, + // Body uses a binding imported from tabster, even when aliased. + { + code: ` + import { getTabsterAttribute as gta } from 'tabster'; + export function useThingBase_unstable(props, ref: React.Ref) { + return { attr: gta({}) }; + } + `, + errors: [ + { + messageId: 'forbiddenPackageUsage', + data: { + hookName: 'useThingBase_unstable', + importedName: 'getTabsterAttribute', + package: 'tabster', + }, + }, + ], + }, + // Allowlist excludes one name; siblings still error. + { + code: ` + import { useFocusFinders, useArrowNavigationGroup } from '@fluentui/react-tabster'; + export const useThingBase_unstable = (props, ref: React.Ref) => { + const finders = useFocusFinders(); + const nav = useArrowNavigationGroup({}); + return { props, ref, finders, nav }; + }; + `, + options: [ + { + forbiddenPackages: [{ name: '@fluentui/react-tabster', allow: ['useFocusFinders'] }], + }, + ], + errors: [ + { + messageId: 'forbiddenPackageUsage', + data: { + hookName: 'useThingBase_unstable', + importedName: 'useArrowNavigationGroup', + package: '@fluentui/react-tabster', + }, + }, + ], + }, + ], +}); diff --git a/tools/eslint-rules/rules/consistent-base-hook.ts b/tools/eslint-rules/rules/consistent-base-hook.ts new file mode 100644 index 00000000000000..55dde83ad1acb2 --- /dev/null +++ b/tools/eslint-rules/rules/consistent-base-hook.ts @@ -0,0 +1,329 @@ +import { ESLintUtils, AST_NODE_TYPES, TSESTree, TSESLint } from '@typescript-eslint/utils'; + +// NOTE: The rule will be available in ESLint configs as "@nx/workspace-consistent-base-hook" +export const RULE_NAME = 'consistent-base-hook'; + +const BASE_HOOK_NAME_PATTERN = /^use[A-Z]\w*Base_unstable$/; +const EXPECTED_PARAM_NAMES = ['props', 'ref'] as const; +const MIN_PARAM_COUNT = 1; +const MAX_PARAM_COUNT = 2; +const DEFAULT_FORBIDDEN_PACKAGES: ReadonlyArray = [ + { + name: '@fluentui/react-tabster', + // APIs that only depend on `keyborg` (no `tabster` runtime) are safe to use inside base hooks. + allow: [ + 'useFocusWithin', + 'useFocusVisible', + 'useKeyboardNavAttribute', + 'useIsNavigatingWithKeyboard', + 'useSetKeyboardNavigation', + 'useOnKeyboardNavigationChange', + 'applyFocusVisiblePolyfill', + // re-exports from `keyborg` + 'KEYBORG_FOCUSIN', + 'KeyborgFocusInEvent', + ], + }, + 'tabster', +]; + +type ForbiddenPackageOption = string | { name: string; allow?: string[] }; +type Options = [ + { + forbiddenPackages?: ForbiddenPackageOption[]; + }?, +]; +type MessageIds = 'invalidParamCount' | 'invalidParamName' | 'invalidRefType' | 'forbiddenPackageUsage'; + +interface NormalizedForbiddenPackage { + name: string; + allow: Set; +} + +interface ForbiddenImport { + package: string; + importedName: string; +} + +type BaseHookFunction = TSESTree.FunctionDeclaration | TSESTree.FunctionExpression | TSESTree.ArrowFunctionExpression; + +export const rule = ESLintUtils.RuleCreator(() => __filename)({ + name: RULE_NAME, + meta: { + type: 'problem', + docs: { + description: + 'Enforce the API contract for v9 "base" hooks (`useBase_unstable`): a required `props` parameter and an optional `ref` parameter typed as `React.Ref<...>`, and no usage of bindings from configured forbidden packages (defaults to `@fluentui/react-tabster`, `tabster`, `keyborg`).', + }, + schema: [ + { + type: 'object', + properties: { + forbiddenPackages: { + type: 'array', + items: { + oneOf: [ + { type: 'string' }, + { + type: 'object', + properties: { + name: { type: 'string' }, + allow: { + type: 'array', + items: { type: 'string' }, + uniqueItems: true, + }, + }, + required: ['name'], + additionalProperties: false, + }, + ], + }, + uniqueItems: false, + }, + }, + additionalProperties: false, + }, + ], + messages: { + invalidParamCount: + 'Base hook `{{hookName}}` must take 1 or 2 positional parameters (`props`, optional `ref`), got {{actual}}.', + invalidParamName: + 'Base hook `{{hookName}}` parameter #{{index}} must be named `{{expected}}` (Identifier), got `{{actual}}`.', + invalidRefType: 'Base hook `{{hookName}}` parameter `ref` must be typed as `React.Ref<...>`, got `{{actual}}`.', + forbiddenPackageUsage: + 'Base hook `{{hookName}}` cannot reference `{{importedName}}` from forbidden package `{{package}}`. Move logic that depends on `{{package}}` to the wrapping `*_unstable` hook instead.', + }, + }, + defaultOptions: [{}], + create(context) { + const sourceCode = context.sourceCode; + const options = context.options[0] ?? {}; + const forbiddenPackages = normalizeForbiddenPackages(options.forbiddenPackages); + const forbiddenPackageByName = new Map(forbiddenPackages.map(pkg => [pkg.name, pkg])); + + // Map from import Variable -> origin (package + original imported name). + // Keyed by Variable identity (not name) so shadowing inside a base hook is handled correctly. + const forbiddenImports = new Map(); + + function checkBaseHook(hookName: string, fn: BaseHookFunction, reportNode: TSESTree.Node): void { + checkParameters(hookName, fn, reportNode); + checkBodyReferences(hookName, fn); + } + + function checkParameters(hookName: string, fn: BaseHookFunction, reportNode: TSESTree.Node): void { + if (fn.params.length < MIN_PARAM_COUNT || fn.params.length > MAX_PARAM_COUNT) { + context.report({ + node: reportNode, + messageId: 'invalidParamCount', + data: { + hookName, + actual: fn.params.length, + }, + }); + return; + } + + fn.params.forEach((param, index) => { + const expected = EXPECTED_PARAM_NAMES[index]; + if (param.type !== AST_NODE_TYPES.Identifier || param.name !== expected) { + context.report({ + node: reportNode, + messageId: 'invalidParamName', + data: { + hookName, + index: index + 1, + expected, + actual: describeParam(param), + }, + }); + return; + } + if (index === 1 && !isReactRefTypeAnnotation(param.typeAnnotation)) { + context.report({ + node: reportNode, + messageId: 'invalidRefType', + data: { + hookName, + actual: describeRefType(param.typeAnnotation), + }, + }); + } + }); + } + + function checkBodyReferences(hookName: string, fn: BaseHookFunction): void { + if (forbiddenImports.size === 0) { + return; + } + + const fnScope = sourceCode.getScope(fn); + visitScope(fnScope, fn, hookName); + } + + function visitScope(scope: TSESLint.Scope.Scope, fn: BaseHookFunction, hookName: string): void { + // Only descend into scopes that belong to this base hook's function body. + if (!isScopeWithinFunction(scope, fn)) { + return; + } + + scope.references.forEach(reference => { + const resolved = reference.resolved; + if (!resolved) { + return; + } + const origin = forbiddenImports.get(resolved); + if (!origin) { + return; + } + const pkg = forbiddenPackageByName.get(origin.package); + if (pkg?.allow.has(origin.importedName)) { + return; + } + context.report({ + node: reference.identifier, + messageId: 'forbiddenPackageUsage', + data: { + hookName, + importedName: origin.importedName, + package: origin.package, + }, + }); + }); + + scope.childScopes.forEach(child => visitScope(child, fn, hookName)); + } + + return { + ImportDeclaration(node) { + const source = node.source.value; + if (typeof source !== 'string' || !forbiddenPackageByName.has(source)) { + return; + } + + node.specifiers.forEach(specifier => { + let importedName: string; + switch (specifier.type) { + case AST_NODE_TYPES.ImportSpecifier: + importedName = + specifier.imported.type === AST_NODE_TYPES.Identifier + ? specifier.imported.name + : String(specifier.imported.value); + break; + case AST_NODE_TYPES.ImportDefaultSpecifier: + importedName = 'default'; + break; + case AST_NODE_TYPES.ImportNamespaceSpecifier: + importedName = '*'; + break; + default: + return; + } + for (const variable of sourceCode.getDeclaredVariables(specifier)) { + forbiddenImports.set(variable, { package: source, importedName }); + } + }); + }, + + [`FunctionDeclaration[id.name=/${BASE_HOOK_NAME_PATTERN.source}/]`]: (node: TSESTree.FunctionDeclaration) => { + if (!node.id) { + return; + } + checkBaseHook(node.id.name, node, node.id); + }, + + [`VariableDeclarator[id.name=/${BASE_HOOK_NAME_PATTERN.source}/]`]: (node: TSESTree.VariableDeclarator) => { + if (node.id.type !== AST_NODE_TYPES.Identifier) { + return; + } + const init = node.init; + if ( + !init || + (init.type !== AST_NODE_TYPES.ArrowFunctionExpression && init.type !== AST_NODE_TYPES.FunctionExpression) + ) { + return; + } + checkBaseHook(node.id.name, init, node.id); + }, + }; + }, +}); + +function normalizeForbiddenPackages(raw: ForbiddenPackageOption[] | undefined): NormalizedForbiddenPackage[] { + const source = raw ?? DEFAULT_FORBIDDEN_PACKAGES; + return source.map(entry => { + if (typeof entry === 'string') { + return { name: entry, allow: new Set() }; + } + return { name: entry.name, allow: new Set(entry.allow ?? []) }; + }); +} + +function describeParam(param: TSESTree.Parameter): string { + switch (param.type) { + case AST_NODE_TYPES.Identifier: + return param.name; + case AST_NODE_TYPES.ObjectPattern: + return '{ ... }'; + case AST_NODE_TYPES.ArrayPattern: + return '[ ... ]'; + case AST_NODE_TYPES.RestElement: + return '...rest'; + case AST_NODE_TYPES.AssignmentPattern: + return param.left.type === AST_NODE_TYPES.Identifier ? `${param.left.name} = …` : '… = …'; + default: + return param.type; + } +} + +function isReactRefTypeAnnotation(annotation: TSESTree.TSTypeAnnotation | undefined): boolean { + if (!annotation) { + return false; + } + const type = annotation.typeAnnotation; + if (type.type !== AST_NODE_TYPES.TSTypeReference) { + return false; + } + const { typeName } = type; + if (typeName.type === AST_NODE_TYPES.Identifier) { + return typeName.name === 'Ref'; + } + if (typeName.type === AST_NODE_TYPES.TSQualifiedName) { + return ( + typeName.left.type === AST_NODE_TYPES.Identifier && + typeName.left.name === 'React' && + typeName.right.name === 'Ref' + ); + } + return false; +} + +function describeRefType(annotation: TSESTree.TSTypeAnnotation | undefined): string { + if (!annotation) { + return ''; + } + const type = annotation.typeAnnotation; + if (type.type !== AST_NODE_TYPES.TSTypeReference) { + return type.type; + } + const { typeName } = type; + if (typeName.type === AST_NODE_TYPES.Identifier) { + return typeName.name; + } + if (typeName.type === AST_NODE_TYPES.TSQualifiedName) { + const left = typeName.left.type === AST_NODE_TYPES.Identifier ? typeName.left.name : '…'; + return `${left}.${typeName.right.name}`; + } + return type.type; +} + +function isScopeWithinFunction(scope: TSESLint.Scope.Scope, fn: BaseHookFunction): boolean { + let current: TSESLint.Scope.Scope | null = scope; + while (current) { + if (current.block === fn) { + return true; + } + current = current.upper; + } + return false; +}