From f297ab528ebe416e5814ab17bf4b30dcafda7c69 Mon Sep 17 00:00:00 2001 From: Martin Hochel Date: Thu, 21 May 2026 14:33:35 +0200 Subject: [PATCH 01/15] feat(eslint-rules): add consistent-base-hook rule Enforces the v9 base hook API contract for any function matching `useBase_unstable`: - 1 or 2 positional parameters; first must be named `props`, optional second must be named `ref` and typed as `React.Ref<...>`. - No references to bindings imported from configured forbidden packages (defaults to `@fluentui/react-tabster`, `tabster`, `keyborg`). Forbidden-package detection is scope-based, so local shadowing is handled correctly. Options: { forbiddenPackages?: Array } --- tools/eslint-rules/index.ts | 2 + .../rules/consistent-base-hook.spec.ts | 248 ++++++++++++++ .../rules/consistent-base-hook.ts | 311 ++++++++++++++++++ 3 files changed, 561 insertions(+) create mode 100644 tools/eslint-rules/rules/consistent-base-hook.spec.ts create mode 100644 tools/eslint-rules/rules/consistent-base-hook.ts diff --git a/tools/eslint-rules/index.ts b/tools/eslint-rules/index.ts index 6aeaae6c06da8..8e6f5eca37fd1 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 0000000000000..a07998f2005b5 --- /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'] }], + }, + ], + }, + // 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', + }, + }, + ], + }, + // Body uses a binding imported from keyborg. + { + code: ` + import { createKeyborg } from 'keyborg'; + export const useThingBase_unstable = (props, ref: React.Ref) => { + return { kb: createKeyborg(window) }; + }; + `, + errors: [ + { + messageId: 'forbiddenPackageUsage', + data: { + hookName: 'useThingBase_unstable', + importedName: 'createKeyborg', + package: 'keyborg', + }, + }, + ], + }, + // 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 0000000000000..76f0bb37733b6 --- /dev/null +++ b/tools/eslint-rules/rules/consistent-base-hook.ts @@ -0,0 +1,311 @@ +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 = ['@fluentui/react-tabster', 'tabster', 'keyborg']; + +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; +} From 2ca0c4200b372334bd5a4608f7ffdd2a4b583a50 Mon Sep 17 00:00:00 2001 From: Martin Hochel Date: Thu, 21 May 2026 14:33:42 +0200 Subject: [PATCH 02/15] chore(eslint-plugin): enable consistent-base-hook for v9 sources --- packages/eslint-plugin/src/internal.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/eslint-plugin/src/internal.js b/packages/eslint-plugin/src/internal.js index 02c31f5aa97d3..19afbe17a6217 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' }], }, From 2c2c0e98651421edaa25624dc3c202a0173cabd5 Mon Sep 17 00:00:00 2001 From: Martin Hochel Date: Thu, 21 May 2026 14:33:53 +0200 Subject: [PATCH 03/15] chore(react-components): suppress consistent-base-hook violations in legacy base hooks Pre-existing base hooks that don't yet conform to the new contract (missing/non-React.Ref-typed `ref` param, or references to tabster/keyborg bindings) are suppressed with line-scoped `eslint-disable-next-line` comments so the rule can ship as `error` for new code. Refactors to remove the suppressions will follow. Affected packages: - react-checkbox, react-menu, react-radio, react-rating, react-slider, react-switch, react-tags, react-tooltip --- .../library/src/components/Checkbox/useCheckbox.tsx | 1 + .../react-radio/library/src/components/Radio/useRadio.tsx | 1 + .../library/src/components/RatingItem/useRatingItem.tsx | 1 + .../react-slider/library/src/components/Slider/useSlider.ts | 1 + .../library/src/components/Switch/useSwitch.tsx | 1 + .../library/src/components/Tooltip/useTooltipBase.tsx | 6 +++++- 6 files changed, 10 insertions(+), 1 deletion(-) diff --git a/packages/react-components/react-checkbox/library/src/components/Checkbox/useCheckbox.tsx b/packages/react-components/react-checkbox/library/src/components/Checkbox/useCheckbox.tsx index 7a89ae277ab07..14dc123d449de 100644 --- a/packages/react-components/react-checkbox/library/src/components/Checkbox/useCheckbox.tsx +++ b/packages/react-components/react-checkbox/library/src/components/Checkbox/useCheckbox.tsx @@ -119,6 +119,7 @@ export const useCheckboxBase_unstable = ( }, root: slot.always(props.root, { defaultProps: { + // eslint-disable-next-line @nx/workspace-consistent-base-hook -- legacy: tabster usage should be moved out of base hook ref: useFocusWithin(), ...nativeProps.root, }, diff --git a/packages/react-components/react-radio/library/src/components/Radio/useRadio.tsx b/packages/react-components/react-radio/library/src/components/Radio/useRadio.tsx index e8d8cda21c01c..6c2af21413349 100644 --- a/packages/react-components/react-radio/library/src/components/Radio/useRadio.tsx +++ b/packages/react-components/react-radio/library/src/components/Radio/useRadio.tsx @@ -61,6 +61,7 @@ export const useRadioBase_unstable = (props: RadioBaseProps, ref: React.Ref(), ...nativeProps.root, }, diff --git a/packages/react-components/react-rating/library/src/components/RatingItem/useRatingItem.tsx b/packages/react-components/react-rating/library/src/components/RatingItem/useRatingItem.tsx index d67d28275be56..447bfa1e6179c 100644 --- a/packages/react-components/react-rating/library/src/components/RatingItem/useRatingItem.tsx +++ b/packages/react-components/react-rating/library/src/components/RatingItem/useRatingItem.tsx @@ -61,6 +61,7 @@ export const useRatingItemBase_unstable = ( const root = slot.always( getIntrinsicElementProps('span', { + // eslint-disable-next-line @nx/workspace-consistent-base-hook -- legacy: tabster usage should be moved out of base hook ref: useMergedRefs(useFocusWithin(), ref), ...props, }), diff --git a/packages/react-components/react-slider/library/src/components/Slider/useSlider.ts b/packages/react-components/react-slider/library/src/components/Slider/useSlider.ts index bdd105f21762c..652b1d6c83994 100644 --- a/packages/react-components/react-slider/library/src/components/Slider/useSlider.ts +++ b/packages/react-components/react-slider/library/src/components/Slider/useSlider.ts @@ -72,6 +72,7 @@ export const useSliderBase_unstable = (props: SliderBaseProps, ref: React.Ref()); useSliderState_unstable(state, props); diff --git a/packages/react-components/react-switch/library/src/components/Switch/useSwitch.tsx b/packages/react-components/react-switch/library/src/components/Switch/useSwitch.tsx index 79fc3262fdcb7..667d7107b1227 100644 --- a/packages/react-components/react-switch/library/src/components/Switch/useSwitch.tsx +++ b/packages/react-components/react-switch/library/src/components/Switch/useSwitch.tsx @@ -68,6 +68,7 @@ export const useSwitchBase_unstable = (props: SwitchBaseProps, ref?: React.Ref(), ...nativeProps.root }, elementType: 'div', }); 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 4919ea58156ac..9c7f99dd23516 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(); @@ -197,10 +197,12 @@ export const useTooltipBase_unstable = (props: TooltipBaseProps): TooltipBaseSta [setDelayTimeout, setVisible, state.showDelay, context], ); + // eslint-disable-next-line @nx/workspace-consistent-base-hook -- legacy: tabster usage should be moved out of base hook const isNavigatingWithKeyboard = useIsNavigatingWithKeyboard(); // Callback ref that attaches a keyborg:focusin event listener. const [keyborgListenerCallbackRef] = React.useState(() => { + // eslint-disable-next-line @nx/workspace-consistent-base-hook -- legacy: keyborg/tabster types used in base hook const onKeyborgFocusIn = ((ev: KeyborgFocusInEvent) => { // Skip showing the tooltip if focus moved programmatically. // For example, we don't want to show the tooltip when a dialog is closed @@ -216,7 +218,9 @@ export const useTooltipBase_unstable = (props: TooltipBaseProps): TooltipBaseSta // Callback ref that attaches the listener to the element return (element: Element | null) => { + // eslint-disable-next-line @nx/workspace-consistent-base-hook -- legacy: keyborg constant used in base hook current?.removeEventListener(KEYBORG_FOCUSIN, onKeyborgFocusIn); + // eslint-disable-next-line @nx/workspace-consistent-base-hook -- legacy: keyborg constant used in base hook element?.addEventListener(KEYBORG_FOCUSIN, onKeyborgFocusIn); current = element; }; From cbbd8c51453731cb0408337905d26c29f8e0dd5f Mon Sep 17 00:00:00 2001 From: Martin Hochel Date: Thu, 21 May 2026 15:42:54 +0200 Subject: [PATCH 04/15] feat(eslint-rules): allow useFocusWithin by default in consistent-base-hook `useFocusWithin` from `@fluentui/react-tabster` is a pure ref-attaching utility with no global side effects; it's safe to use inside base hooks. Added to the default allowlist so callers don't need to opt-out per-site. --- tools/eslint-rules/rules/consistent-base-hook.spec.ts | 9 +++++++++ tools/eslint-rules/rules/consistent-base-hook.ts | 6 +++++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/tools/eslint-rules/rules/consistent-base-hook.spec.ts b/tools/eslint-rules/rules/consistent-base-hook.spec.ts index a07998f2005b5..a9a6fcc166175 100644 --- a/tools/eslint-rules/rules/consistent-base-hook.spec.ts +++ b/tools/eslint-rules/rules/consistent-base-hook.spec.ts @@ -67,6 +67,15 @@ ruleTester.run(RULE_NAME, rule, { }, ], }, + // Default allowlist: `useFocusWithin` from `@fluentui/react-tabster` is permitted inside base hooks. + { + code: ` + import { useFocusWithin } from '@fluentui/react-tabster'; + export const useThingBase_unstable = (props, ref: React.Ref) => { + return { props, ref: useFocusWithin() }; + }; + `, + }, // Identifier with the same local name as a forbidden import alias does not collide via scope analysis. { code: ` diff --git a/tools/eslint-rules/rules/consistent-base-hook.ts b/tools/eslint-rules/rules/consistent-base-hook.ts index 76f0bb37733b6..0fa5fd2ec5a84 100644 --- a/tools/eslint-rules/rules/consistent-base-hook.ts +++ b/tools/eslint-rules/rules/consistent-base-hook.ts @@ -7,7 +7,11 @@ 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 = ['@fluentui/react-tabster', 'tabster', 'keyborg']; +const DEFAULT_FORBIDDEN_PACKAGES: ReadonlyArray = [ + { name: '@fluentui/react-tabster', allow: ['useFocusWithin'] }, + 'tabster', + 'keyborg', +]; type ForbiddenPackageOption = string | { name: string; allow?: string[] }; type Options = [ From a8ea39b3df06077ceab9a166eec3a1d9863fdb71 Mon Sep 17 00:00:00 2001 From: Martin Hochel Date: Thu, 21 May 2026 15:43:02 +0200 Subject: [PATCH 05/15] chore(react-components): drop now-unneeded consistent-base-hook suppressions `useFocusWithin` is now part of the rule's default allowlist, so the line-scoped suppressions in react-checkbox, react-radio, react-rating, react-slider, and react-switch are no longer needed. --- .../library/src/components/Checkbox/useCheckbox.tsx | 1 - .../react-radio/library/src/components/Radio/useRadio.tsx | 1 - .../library/src/components/RatingItem/useRatingItem.tsx | 1 - .../react-slider/library/src/components/Slider/useSlider.ts | 1 - .../react-switch/library/src/components/Switch/useSwitch.tsx | 1 - 5 files changed, 5 deletions(-) diff --git a/packages/react-components/react-checkbox/library/src/components/Checkbox/useCheckbox.tsx b/packages/react-components/react-checkbox/library/src/components/Checkbox/useCheckbox.tsx index 14dc123d449de..7a89ae277ab07 100644 --- a/packages/react-components/react-checkbox/library/src/components/Checkbox/useCheckbox.tsx +++ b/packages/react-components/react-checkbox/library/src/components/Checkbox/useCheckbox.tsx @@ -119,7 +119,6 @@ export const useCheckboxBase_unstable = ( }, root: slot.always(props.root, { defaultProps: { - // eslint-disable-next-line @nx/workspace-consistent-base-hook -- legacy: tabster usage should be moved out of base hook ref: useFocusWithin(), ...nativeProps.root, }, diff --git a/packages/react-components/react-radio/library/src/components/Radio/useRadio.tsx b/packages/react-components/react-radio/library/src/components/Radio/useRadio.tsx index 6c2af21413349..e8d8cda21c01c 100644 --- a/packages/react-components/react-radio/library/src/components/Radio/useRadio.tsx +++ b/packages/react-components/react-radio/library/src/components/Radio/useRadio.tsx @@ -61,7 +61,6 @@ export const useRadioBase_unstable = (props: RadioBaseProps, ref: React.Ref(), ...nativeProps.root, }, diff --git a/packages/react-components/react-rating/library/src/components/RatingItem/useRatingItem.tsx b/packages/react-components/react-rating/library/src/components/RatingItem/useRatingItem.tsx index 447bfa1e6179c..d67d28275be56 100644 --- a/packages/react-components/react-rating/library/src/components/RatingItem/useRatingItem.tsx +++ b/packages/react-components/react-rating/library/src/components/RatingItem/useRatingItem.tsx @@ -61,7 +61,6 @@ export const useRatingItemBase_unstable = ( const root = slot.always( getIntrinsicElementProps('span', { - // eslint-disable-next-line @nx/workspace-consistent-base-hook -- legacy: tabster usage should be moved out of base hook ref: useMergedRefs(useFocusWithin(), ref), ...props, }), diff --git a/packages/react-components/react-slider/library/src/components/Slider/useSlider.ts b/packages/react-components/react-slider/library/src/components/Slider/useSlider.ts index 652b1d6c83994..bdd105f21762c 100644 --- a/packages/react-components/react-slider/library/src/components/Slider/useSlider.ts +++ b/packages/react-components/react-slider/library/src/components/Slider/useSlider.ts @@ -72,7 +72,6 @@ export const useSliderBase_unstable = (props: SliderBaseProps, ref: React.Ref()); useSliderState_unstable(state, props); diff --git a/packages/react-components/react-switch/library/src/components/Switch/useSwitch.tsx b/packages/react-components/react-switch/library/src/components/Switch/useSwitch.tsx index 667d7107b1227..79fc3262fdcb7 100644 --- a/packages/react-components/react-switch/library/src/components/Switch/useSwitch.tsx +++ b/packages/react-components/react-switch/library/src/components/Switch/useSwitch.tsx @@ -68,7 +68,6 @@ export const useSwitchBase_unstable = (props: SwitchBaseProps, ref?: React.Ref(), ...nativeProps.root }, elementType: 'div', }); From 153a6360caba8c1bafe1477012c26fe4491554bc Mon Sep 17 00:00:00 2001 From: Martin Hochel Date: Thu, 21 May 2026 15:50:32 +0200 Subject: [PATCH 06/15] feat(eslint-rules): allow useFocusVisible by default in consistent-base-hook Same rationale as `useFocusWithin`: `useFocusVisible` from `@fluentui/react-tabster` is safe to use inside base hooks. --- tools/eslint-rules/rules/consistent-base-hook.spec.ts | 5 +++-- tools/eslint-rules/rules/consistent-base-hook.ts | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/tools/eslint-rules/rules/consistent-base-hook.spec.ts b/tools/eslint-rules/rules/consistent-base-hook.spec.ts index a9a6fcc166175..fb6b4318d49f8 100644 --- a/tools/eslint-rules/rules/consistent-base-hook.spec.ts +++ b/tools/eslint-rules/rules/consistent-base-hook.spec.ts @@ -67,11 +67,12 @@ ruleTester.run(RULE_NAME, rule, { }, ], }, - // Default allowlist: `useFocusWithin` from `@fluentui/react-tabster` is permitted inside base hooks. + // Default allowlist: `useFocusWithin` and `useFocusVisible` from `@fluentui/react-tabster` are permitted inside base hooks. { code: ` - import { useFocusWithin } from '@fluentui/react-tabster'; + import { useFocusWithin, useFocusVisible } from '@fluentui/react-tabster'; export const useThingBase_unstable = (props, ref: React.Ref) => { + useFocusVisible(); return { props, ref: useFocusWithin() }; }; `, diff --git a/tools/eslint-rules/rules/consistent-base-hook.ts b/tools/eslint-rules/rules/consistent-base-hook.ts index 0fa5fd2ec5a84..ae44534fc663b 100644 --- a/tools/eslint-rules/rules/consistent-base-hook.ts +++ b/tools/eslint-rules/rules/consistent-base-hook.ts @@ -8,7 +8,7 @@ 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', allow: ['useFocusWithin'] }, + { name: '@fluentui/react-tabster', allow: ['useFocusWithin', 'useFocusVisible'] }, 'tabster', 'keyborg', ]; From 3504c3a293c1f8b3b808cf9171b325daa369d7a3 Mon Sep 17 00:00:00 2001 From: Martin Hochel Date: Thu, 21 May 2026 19:21:53 +0200 Subject: [PATCH 07/15] feat(eslint-rules): allow keyborg-only react-tabster APIs and drop keyborg from forbidden list - Remove `keyborg` from the default forbidden packages list. Base hooks may freely depend on `keyborg` since it carries no tabster runtime. - Expand the default allowlist for `@fluentui/react-tabster` with APIs that internally depend only on `keyborg` (no `tabster` runtime): - useKeyboardNavAttribute - useIsNavigatingWithKeyboard - useSetKeyboardNavigation - useOnKeyboardNavigationChange - applyFocusVisiblePolyfill - KEYBORG_FOCUSIN / KeyborgFocusInEvent (re-exports from `keyborg`) --- .../rules/consistent-base-hook.spec.ts | 28 ++++++------------- .../rules/consistent-base-hook.ts | 18 ++++++++++-- 2 files changed, 25 insertions(+), 21 deletions(-) diff --git a/tools/eslint-rules/rules/consistent-base-hook.spec.ts b/tools/eslint-rules/rules/consistent-base-hook.spec.ts index fb6b4318d49f8..c15112c95fb9f 100644 --- a/tools/eslint-rules/rules/consistent-base-hook.spec.ts +++ b/tools/eslint-rules/rules/consistent-base-hook.spec.ts @@ -77,6 +77,15 @@ ruleTester.run(RULE_NAME, rule, { }; `, }, + // `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: ` @@ -209,25 +218,6 @@ ruleTester.run(RULE_NAME, rule, { }, ], }, - // Body uses a binding imported from keyborg. - { - code: ` - import { createKeyborg } from 'keyborg'; - export const useThingBase_unstable = (props, ref: React.Ref) => { - return { kb: createKeyborg(window) }; - }; - `, - errors: [ - { - messageId: 'forbiddenPackageUsage', - data: { - hookName: 'useThingBase_unstable', - importedName: 'createKeyborg', - package: 'keyborg', - }, - }, - ], - }, // Allowlist excludes one name; siblings still error. { code: ` diff --git a/tools/eslint-rules/rules/consistent-base-hook.ts b/tools/eslint-rules/rules/consistent-base-hook.ts index ae44534fc663b..55dde83ad1acb 100644 --- a/tools/eslint-rules/rules/consistent-base-hook.ts +++ b/tools/eslint-rules/rules/consistent-base-hook.ts @@ -8,9 +8,23 @@ 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', allow: ['useFocusWithin', 'useFocusVisible'] }, + { + 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', - 'keyborg', ]; type ForbiddenPackageOption = string | { name: string; allow?: string[] }; From 578c7daea4b38690e589c29c2072c4aaf691dbcc Mon Sep 17 00:00:00 2001 From: Martin Hochel Date: Thu, 21 May 2026 19:21:55 +0200 Subject: [PATCH 08/15] chore(react-tooltip): drop now-unneeded consistent-base-hook suppressions `useIsNavigatingWithKeyboard`, `KEYBORG_FOCUSIN`, and `KeyborgFocusInEvent` are now part of the rule's default allowlist. --- .../library/src/components/Tooltip/useTooltipBase.tsx | 4 ---- 1 file changed, 4 deletions(-) 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 9c7f99dd23516..30d586b5e79b4 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 @@ -197,12 +197,10 @@ export const useTooltipBase_unstable = (props: TooltipBaseProps): TooltipBaseSta [setDelayTimeout, setVisible, state.showDelay, context], ); - // eslint-disable-next-line @nx/workspace-consistent-base-hook -- legacy: tabster usage should be moved out of base hook const isNavigatingWithKeyboard = useIsNavigatingWithKeyboard(); // Callback ref that attaches a keyborg:focusin event listener. const [keyborgListenerCallbackRef] = React.useState(() => { - // eslint-disable-next-line @nx/workspace-consistent-base-hook -- legacy: keyborg/tabster types used in base hook const onKeyborgFocusIn = ((ev: KeyborgFocusInEvent) => { // Skip showing the tooltip if focus moved programmatically. // For example, we don't want to show the tooltip when a dialog is closed @@ -218,9 +216,7 @@ export const useTooltipBase_unstable = (props: TooltipBaseProps): TooltipBaseSta // Callback ref that attaches the listener to the element return (element: Element | null) => { - // eslint-disable-next-line @nx/workspace-consistent-base-hook -- legacy: keyborg constant used in base hook current?.removeEventListener(KEYBORG_FOCUSIN, onKeyborgFocusIn); - // eslint-disable-next-line @nx/workspace-consistent-base-hook -- legacy: keyborg constant used in base hook element?.addEventListener(KEYBORG_FOCUSIN, onKeyborgFocusIn); current = element; }; From 338183aabb2608d6996ec4b57818fbdb51248063 Mon Sep 17 00:00:00 2001 From: Martin Hochel Date: Mon, 25 May 2026 10:42:04 +0200 Subject: [PATCH 09/15] feat(eslint-rules): auto-detect forbidden runtime deps via TS Program in consistent-base-hook Replace the hand-curated allow/forbidden package lists with per-symbol transitive value-import analysis powered by typescript-eslint ParserServices and the TypeScript Program. The rule now traces each base-hook import back to its defining source file and walks non-type-only re-exports/imports to detect whether the symbol's module graph reaches any configured forbidden runtime package. Options shape: { watchedPackages?: string[]; forbiddenRuntimes?: string[] }. Defaults: watchedPackages=['@fluentui/react-tabster'], forbiddenRuntimes=['tabster']. New message ids: forbiddenRuntimeDirect, forbiddenRuntimeReach. Type-only imports are skipped. When typed services are unavailable, only direct forbidden-runtime imports are reported. --- .../consistent-base-hook/src/dummy.ts | 4 + .../consistent-base-hook/src/test.ts | 4 + .../stubs/cyclic-pkg/a.ts | 5 + .../stubs/cyclic-pkg/b.ts | 6 + .../stubs/cyclic-pkg/index.ts | 3 + .../stubs/heavy-runtime/index.ts | 3 + .../stubs/light-helper/index.ts | 3 + .../stubs/watched-pkg/heavy.ts | 7 + .../stubs/watched-pkg/index.ts | 5 + .../stubs/watched-pkg/light.ts | 7 + .../consistent-base-hook/tsconfig.json | 21 + .../rules/consistent-base-hook.spec.ts | 232 +++++---- .../rules/consistent-base-hook.ts | 461 ++++++++++++++---- 13 files changed, 585 insertions(+), 176 deletions(-) create mode 100644 tools/eslint-rules/rules/__fixtures__/consistent-base-hook/src/dummy.ts create mode 100644 tools/eslint-rules/rules/__fixtures__/consistent-base-hook/src/test.ts create mode 100644 tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/cyclic-pkg/a.ts create mode 100644 tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/cyclic-pkg/b.ts create mode 100644 tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/cyclic-pkg/index.ts create mode 100644 tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/heavy-runtime/index.ts create mode 100644 tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/light-helper/index.ts create mode 100644 tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/watched-pkg/heavy.ts create mode 100644 tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/watched-pkg/index.ts create mode 100644 tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/watched-pkg/light.ts create mode 100644 tools/eslint-rules/rules/__fixtures__/consistent-base-hook/tsconfig.json diff --git a/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/src/dummy.ts b/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/src/dummy.ts new file mode 100644 index 0000000000000..73f11d7b5c58a --- /dev/null +++ b/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/src/dummy.ts @@ -0,0 +1,4 @@ +// Placeholder so the tsconfig has an actual file to anchor the project. +// RuleTester test cases reference filenames inside this directory but their +// content comes from the inline `code` field. +export const dummy = 1; diff --git a/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/src/test.ts b/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/src/test.ts new file mode 100644 index 0000000000000..1e866fcbc8322 --- /dev/null +++ b/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/src/test.ts @@ -0,0 +1,4 @@ +// Placeholder anchor for typed RuleTester cases. The actual code being linted +// comes from each test's inline `code` field; this file just needs to exist so +// that the fixture tsconfig's Program contains the filename used by tests. +export {}; diff --git a/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/cyclic-pkg/a.ts b/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/cyclic-pkg/a.ts new file mode 100644 index 0000000000000..12e940cc7f1fd --- /dev/null +++ b/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/cyclic-pkg/a.ts @@ -0,0 +1,5 @@ +import { useB } from './b'; + +export function useA(): number { + return useB() + 1; +} diff --git a/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/cyclic-pkg/b.ts b/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/cyclic-pkg/b.ts new file mode 100644 index 0000000000000..777b389e45df5 --- /dev/null +++ b/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/cyclic-pkg/b.ts @@ -0,0 +1,6 @@ +import { useA } from './a'; + +export function useB(): number { + // Pretend lazy ref to break true cycle at runtime; the static graph is cyclic. + return (useA as unknown as () => number).length; +} diff --git a/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/cyclic-pkg/index.ts b/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/cyclic-pkg/index.ts new file mode 100644 index 0000000000000..81c4ddc44291b --- /dev/null +++ b/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/cyclic-pkg/index.ts @@ -0,0 +1,3 @@ +// Cyclic re-export — exercises the cycle-safety of the transitive walk. +export { useA } from './a'; +export { useB } from './b'; diff --git a/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/heavy-runtime/index.ts b/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/heavy-runtime/index.ts new file mode 100644 index 0000000000000..5fc7a81d62453 --- /dev/null +++ b/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/heavy-runtime/index.ts @@ -0,0 +1,3 @@ +export function runHeavy(): { tag: 'heavy' } { + return { tag: 'heavy' }; +} diff --git a/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/light-helper/index.ts b/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/light-helper/index.ts new file mode 100644 index 0000000000000..9ab573b97e25e --- /dev/null +++ b/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/light-helper/index.ts @@ -0,0 +1,3 @@ +export function runLight(_opts?: { mode: 'light' }): void { + /* noop */ +} diff --git a/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/watched-pkg/heavy.ts b/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/watched-pkg/heavy.ts new file mode 100644 index 0000000000000..9798a4b8d6b35 --- /dev/null +++ b/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/watched-pkg/heavy.ts @@ -0,0 +1,7 @@ +import { runHeavy } from 'heavy-runtime'; + +export type HeavyType = { tag: 'heavy' }; + +export function useHeavy(): HeavyType { + return runHeavy(); +} diff --git a/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/watched-pkg/index.ts b/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/watched-pkg/index.ts new file mode 100644 index 0000000000000..f20537a7b393d --- /dev/null +++ b/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/watched-pkg/index.ts @@ -0,0 +1,5 @@ +export { useHeavy } from './heavy'; +export { useLight } from './light'; +export type { LightOptions } from './light'; +// Re-export of a type-only thing from the heavy module — must not count as a runtime reach. +export type { HeavyType } from './heavy'; diff --git a/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/watched-pkg/light.ts b/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/watched-pkg/light.ts new file mode 100644 index 0000000000000..970deab9edbbd --- /dev/null +++ b/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/watched-pkg/light.ts @@ -0,0 +1,7 @@ +import { runLight } from 'light-helper'; + +export type LightOptions = { mode: 'light' }; + +export function useLight(opts?: LightOptions): void { + runLight(opts); +} diff --git a/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/tsconfig.json b/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/tsconfig.json new file mode 100644 index 0000000000000..432b9967e725a --- /dev/null +++ b/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/tsconfig.json @@ -0,0 +1,21 @@ +{ + "compilerOptions": { + "target": "ES2022", + "module": "ESNext", + "moduleResolution": "node", + "lib": ["ES2022", "DOM"], + "strict": true, + "esModuleInterop": true, + "skipLibCheck": true, + "noEmit": true, + "baseUrl": ".", + "paths": { + "watched-pkg": ["./stubs/watched-pkg/index.ts"], + "watched-pkg/*": ["./stubs/watched-pkg/*"], + "heavy-runtime": ["./stubs/heavy-runtime/index.ts"], + "light-helper": ["./stubs/light-helper/index.ts"], + "cyclic-pkg": ["./stubs/cyclic-pkg/index.ts"] + } + }, + "include": ["src/**/*.ts", "src/**/*.tsx", "stubs/**/*.ts"] +} diff --git a/tools/eslint-rules/rules/consistent-base-hook.spec.ts b/tools/eslint-rules/rules/consistent-base-hook.spec.ts index c15112c95fb9f..0069ed215cadc 100644 --- a/tools/eslint-rules/rules/consistent-base-hook.spec.ts +++ b/tools/eslint-rules/rules/consistent-base-hook.spec.ts @@ -1,6 +1,20 @@ +import * as path from 'node:path'; import { RuleTester } from '@typescript-eslint/rule-tester'; import { rule, RULE_NAME } from './consistent-base-hook'; +const FIXTURE_ROOT = path.join(__dirname, '__fixtures__/consistent-base-hook'); +const TYPED_FILENAME = 'src/test.ts'; + +const typedLanguageOptions = { + parserOptions: { + project: path.join(FIXTURE_ROOT, 'tsconfig.json'), + tsconfigRootDir: FIXTURE_ROOT, + }, +}; + +// --------------------------------------------------------------------------- +// Parameter-shape checks — no typed linting required. +// --------------------------------------------------------------------------- const ruleTester = new RuleTester(); ruleTester.run(RULE_NAME, rule, { @@ -23,7 +37,7 @@ ruleTester.run(RULE_NAME, rule, { } `, }, - // Valid base hook with only `props` (ref is optional). + // Valid base hook with only \`props\` (ref is optional). { code: ` export const useThingBase_unstable = (props) => { @@ -31,19 +45,6 @@ ruleTester.run(RULE_NAME, rule, { }; `, }, - // 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: ` @@ -52,32 +53,20 @@ ruleTester.run(RULE_NAME, rule, { }; `, }, - // Allowlist opt-out: a specific imported name is permitted inside base hooks. + // Identifier with the same local name as a forbidden import alias does not collide via scope analysis. { code: ` - import { useFocusFinders } from '@fluentui/react-tabster'; - export const useThingBase_unstable = (props, ref: React.Ref) => { - const finders = useFocusFinders(); - return { props, ref, finders }; + import { useArrowNavigationGroup } from '@fluentui/react-tabster'; + export const useThing_unstable = (props, ref) => { + return useArrowNavigationGroup({}); }; - `, - 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() }; + export const useThingBase_unstable = (props, ref: React.Ref) => { + const useArrowNavigationGroup = () => 1; + return { value: useArrowNavigationGroup() }; }; `, }, - // `keyborg` is not forbidden by default — bindings imported from it are allowed inside base hooks. + // \`keyborg\` is not in the default forbidden runtime list — bindings imported from it are allowed inside base hooks. { code: ` import { createKeyborg, KEYBORG_FOCUSIN } from 'keyborg'; @@ -86,19 +75,6 @@ ruleTester.run(RULE_NAME, rule, { }; `, }, - // 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). @@ -131,7 +107,7 @@ ruleTester.run(RULE_NAME, rule, { }, ], }, - // ObjectPattern for `props` is not allowed. + // ObjectPattern for \`props\` is not allowed. { code: ` export const useThingBase_unstable = ({ a }, ref: React.Ref) => ({ a, ref }); @@ -143,7 +119,7 @@ ruleTester.run(RULE_NAME, rule, { }, ], }, - // `ref` parameter without a type annotation. + // \`ref\` parameter without a type annotation. { code: ` export const useThingBase_unstable = (props, ref) => ({ props, ref }); @@ -155,7 +131,7 @@ ruleTester.run(RULE_NAME, rule, { }, ], }, - // `ref` parameter typed as something other than React.Ref. + // \`ref\` parameter typed as something other than React.Ref. { code: ` export const useThingBase_unstable = (props, ref: HTMLElement) => ({ props, ref }); @@ -167,7 +143,7 @@ ruleTester.run(RULE_NAME, rule, { }, ], }, - // `ref` parameter typed as React.ForwardedRef (must be React.Ref). + // \`ref\` parameter typed as React.ForwardedRef (must be React.Ref). { code: ` export const useThingBase_unstable = (props, ref: React.ForwardedRef) => ({ props, ref }); @@ -179,67 +155,159 @@ ruleTester.run(RULE_NAME, rule, { }, ], }, - // Body uses a binding imported from @fluentui/react-tabster. + ], +}); + +// --------------------------------------------------------------------------- +// Forbidden-runtime + transitive-reach checks — require typed linting. +// --------------------------------------------------------------------------- +const typedRuleTester = new RuleTester(); + +const transitiveOptions: readonly [{ watchedPackages: string[]; forbiddenRuntimes: string[] }] = [ + { + watchedPackages: ['watched-pkg'], + forbiddenRuntimes: ['heavy-runtime'], + }, +]; + +typedRuleTester.run(`${RULE_NAME} (typed)`, rule, { + valid: [ + // The defining file of \`useLight\` only reaches \`light-helper\`, not \`heavy-runtime\`. { + languageOptions: typedLanguageOptions, + filename: TYPED_FILENAME, + options: transitiveOptions, code: ` - import { useArrowNavigationGroup } from '@fluentui/react-tabster'; - export const useThingBase_unstable = (props, ref: React.Ref) => { - const nav = useArrowNavigationGroup({}); - return { props, ref, nav }; + import { useLight } from 'watched-pkg'; + export const useThingBase_unstable = (props: { a: number }, ref: React.Ref) => { + useLight(); + return { props, ref }; }; `, - errors: [ + }, + // Type-only import of a symbol whose defining file reaches the forbidden runtime is allowed. + { + languageOptions: typedLanguageOptions, + filename: TYPED_FILENAME, + options: transitiveOptions, + code: ` + import type { HeavyType } from 'watched-pkg'; + export const useThingBase_unstable = (props: HeavyType, ref: React.Ref) => { + return { props, ref }; + }; + `, + }, + // Per-specifier type-only import is also OK. + { + languageOptions: typedLanguageOptions, + filename: TYPED_FILENAME, + options: transitiveOptions, + code: ` + import { type HeavyType, useLight } from 'watched-pkg'; + export const useThingBase_unstable = (props: HeavyType, ref: React.Ref) => { + useLight(); + return { props, ref }; + }; + `, + }, + // Watched-package import exists but only used by a non-base hook in the same file. + { + languageOptions: typedLanguageOptions, + filename: TYPED_FILENAME, + options: transitiveOptions, + code: ` + import { useHeavy } from 'watched-pkg'; + export const useThingBase_unstable = (props: { a: number }, ref: React.Ref) => { + return { props, ref }; + }; + export const useThing_unstable = (props, ref) => { + return useHeavy(); + }; + `, + }, + // Cyclic re-export graph must not infinite-loop; \`useA\` does not reach heavy-runtime. + { + languageOptions: typedLanguageOptions, + filename: TYPED_FILENAME, + options: [ { - messageId: 'forbiddenPackageUsage', - data: { - hookName: 'useThingBase_unstable', - importedName: 'useArrowNavigationGroup', - package: '@fluentui/react-tabster', - }, + watchedPackages: ['cyclic-pkg'], + forbiddenRuntimes: ['heavy-runtime'], }, ], + code: ` + import { useA } from 'cyclic-pkg'; + export const useThingBase_unstable = (props: { a: number }, ref: React.Ref) => { + return { props, ref, value: useA() }; + }; + `, }, - // Body uses a binding imported from tabster, even when aliased. + ], + invalid: [ + // Direct import from a forbidden-runtime package. { + languageOptions: typedLanguageOptions, + filename: TYPED_FILENAME, + options: transitiveOptions, code: ` - import { getTabsterAttribute as gta } from 'tabster'; - export function useThingBase_unstable(props, ref: React.Ref) { - return { attr: gta({}) }; - } + import { runHeavy } from 'heavy-runtime'; + export const useThingBase_unstable = (props: { a: number }, ref: React.Ref) => { + return { props, ref, x: runHeavy() }; + }; `, errors: [ { - messageId: 'forbiddenPackageUsage', + messageId: 'forbiddenRuntimeDirect', data: { hookName: 'useThingBase_unstable', - importedName: 'getTabsterAttribute', - package: 'tabster', + importedName: 'runHeavy', + package: 'heavy-runtime', }, }, ], }, - // Allowlist excludes one name; siblings still error. + // Symbol from watched package whose defining file transitively imports the forbidden runtime. { + languageOptions: typedLanguageOptions, + filename: TYPED_FILENAME, + options: transitiveOptions, 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 }; + import { useHeavy } from 'watched-pkg'; + export const useThingBase_unstable = (props: { a: number }, ref: React.Ref) => { + return { props, ref, x: useHeavy() }; }; `, - options: [ + errors: [ { - forbiddenPackages: [{ name: '@fluentui/react-tabster', allow: ['useFocusFinders'] }], + messageId: 'forbiddenRuntimeReach', + data: { + hookName: 'useThingBase_unstable', + importedName: 'useHeavy', + package: 'watched-pkg', + runtime: 'heavy-runtime', + viaFile: 'rules/__fixtures__/consistent-base-hook/stubs/watched-pkg/heavy.ts', + }, }, ], + }, + // Aliased import from a forbidden-runtime package is still flagged on the alias use site. + { + languageOptions: typedLanguageOptions, + filename: TYPED_FILENAME, + options: transitiveOptions, + code: ` + import { runHeavy as go } from 'heavy-runtime'; + export function useThingBase_unstable(props: { a: number }, ref: React.Ref) { + return go(); + } + `, errors: [ { - messageId: 'forbiddenPackageUsage', + messageId: 'forbiddenRuntimeDirect', data: { hookName: 'useThingBase_unstable', - importedName: 'useArrowNavigationGroup', - package: '@fluentui/react-tabster', + importedName: 'runHeavy', + package: 'heavy-runtime', }, }, ], diff --git a/tools/eslint-rules/rules/consistent-base-hook.ts b/tools/eslint-rules/rules/consistent-base-hook.ts index 55dde83ad1acb..c08226d808958 100644 --- a/tools/eslint-rules/rules/consistent-base-hook.ts +++ b/tools/eslint-rules/rules/consistent-base-hook.ts @@ -1,4 +1,11 @@ -import { ESLintUtils, AST_NODE_TYPES, TSESTree, TSESLint } from '@typescript-eslint/utils'; +import { + ESLintUtils, + AST_NODE_TYPES, + TSESTree, + TSESLint, + ParserServicesWithTypeInformation, +} from '@typescript-eslint/utils'; +import * as ts from 'typescript'; // NOTE: The rule will be available in ESLint configs as "@nx/workspace-consistent-base-hook" export const RULE_NAME = 'consistent-base-hook'; @@ -7,79 +14,83 @@ 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[] }; +const DEFAULT_WATCHED_PACKAGES: ReadonlyArray = ['@fluentui/react-tabster']; +const DEFAULT_FORBIDDEN_RUNTIMES: ReadonlyArray = ['tabster']; + type Options = [ { - forbiddenPackages?: ForbiddenPackageOption[]; + /** + * Packages whose imported symbols must be analyzed transitively. + * A symbol imported from one of these packages is allowed inside a base + * hook only if its defining source file does not reach any + * `forbiddenRuntimes` package via value imports. + */ + watchedPackages?: string[]; + /** + * Runtime packages whose presence in the transitive value-import graph of + * a referenced symbol is forbidden inside base hooks. Direct imports from + * these packages are also forbidden. + */ + forbiddenRuntimes?: string[]; }?, ]; -type MessageIds = 'invalidParamCount' | 'invalidParamName' | 'invalidRefType' | 'forbiddenPackageUsage'; -interface NormalizedForbiddenPackage { - name: string; - allow: Set; -} +type MessageIds = + | 'invalidParamCount' + | 'invalidParamName' + | 'invalidRefType' + | 'forbiddenRuntimeDirect' + | 'forbiddenRuntimeReach'; + +type ImportSpecifierNode = + | TSESTree.ImportSpecifier + | TSESTree.ImportDefaultSpecifier + | TSESTree.ImportNamespaceSpecifier; -interface ForbiddenImport { +interface TrackedImport { + /** The package the binding came from (a watched OR forbidden-runtime package). */ package: string; + /** Original imported name (not the local alias). `default` or `*` for default / namespace. */ importedName: string; + /** Kind of package — controls how the reference is checked. */ + kind: 'watched' | 'forbidden'; + /** The specifier node (used for symbol lookup via ParserServices). */ + specifier: ImportSpecifierNode; } type BaseHookFunction = TSESTree.FunctionDeclaration | TSESTree.FunctionExpression | TSESTree.ArrowFunctionExpression; +/** + * Per-Program cache: source file path → set of bare package specifiers transitively reached + * via value (non type-only) imports starting from that file. + * + * Keyed by `ts.Program` identity so the cache is invalidated whenever + * typescript-eslint rebuilds the Program. + */ +const programCache = new WeakMap>>(); + 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`).', + 'Enforce the API contract for v9 "base" hooks (`useBase_unstable`): a required `props` parameter and an optional `ref` parameter typed as `React.Ref<...>`, and disallow referencing any binding whose defining module transitively pulls a forbidden runtime package (default `tabster`).', }, schema: [ { type: 'object', properties: { - forbiddenPackages: { + watchedPackages: { 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, + items: { type: 'string' }, + uniqueItems: true, + }, + forbiddenRuntimes: { + type: 'array', + items: { type: 'string' }, + uniqueItems: true, }, }, additionalProperties: false, @@ -91,20 +102,37 @@ export const rule = ESLintUtils.RuleCreator(() => __filename)`, 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.', + forbiddenRuntimeDirect: + 'Base hook `{{hookName}}` cannot reference `{{importedName}}` from forbidden runtime package `{{package}}`. Move logic that depends on `{{package}}` to the wrapping `*_unstable` hook instead.', + forbiddenRuntimeReach: + 'Base hook `{{hookName}}` cannot reference `{{importedName}}` from `{{package}}` because its defining module transitively imports forbidden runtime `{{runtime}}` (via `{{viaFile}}`). Move logic that depends on `{{runtime}}` 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])); + const watchedPackages = new Set(options.watchedPackages ?? DEFAULT_WATCHED_PACKAGES); + const forbiddenRuntimes = new Set(options.forbiddenRuntimes ?? DEFAULT_FORBIDDEN_RUNTIMES); + // `forbidden` takes precedence: if the same name appears in both lists, treat the binding as forbidden. + const trackedPackages = new Set([...watchedPackages, ...forbiddenRuntimes]); + + // Variable → import origin. Keyed by Variable identity so shadowing inside the base hook is handled. + const trackedImports = new Map(); - // 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(); + // Lazily-acquired typed services. Untyped lint silently skips transitive analysis. + let typedServices: ParserServicesWithTypeInformation | null | undefined; + function getTypedServices(): ParserServicesWithTypeInformation | null { + if (typedServices !== undefined) { + return typedServices; + } + try { + typedServices = ESLintUtils.getParserServices(context); + } catch { + typedServices = null; + } + return typedServices; + } function checkBaseHook(hookName: string, fn: BaseHookFunction, reportNode: TSESTree.Node): void { checkParameters(hookName, fn, reportNode); @@ -116,10 +144,7 @@ export const rule = ESLintUtils.RuleCreator(() => __filename) __filename) __filename) __filename) visitScope(child, fn, hookName)); } + function computeSymbolReach(origin: TrackedImport): { reached: ReadonlySet; viaFile: string } | null { + const services = getTypedServices(); + if (!services) { + return null; + } + const checker = services.program.getTypeChecker(); + const tsNode = services.esTreeNodeToTSNodeMap.get(origin.specifier); + if (!tsNode) { + return null; + } + + // For an ImportSpecifier we want the imported (right-hand) identifier so the symbol resolves to + // the exported name on the source module, not the local alias. + let nameNode: ts.Node | undefined; + if (ts.isImportSpecifier(tsNode)) { + nameNode = tsNode.propertyName ?? tsNode.name; + } else if (ts.isImportClause(tsNode) || ts.isNamespaceImport(tsNode)) { + nameNode = tsNode.name; + } else { + nameNode = tsNode; + } + if (!nameNode) { + return null; + } + + let symbol = checker.getSymbolAtLocation(nameNode); + if (!symbol) { + return null; + } + if (symbol.flags & ts.SymbolFlags.Alias) { + try { + symbol = checker.getAliasedSymbol(symbol); + } catch { + return null; + } + } + const decl = symbol.declarations?.[0]; + const sf = decl?.getSourceFile(); + if (!sf) { + return null; + } + + const reached = transitiveValuePackages(services.program, sf); + return { reached, viaFile: shortenPath(sf.fileName) }; + } + return { ImportDeclaration(node) { const source = node.source.value; - if (typeof source !== 'string' || !forbiddenPackageByName.has(source)) { + if (typeof source !== 'string' || !trackedPackages.has(source)) { return; } + // Purely type-only imports cannot pull runtime. + if (node.importKind === 'type') { + return; + } + const kind: TrackedImport['kind'] = forbiddenRuntimes.has(source) ? 'forbidden' : 'watched'; node.specifiers.forEach(specifier => { + // Per-specifier type-only imports are also runtime-free. + if (specifier.type === AST_NODE_TYPES.ImportSpecifier && specifier.importKind === 'type') { + return; + } let importedName: string; switch (specifier.type) { case AST_NODE_TYPES.ImportSpecifier: @@ -220,7 +313,7 @@ export const rule = ESLintUtils.RuleCreator(() => __filename) __filename) { - if (typeof entry === 'string') { - return { name: entry, allow: new Set() }; +// --------------------------------------------------------------------------- +// Transitive value-import analysis +// --------------------------------------------------------------------------- + +/** + * Returns the set of bare package specifiers transitively reachable from `sf` + * via non-type-only imports. Memoized per Program × file. Cycle-safe. + */ +function transitiveValuePackages(program: ts.Program, sf: ts.SourceFile): ReadonlySet { + let cache = programCache.get(program); + if (!cache) { + cache = new Map(); + programCache.set(program, cache); + } + return computeReach(program, sf, cache, new Set()); +} + +function computeReach( + program: ts.Program, + sf: ts.SourceFile, + cache: Map>, + inProgress: Set, +): ReadonlySet { + const cached = cache.get(sf.fileName); + if (cached) { + return cached; + } + if (inProgress.has(sf.fileName)) { + // Cycle: return an empty set without caching so the eventual full result is committed by the originator. + return EMPTY_SET; + } + inProgress.add(sf.fileName); + + const result = new Set(); + for (const { specifier, literal } of collectValueImports(sf)) { + if (isBareSpecifier(specifier)) { + result.add(packageNameOf(specifier)); + } + const resolved = resolveModule(program, sf, specifier, literal); + if (!resolved) { + continue; + } + const childSf = program.getSourceFile(resolved); + if (!childSf) { + continue; + } + const childReach = computeReach(program, childSf, cache, inProgress); + for (const pkg of childReach) { + result.add(pkg); + } + } + + inProgress.delete(sf.fileName); + cache.set(sf.fileName, result); + return result; +} + +const EMPTY_SET: ReadonlySet = new Set(); + +interface ValueImport { + specifier: string; + literal: ts.StringLiteralLike; +} + +function collectValueImports(sf: ts.SourceFile): ValueImport[] { + const result: ValueImport[] = []; + for (const stmt of sf.statements) { + if (ts.isImportDeclaration(stmt)) { + if (stmt.importClause?.isTypeOnly) { + continue; + } + // `import './side-effect'` (no clause) IS a value import. + // For named imports, drop if every specifier is type-only AND there's no default/namespace. + if (stmt.importClause && stmt.importClause.namedBindings && ts.isNamedImports(stmt.importClause.namedBindings)) { + const named = stmt.importClause.namedBindings; + const hasValue = !!stmt.importClause.name || named.elements.some(el => !el.isTypeOnly); + if (!hasValue) { + continue; + } + } + if (ts.isStringLiteralLike(stmt.moduleSpecifier)) { + result.push({ specifier: stmt.moduleSpecifier.text, literal: stmt.moduleSpecifier }); + } + continue; + } + if (ts.isExportDeclaration(stmt) && stmt.moduleSpecifier && ts.isStringLiteralLike(stmt.moduleSpecifier)) { + if (stmt.isTypeOnly) { + continue; + } + if (stmt.exportClause && ts.isNamedExports(stmt.exportClause)) { + const allTypeOnly = stmt.exportClause.elements.every(el => el.isTypeOnly); + if (allTypeOnly) { + continue; + } + } + result.push({ specifier: stmt.moduleSpecifier.text, literal: stmt.moduleSpecifier }); + continue; + } + if ( + ts.isImportEqualsDeclaration(stmt) && + ts.isExternalModuleReference(stmt.moduleReference) && + ts.isStringLiteralLike(stmt.moduleReference.expression) + ) { + if (stmt.isTypeOnly) { + continue; + } + result.push({ + specifier: stmt.moduleReference.expression.text, + literal: stmt.moduleReference.expression, + }); + } + } + return result; +} + +function resolveModule( + program: ts.Program, + sf: ts.SourceFile, + specifier: string, + literal: ts.StringLiteralLike, +): string | undefined { + // TS ≥ 5.3 exposes program.getResolvedModule + const getResolvedModule = ( + program as unknown as { + getResolvedModule?: ( + file: ts.SourceFile, + moduleName: string, + mode?: ts.ResolutionMode, + ) => { resolvedModule?: ts.ResolvedModuleFull } | undefined; + } + ).getResolvedModule; + const mode = ( + ts as unknown as { + getModeForUsageLocation?: (file: ts.SourceFile, usage: ts.StringLiteralLike) => ts.ResolutionMode; + } + ).getModeForUsageLocation?.(sf, literal); + + if (typeof getResolvedModule === 'function') { + const r = getResolvedModule.call(program, sf, specifier, mode); + if (r?.resolvedModule) { + return r.resolvedModule.resolvedFileName; } - return { name: entry.name, allow: new Set(entry.allow ?? []) }; - }); + } + + // Fallback for older TS: use ts.resolveModuleName against the compiler host. + const compilerOptions = program.getCompilerOptions(); + const host = + (program as unknown as { getCompilerHost?: () => ts.ModuleResolutionHost }).getCompilerHost?.() ?? ts.sys; + const result = ts.resolveModuleName( + specifier, + sf.fileName, + compilerOptions, + host as ts.ModuleResolutionHost, + undefined, + undefined, + mode, + ); + return result.resolvedModule?.resolvedFileName; +} + +function isBareSpecifier(specifier: string): boolean { + return !specifier.startsWith('.') && !specifier.startsWith('/'); +} + +function packageNameOf(specifier: string): string { + if (specifier.startsWith('@')) { + const [scope, name] = specifier.split('/', 2); + return name ? `${scope}/${name}` : scope; + } + const slash = specifier.indexOf('/'); + return slash === -1 ? specifier : specifier.slice(0, slash); +} + +function shortenPath(absolute: string): string { + const marker = '/node_modules/'; + const idx = absolute.lastIndexOf(marker); + if (idx !== -1) { + return absolute.slice(idx + marker.length); + } + const cwd = process.cwd(); + if (absolute.startsWith(cwd)) { + return absolute.slice(cwd.length + 1); + } + return absolute; } +// --------------------------------------------------------------------------- +// AST helpers (unchanged from previous version) +// --------------------------------------------------------------------------- + function describeParam(param: TSESTree.Parameter): string { switch (param.type) { case AST_NODE_TYPES.Identifier: From b4ed75940677d331041a2905653f8e093fbc3b65 Mon Sep 17 00:00:00 2001 From: Martin Hochel Date: Mon, 25 May 2026 11:26:47 +0200 Subject: [PATCH 10/15] feat(eslint-rules): add allowTypeImports option to consistent-base-hook By default (allowTypeImports: false) type-only imports from forbiddenRuntimes packages are disallowed inside base hooks, to keep the base hook's public API fully decoupled from those packages. Setting allowTypeImports: true restores the previous behavior of skipping type-only imports. Type-only imports from watched packages are unaffected (types cannot transitively pull runtime). --- .../stubs/heavy-runtime/index.ts | 2 + .../rules/consistent-base-hook.spec.ts | 77 +++++++++++++++++++ .../rules/consistent-base-hook.ts | 26 +++++-- 3 files changed, 100 insertions(+), 5 deletions(-) diff --git a/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/heavy-runtime/index.ts b/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/heavy-runtime/index.ts index 5fc7a81d62453..8f5f7d2b4e91f 100644 --- a/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/heavy-runtime/index.ts +++ b/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/heavy-runtime/index.ts @@ -1,3 +1,5 @@ export function runHeavy(): { tag: 'heavy' } { return { tag: 'heavy' }; } + +export type HeavyOptions = { kind: 'heavy' }; diff --git a/tools/eslint-rules/rules/consistent-base-hook.spec.ts b/tools/eslint-rules/rules/consistent-base-hook.spec.ts index 0069ed215cadc..3713c9a4f43a1 100644 --- a/tools/eslint-rules/rules/consistent-base-hook.spec.ts +++ b/tools/eslint-rules/rules/consistent-base-hook.spec.ts @@ -170,6 +170,16 @@ const transitiveOptions: readonly [{ watchedPackages: string[]; forbiddenRuntime }, ]; +const transitiveOptionsAllowTypeImports: readonly [ + { watchedPackages: string[]; forbiddenRuntimes: string[]; allowTypeImports: boolean }, +] = [ + { + watchedPackages: ['watched-pkg'], + forbiddenRuntimes: ['heavy-runtime'], + allowTypeImports: true, + }, +]; + typedRuleTester.run(`${RULE_NAME} (typed)`, rule, { valid: [ // The defining file of \`useLight\` only reaches \`light-helper\`, not \`heavy-runtime\`. @@ -241,6 +251,29 @@ typedRuleTester.run(`${RULE_NAME} (typed)`, rule, { return { props, ref, value: useA() }; }; `, + }, // With `allowTypeImports: true`, type-only imports from a forbidden runtime are permitted. + { + languageOptions: typedLanguageOptions, + filename: TYPED_FILENAME, + options: transitiveOptionsAllowTypeImports, + code: ` + import type { HeavyOptions } from 'heavy-runtime'; + export const useThingBase_unstable = (props: HeavyOptions, ref: React.Ref) => { + return { props, ref }; + }; + `, + }, + // With `allowTypeImports: true`, per-specifier type-only import from a forbidden runtime is permitted. + { + languageOptions: typedLanguageOptions, + filename: TYPED_FILENAME, + options: transitiveOptionsAllowTypeImports, + code: ` + import { type HeavyOptions } from 'heavy-runtime'; + export const useThingBase_unstable = (props: HeavyOptions, ref: React.Ref) => { + return { props, ref }; + }; + `, }, ], invalid: [ @@ -312,5 +345,49 @@ typedRuleTester.run(`${RULE_NAME} (typed)`, rule, { }, ], }, + // By default, a top-level type-only import from a forbidden runtime is disallowed. + { + languageOptions: typedLanguageOptions, + filename: TYPED_FILENAME, + options: transitiveOptions, + code: ` + import type { HeavyOptions } from 'heavy-runtime'; + export const useThingBase_unstable = (props: HeavyOptions, ref: React.Ref) => { + return { props, ref }; + }; + `, + errors: [ + { + messageId: 'forbiddenRuntimeDirect', + data: { + hookName: 'useThingBase_unstable', + importedName: 'HeavyOptions', + package: 'heavy-runtime', + }, + }, + ], + }, + // By default, a per-specifier type-only import from a forbidden runtime is also disallowed. + { + languageOptions: typedLanguageOptions, + filename: TYPED_FILENAME, + options: transitiveOptions, + code: ` + import { type HeavyOptions } from 'heavy-runtime'; + export const useThingBase_unstable = (props: HeavyOptions, ref: React.Ref) => { + return { props, ref }; + }; + `, + errors: [ + { + messageId: 'forbiddenRuntimeDirect', + data: { + hookName: 'useThingBase_unstable', + importedName: 'HeavyOptions', + package: 'heavy-runtime', + }, + }, + ], + }, ], }); diff --git a/tools/eslint-rules/rules/consistent-base-hook.ts b/tools/eslint-rules/rules/consistent-base-hook.ts index c08226d808958..481d221315f71 100644 --- a/tools/eslint-rules/rules/consistent-base-hook.ts +++ b/tools/eslint-rules/rules/consistent-base-hook.ts @@ -33,6 +33,15 @@ type Options = [ * these packages are also forbidden. */ forbiddenRuntimes?: string[]; + /** + * When `true`, type-only imports from `forbiddenRuntimes` packages are + * permitted inside base hooks (they emit no runtime code). + * + * Defaults to `false` — type-only imports from forbidden runtimes are + * disallowed as well, to keep the base hook's public API fully decoupled + * from those packages. + */ + allowTypeImports?: boolean; }?, ]; @@ -92,6 +101,9 @@ export const rule = ESLintUtils.RuleCreator(() => __filename) __filename)([...watchedPackages, ...forbiddenRuntimes]); @@ -284,15 +297,18 @@ export const rule = ESLintUtils.RuleCreator(() => __filename) { - // Per-specifier type-only imports are also runtime-free. - if (specifier.type === AST_NODE_TYPES.ImportSpecifier && specifier.importKind === 'type') { + const specTypeOnly = specifier.type === AST_NODE_TYPES.ImportSpecifier && specifier.importKind === 'type'; + if (specTypeOnly && (!isForbiddenPkg || allowTypeImports)) { return; } let importedName: string; From 77938a0afe3f16771ef6b39611102e9d5910e848 Mon Sep 17 00:00:00 2001 From: Martin Hochel Date: Mon, 25 May 2026 13:08:27 +0200 Subject: [PATCH 11/15] style: encapsulate import tracking and apply explicit type imports/ignore fixtures from type check --- .../rules/consistent-base-hook.ts | 94 ++++++++++--------- .../rules/consistent-callback-type.ts | 3 +- tools/eslint-rules/tsconfig.lint.json | 2 +- 3 files changed, 52 insertions(+), 47 deletions(-) diff --git a/tools/eslint-rules/rules/consistent-base-hook.ts b/tools/eslint-rules/rules/consistent-base-hook.ts index 481d221315f71..d4118052bd203 100644 --- a/tools/eslint-rules/rules/consistent-base-hook.ts +++ b/tools/eslint-rules/rules/consistent-base-hook.ts @@ -1,10 +1,5 @@ -import { - ESLintUtils, - AST_NODE_TYPES, - TSESTree, - TSESLint, - ParserServicesWithTypeInformation, -} from '@typescript-eslint/utils'; +import type { TSESTree, TSESLint, ParserServicesWithTypeInformation } from '@typescript-eslint/utils'; +import { ESLintUtils, AST_NODE_TYPES } from '@typescript-eslint/utils'; import * as ts from 'typescript'; // NOTE: The rule will be available in ESLint configs as "@nx/workspace-consistent-base-hook" @@ -274,7 +269,8 @@ export const rule = ESLintUtils.RuleCreator(() => __filename) __filename) { + const specTypeOnly = specifier.type === AST_NODE_TYPES.ImportSpecifier && specifier.importKind === 'type'; + if (specTypeOnly && (!isForbiddenPkg || allowTypeImports)) { return; } - const isForbiddenPkg = forbiddenRuntimes.has(source); - const declTypeOnly = node.importKind === 'type'; - // Type-only imports from a watched package can never pull runtime; always skip. - // Type-only imports from a forbidden-runtime package are skipped only when explicitly allowed. - if (declTypeOnly && (!isForbiddenPkg || allowTypeImports)) { + const importedName = getImportedName(specifier); + if (importedName === undefined) { return; } - const kind: TrackedImport['kind'] = isForbiddenPkg ? 'forbidden' : 'watched'; + for (const variable of sourceCode.getDeclaredVariables(specifier)) { + trackedImports.set(variable, { package: source, importedName, kind, specifier }); + } + }); + } - node.specifiers.forEach(specifier => { - const specTypeOnly = specifier.type === AST_NODE_TYPES.ImportSpecifier && specifier.importKind === 'type'; - if (specTypeOnly && (!isForbiddenPkg || allowTypeImports)) { - return; - } - 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)) { - trackedImports.set(variable, { package: source, importedName, kind, specifier }); - } - }); - }, + return { + ImportDeclaration: trackImportDeclaration, [`FunctionDeclaration[id.name=/${BASE_HOOK_NAME_PATTERN.source}/]`]: (node: TSESTree.FunctionDeclaration) => { if (!node.id) { @@ -548,6 +532,26 @@ function shortenPath(absolute: string): string { // AST helpers (unchanged from previous version) // --------------------------------------------------------------------------- +/** + * Resolves the original (imported) name from an import specifier. + * Returns `'default'` for default imports, `'*'` for namespace imports, + * and the imported identifier/string-literal name for named imports. + */ +function getImportedName(specifier: ImportSpecifierNode): string | undefined { + switch (specifier.type) { + case AST_NODE_TYPES.ImportSpecifier: + return specifier.imported.type === AST_NODE_TYPES.Identifier + ? specifier.imported.name + : String(specifier.imported.value); + case AST_NODE_TYPES.ImportDefaultSpecifier: + return 'default'; + case AST_NODE_TYPES.ImportNamespaceSpecifier: + return '*'; + default: + return undefined; + } +} + function describeParam(param: TSESTree.Parameter): string { switch (param.type) { case AST_NODE_TYPES.Identifier: diff --git a/tools/eslint-rules/rules/consistent-callback-type.ts b/tools/eslint-rules/rules/consistent-callback-type.ts index 20f012ef6d3ea..ed5ea947187f1 100644 --- a/tools/eslint-rules/rules/consistent-callback-type.ts +++ b/tools/eslint-rules/rules/consistent-callback-type.ts @@ -1,4 +1,5 @@ -import { ESLintUtils, AST_NODE_TYPES, TSESTree } from '@typescript-eslint/utils'; +import type { TSESTree } from '@typescript-eslint/utils'; +import { ESLintUtils, AST_NODE_TYPES } from '@typescript-eslint/utils'; // NOTE: The rule will be available in ESLint configs as "@nx/workspace-consistent-callback-type" export const RULE_NAME = 'consistent-callback-type'; diff --git a/tools/eslint-rules/tsconfig.lint.json b/tools/eslint-rules/tsconfig.lint.json index bb717c5e289e3..5215cb85923e4 100644 --- a/tools/eslint-rules/tsconfig.lint.json +++ b/tools/eslint-rules/tsconfig.lint.json @@ -4,6 +4,6 @@ "outDir": "../../dist/out-tsc", "types": ["node"] }, - "exclude": ["**/*.spec.ts"], + "exclude": ["**/*.spec.ts", "**/__fixtures__/**"], "include": ["**/*.ts"] } From 6ada519c81110819d3fd9ecff96727b2796e573f Mon Sep 17 00:00:00 2001 From: Martin Hochel Date: Mon, 25 May 2026 20:13:04 +0200 Subject: [PATCH 12/15] fix(eslint-rules): validate Ref import origin in consistent-base-hook; fix tooltip directive - isReactRefTypeAnnotation now resolves the Ref/React identifier via scope analysis and requires it to be imported from 'react'. Adds tests for local aliases, non-react imports, and shadowed React.- useTooltipBase_unstable: use bare 'use no memo' directive prologue instead of parenthesized expression so React Compiler detects it. --- .../src/components/Tooltip/useTooltipBase.tsx | 2 +- .../rules/consistent-base-hook.spec.ts | 67 ++++++++++++++++- .../rules/consistent-base-hook.ts | 73 ++++++++++++++++++- 3 files changed, 135 insertions(+), 7 deletions(-) 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 30d586b5e79b4..4919ea58156ac 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/rules/consistent-base-hook.spec.ts b/tools/eslint-rules/rules/consistent-base-hook.spec.ts index 3713c9a4f43a1..4319b36824f45 100644 --- a/tools/eslint-rules/rules/consistent-base-hook.spec.ts +++ b/tools/eslint-rules/rules/consistent-base-hook.spec.ts @@ -19,7 +19,7 @@ const ruleTester = new RuleTester(); ruleTester.run(RULE_NAME, rule, { valid: [ - // Valid base hook: 2 Identifier params, no forbidden imports. + // Valid base hook: namespace import — `import * as React from 'react'` + `React.Ref<...>`. { code: ` import * as React from 'react'; @@ -28,7 +28,7 @@ ruleTester.run(RULE_NAME, rule, { }; `, }, - // Valid base hook declared as FunctionDeclaration. + // Valid base hook: named import — `import { Ref } from 'react'` + `Ref<...>` (FunctionDeclaration form). { code: ` import { Ref } from 'react'; @@ -37,6 +37,15 @@ ruleTester.run(RULE_NAME, rule, { } `, }, + // Valid base hook: default import — `import React from 'react'` + `React.Ref<...>`. + { + code: ` + import React from 'react'; + export const useThingBase_unstable = (props, ref: React.Ref) => { + return { props, ref }; + }; + `, + }, // Valid base hook with only \`props\` (ref is optional). { code: ` @@ -56,6 +65,7 @@ ruleTester.run(RULE_NAME, rule, { // Identifier with the same local name as a forbidden import alias does not collide via scope analysis. { code: ` + import * as React from 'react'; import { useArrowNavigationGroup } from '@fluentui/react-tabster'; export const useThing_unstable = (props, ref) => { return useArrowNavigationGroup({}); @@ -69,6 +79,7 @@ ruleTester.run(RULE_NAME, rule, { // \`keyborg\` is not in the default forbidden runtime list — bindings imported from it are allowed inside base hooks. { code: ` + import * as React from 'react'; import { createKeyborg, KEYBORG_FOCUSIN } from 'keyborg'; export const useThingBase_unstable = (props, ref: React.Ref) => { return { kb: createKeyborg(window), evt: KEYBORG_FOCUSIN }; @@ -110,6 +121,7 @@ ruleTester.run(RULE_NAME, rule, { // ObjectPattern for \`props\` is not allowed. { code: ` + import * as React from 'react'; export const useThingBase_unstable = ({ a }, ref: React.Ref) => ({ a, ref }); `, errors: [ @@ -155,6 +167,45 @@ ruleTester.run(RULE_NAME, rule, { }, ], }, + // \`Ref\` is a locally declared type alias, not imported from react. + { + code: ` + type Ref = { current: T | null }; + export const useThingBase_unstable = (props, ref: Ref) => ({ props, ref }); + `, + errors: [ + { + messageId: 'invalidRefType', + data: { hookName: 'useThingBase_unstable', actual: 'Ref' }, + }, + ], + }, + // \`Ref\` imported from a non-react package is not accepted. + { + code: ` + import { Ref } from 'not-react'; + export const useThingBase_unstable = (props, ref: Ref) => ({ props, ref }); + `, + errors: [ + { + messageId: 'invalidRefType', + data: { hookName: 'useThingBase_unstable', actual: 'Ref' }, + }, + ], + }, + // \`React\` is a locally declared identifier, not the react module namespace. + { + code: ` + const React = { Ref: null }; + export const useThingBase_unstable = (props, ref: React.Ref) => ({ props, ref }); + `, + errors: [ + { + messageId: 'invalidRefType', + data: { hookName: 'useThingBase_unstable', actual: 'React.Ref' }, + }, + ], + }, ], }); @@ -188,6 +239,7 @@ typedRuleTester.run(`${RULE_NAME} (typed)`, rule, { filename: TYPED_FILENAME, options: transitiveOptions, code: ` + import * as React from 'react'; import { useLight } from 'watched-pkg'; export const useThingBase_unstable = (props: { a: number }, ref: React.Ref) => { useLight(); @@ -201,6 +253,7 @@ typedRuleTester.run(`${RULE_NAME} (typed)`, rule, { filename: TYPED_FILENAME, options: transitiveOptions, code: ` + import * as React from 'react'; import type { HeavyType } from 'watched-pkg'; export const useThingBase_unstable = (props: HeavyType, ref: React.Ref) => { return { props, ref }; @@ -213,6 +266,7 @@ typedRuleTester.run(`${RULE_NAME} (typed)`, rule, { filename: TYPED_FILENAME, options: transitiveOptions, code: ` + import * as React from 'react'; import { type HeavyType, useLight } from 'watched-pkg'; export const useThingBase_unstable = (props: HeavyType, ref: React.Ref) => { useLight(); @@ -226,6 +280,7 @@ typedRuleTester.run(`${RULE_NAME} (typed)`, rule, { filename: TYPED_FILENAME, options: transitiveOptions, code: ` + import * as React from 'react'; import { useHeavy } from 'watched-pkg'; export const useThingBase_unstable = (props: { a: number }, ref: React.Ref) => { return { props, ref }; @@ -246,6 +301,7 @@ typedRuleTester.run(`${RULE_NAME} (typed)`, rule, { }, ], code: ` + import * as React from 'react'; import { useA } from 'cyclic-pkg'; export const useThingBase_unstable = (props: { a: number }, ref: React.Ref) => { return { props, ref, value: useA() }; @@ -257,6 +313,7 @@ typedRuleTester.run(`${RULE_NAME} (typed)`, rule, { filename: TYPED_FILENAME, options: transitiveOptionsAllowTypeImports, code: ` + import * as React from 'react'; import type { HeavyOptions } from 'heavy-runtime'; export const useThingBase_unstable = (props: HeavyOptions, ref: React.Ref) => { return { props, ref }; @@ -269,6 +326,7 @@ typedRuleTester.run(`${RULE_NAME} (typed)`, rule, { filename: TYPED_FILENAME, options: transitiveOptionsAllowTypeImports, code: ` + import * as React from 'react'; import { type HeavyOptions } from 'heavy-runtime'; export const useThingBase_unstable = (props: HeavyOptions, ref: React.Ref) => { return { props, ref }; @@ -283,6 +341,7 @@ typedRuleTester.run(`${RULE_NAME} (typed)`, rule, { filename: TYPED_FILENAME, options: transitiveOptions, code: ` + import * as React from 'react'; import { runHeavy } from 'heavy-runtime'; export const useThingBase_unstable = (props: { a: number }, ref: React.Ref) => { return { props, ref, x: runHeavy() }; @@ -305,6 +364,7 @@ typedRuleTester.run(`${RULE_NAME} (typed)`, rule, { filename: TYPED_FILENAME, options: transitiveOptions, code: ` + import * as React from 'react'; import { useHeavy } from 'watched-pkg'; export const useThingBase_unstable = (props: { a: number }, ref: React.Ref) => { return { props, ref, x: useHeavy() }; @@ -329,6 +389,7 @@ typedRuleTester.run(`${RULE_NAME} (typed)`, rule, { filename: TYPED_FILENAME, options: transitiveOptions, code: ` + import * as React from 'react'; import { runHeavy as go } from 'heavy-runtime'; export function useThingBase_unstable(props: { a: number }, ref: React.Ref) { return go(); @@ -351,6 +412,7 @@ typedRuleTester.run(`${RULE_NAME} (typed)`, rule, { filename: TYPED_FILENAME, options: transitiveOptions, code: ` + import * as React from 'react'; import type { HeavyOptions } from 'heavy-runtime'; export const useThingBase_unstable = (props: HeavyOptions, ref: React.Ref) => { return { props, ref }; @@ -373,6 +435,7 @@ typedRuleTester.run(`${RULE_NAME} (typed)`, rule, { filename: TYPED_FILENAME, options: transitiveOptions, code: ` + import * as React from 'react'; import { type HeavyOptions } from 'heavy-runtime'; export const useThingBase_unstable = (props: HeavyOptions, ref: React.Ref) => { return { props, ref }; diff --git a/tools/eslint-rules/rules/consistent-base-hook.ts b/tools/eslint-rules/rules/consistent-base-hook.ts index d4118052bd203..712e96b424c37 100644 --- a/tools/eslint-rules/rules/consistent-base-hook.ts +++ b/tools/eslint-rules/rules/consistent-base-hook.ts @@ -167,7 +167,7 @@ export const rule = ESLintUtils.RuleCreator(() => __filename) { + if (def.type !== 'ImportBinding') { + return false; + } + const importDecl = def.parent; + if (!importDecl || importDecl.type !== AST_NODE_TYPES.ImportDeclaration) { + return false; + } + if (importDecl.source.value !== 'react') { + return false; + } + const specifier = def.node; + switch (specifier.type) { + case AST_NODE_TYPES.ImportSpecifier: { + const importedName = + specifier.imported.type === AST_NODE_TYPES.Identifier + ? specifier.imported.name + : String(specifier.imported.value); + return importedName === expectedImportedName; + } + case AST_NODE_TYPES.ImportNamespaceSpecifier: + return expectedImportedName === '*'; + case AST_NODE_TYPES.ImportDefaultSpecifier: + // `import React from 'react'` is also a valid way to access `React.Ref`. + return expectedImportedName === '*' || expectedImportedName === 'default'; + default: + return false; + } + }); +} + +function findVariableInScope(scope: TSESLint.Scope.Scope, name: string): TSESLint.Scope.Variable | undefined { + let current: TSESLint.Scope.Scope | null = scope; + while (current) { + const variable = current.set.get(name); + if (variable) { + return variable; + } + current = current.upper; + } + return undefined; +} + function describeRefType(annotation: TSESTree.TSTypeAnnotation | undefined): string { if (!annotation) { return ''; From dffca5aad2846cdf0db9518a578b32d57288cb76 Mon Sep 17 00:00:00 2001 From: Martin Hochel Date: Tue, 26 May 2026 11:13:43 +0200 Subject: [PATCH 13/15] revert(eslint-rules): undo unrelated import change in consistent-callback-type --- tools/eslint-rules/rules/consistent-callback-type.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tools/eslint-rules/rules/consistent-callback-type.ts b/tools/eslint-rules/rules/consistent-callback-type.ts index ed5ea947187f1..20f012ef6d3ea 100644 --- a/tools/eslint-rules/rules/consistent-callback-type.ts +++ b/tools/eslint-rules/rules/consistent-callback-type.ts @@ -1,5 +1,4 @@ -import type { TSESTree } from '@typescript-eslint/utils'; -import { ESLintUtils, AST_NODE_TYPES } from '@typescript-eslint/utils'; +import { ESLintUtils, AST_NODE_TYPES, TSESTree } from '@typescript-eslint/utils'; // NOTE: The rule will be available in ESLint configs as "@nx/workspace-consistent-callback-type" export const RULE_NAME = 'consistent-callback-type'; From 1084d65b51be128fd7326cee1d564e9055e61a6f Mon Sep 17 00:00:00 2001 From: Martin Hochel Date: Tue, 26 May 2026 12:13:35 +0200 Subject: [PATCH 14/15] feat(eslint-rules): detect type-leakage through watched packages in consistent-base-hook Extend the rule's transitive analysis so type references in a base hook signature are also checked against forbidden runtimes when they originate from a watched package whose defining module reaches a forbidden runtime (via either value or type imports). - Track type-only imports from watched packages (previously skipped). - Per-Program cache now stores { value, all } reach sets, filled in a single DFS pass so the new analysis adds no extra resolution work. - visitScope picks the reach set based on reference.isTypeReference: value refs use value reach (runtime coupling), type refs use all reach (API-surface coupling). - allowTypeImports is now symmetric: when true, type-only imports are exempt from BOTH direct forbidden checks AND transitive watched reach checks. - New tests cover symmetric exemption, type-only watched -> clean module (allowed), and indirect type leakage via HeavyWrapper whose defining file value-re-exports the heavy module. --- .../stubs/watched-pkg/index.ts | 4 + .../rules/consistent-base-hook.spec.ts | 136 +++++- .../rules/consistent-base-hook.ts | 388 +++++++++++++----- 3 files changed, 411 insertions(+), 117 deletions(-) diff --git a/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/watched-pkg/index.ts b/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/watched-pkg/index.ts index f20537a7b393d..ecc54ec64c43d 100644 --- a/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/watched-pkg/index.ts +++ b/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/stubs/watched-pkg/index.ts @@ -1,5 +1,9 @@ +import type { HeavyType } from './heavy'; + export { useHeavy } from './heavy'; export { useLight } from './light'; export type { LightOptions } from './light'; // Re-export of a type-only thing from the heavy module — must not count as a runtime reach. export type { HeavyType } from './heavy'; + +export type HeavyWrapper = { tag: 'heavy-wrapper'; inner: HeavyType }; diff --git a/tools/eslint-rules/rules/consistent-base-hook.spec.ts b/tools/eslint-rules/rules/consistent-base-hook.spec.ts index 4319b36824f45..58ed8c66266e1 100644 --- a/tools/eslint-rules/rules/consistent-base-hook.spec.ts +++ b/tools/eslint-rules/rules/consistent-base-hook.spec.ts @@ -206,6 +206,26 @@ ruleTester.run(RULE_NAME, rule, { }, ], }, + // Referencing a watched-package binding inside a base hook without typed services available + // surfaces a one-shot `typedServicesUnavailable` diagnostic so the misconfiguration is visible. + { + code: ` + import * as React from 'react'; + import { useArrowNavigationGroup } from '@fluentui/react-tabster'; + export const useThingBase_unstable = (props, ref: React.Ref) => { + return useArrowNavigationGroup({}); + }; + `, + errors: [ + { + messageId: 'typedServicesUnavailable', + data: { + watchedPackages: '@fluentui/react-tabster', + forbiddenRuntimes: 'tabster', + }, + }, + ], + }, ], }); @@ -247,29 +267,17 @@ typedRuleTester.run(`${RULE_NAME} (typed)`, rule, { }; `, }, - // Type-only import of a symbol whose defining file reaches the forbidden runtime is allowed. + // Type-only import of a watched-package symbol whose defining file does NOT reach the + // forbidden runtime is allowed (`LightOptions` is defined in `light.ts` which only pulls + // `light-helper`). { languageOptions: typedLanguageOptions, filename: TYPED_FILENAME, options: transitiveOptions, code: ` import * as React from 'react'; - import type { HeavyType } from 'watched-pkg'; - export const useThingBase_unstable = (props: HeavyType, ref: React.Ref) => { - return { props, ref }; - }; - `, - }, - // Per-specifier type-only import is also OK. - { - languageOptions: typedLanguageOptions, - filename: TYPED_FILENAME, - options: transitiveOptions, - code: ` - import * as React from 'react'; - import { type HeavyType, useLight } from 'watched-pkg'; - export const useThingBase_unstable = (props: HeavyType, ref: React.Ref) => { - useLight(); + import type { LightOptions } from 'watched-pkg'; + export const useThingBase_unstable = (props: LightOptions, ref: React.Ref) => { return { props, ref }; }; `, @@ -333,6 +341,20 @@ typedRuleTester.run(`${RULE_NAME} (typed)`, rule, { }; `, }, + // Symmetric `allowTypeImports`: also exempts watched-package type-only imports whose defining + // module reaches a forbidden runtime (no runtime coupling is possible from a type). + { + languageOptions: typedLanguageOptions, + filename: TYPED_FILENAME, + options: transitiveOptionsAllowTypeImports, + code: ` + import * as React from 'react'; + import type { HeavyType } from 'watched-pkg'; + export const useThingBase_unstable = (props: HeavyType, ref: React.Ref) => { + return { props, ref }; + }; + `, + }, ], invalid: [ // Direct import from a forbidden-runtime package. @@ -452,5 +474,85 @@ typedRuleTester.run(`${RULE_NAME} (typed)`, rule, { }, ], }, + // Type-leakage through a watched package: a top-level `import type` of a watched-package + // symbol whose defining module transitively reaches the forbidden runtime is disallowed + // because the type still ties the base hook's public API to the forbidden runtime. + { + languageOptions: typedLanguageOptions, + filename: TYPED_FILENAME, + options: transitiveOptions, + code: ` + import * as React from 'react'; + import type { HeavyType } from 'watched-pkg'; + export const useThingBase_unstable = (props: HeavyType, ref: React.Ref) => { + return { props, ref }; + }; + `, + errors: [ + { + messageId: 'forbiddenRuntimeReach', + data: { + hookName: 'useThingBase_unstable', + importedName: 'HeavyType', + package: 'watched-pkg', + runtime: 'heavy-runtime', + viaFile: 'rules/__fixtures__/consistent-base-hook/stubs/watched-pkg/heavy.ts', + }, + }, + ], + }, + // Per-specifier `type` modifier variant of the same scenario. + { + languageOptions: typedLanguageOptions, + filename: TYPED_FILENAME, + options: transitiveOptions, + code: ` + import * as React from 'react'; + import { type HeavyType, useLight } from 'watched-pkg'; + export const useThingBase_unstable = (props: HeavyType, ref: React.Ref) => { + useLight(); + return { props, ref }; + }; + `, + errors: [ + { + messageId: 'forbiddenRuntimeReach', + data: { + hookName: 'useThingBase_unstable', + importedName: 'HeavyType', + package: 'watched-pkg', + runtime: 'heavy-runtime', + viaFile: 'rules/__fixtures__/consistent-base-hook/stubs/watched-pkg/heavy.ts', + }, + }, + ], + }, + // Indirect type leakage: `HeavyWrapper` is declared in `watched-pkg/index.ts` (not in `heavy.ts`), + // but its defining file value-re-exports `./heavy`, so the type-graph reach from `index.ts` still + // includes `heavy-runtime`. The base hook surface is therefore tied to the forbidden runtime. + { + languageOptions: typedLanguageOptions, + filename: TYPED_FILENAME, + options: transitiveOptions, + code: ` + import * as React from 'react'; + import type { HeavyWrapper } from 'watched-pkg'; + export const useThingBase_unstable = (props: HeavyWrapper, ref: React.Ref) => { + return { props, ref }; + }; + `, + errors: [ + { + messageId: 'forbiddenRuntimeReach', + data: { + hookName: 'useThingBase_unstable', + importedName: 'HeavyWrapper', + package: 'watched-pkg', + runtime: 'heavy-runtime', + viaFile: 'rules/__fixtures__/consistent-base-hook/stubs/watched-pkg/index.ts', + }, + }, + ], + }, ], }); diff --git a/tools/eslint-rules/rules/consistent-base-hook.ts b/tools/eslint-rules/rules/consistent-base-hook.ts index 712e96b424c37..165582b0b8be1 100644 --- a/tools/eslint-rules/rules/consistent-base-hook.ts +++ b/tools/eslint-rules/rules/consistent-base-hook.ts @@ -29,12 +29,13 @@ type Options = [ */ forbiddenRuntimes?: string[]; /** - * When `true`, type-only imports from `forbiddenRuntimes` packages are - * permitted inside base hooks (they emit no runtime code). + * When `true`, type-only imports (both from `forbiddenRuntimes` packages directly and + * from `watchedPackages` whose defining module reaches a forbidden runtime) are permitted + * inside base hooks. Type-only imports emit no runtime code, so this option trades API + * decoupling for ergonomics. * - * Defaults to `false` — type-only imports from forbidden runtimes are - * disallowed as well, to keep the base hook's public API fully decoupled - * from those packages. + * Defaults to `false` — type-only imports are checked the same way as value imports, to + * keep the base hook's public API fully decoupled from forbidden runtimes. */ allowTypeImports?: boolean; }?, @@ -45,7 +46,8 @@ type MessageIds = | 'invalidParamName' | 'invalidRefType' | 'forbiddenRuntimeDirect' - | 'forbiddenRuntimeReach'; + | 'forbiddenRuntimeReach' + | 'typedServicesUnavailable'; type ImportSpecifierNode = | TSESTree.ImportSpecifier @@ -59,6 +61,12 @@ interface TrackedImport { importedName: string; /** Kind of package — controls how the reference is checked. */ kind: 'watched' | 'forbidden'; + /** + * `true` when the binding is type-only (either the declaration is `import type ...` + * or the specifier is `import { type Foo }`). Used to gate whether direct usage in a + * value position is even possible (type-only bindings only surface in type positions). + */ + isTypeOnly: boolean; /** The specifier node (used for symbol lookup via ParserServices). */ specifier: ImportSpecifierNode; } @@ -66,13 +74,26 @@ interface TrackedImport { type BaseHookFunction = TSESTree.FunctionDeclaration | TSESTree.FunctionExpression | TSESTree.ArrowFunctionExpression; /** - * Per-Program cache: source file path → set of bare package specifiers transitively reached - * via value (non type-only) imports starting from that file. + * Result of a single transitive-reach DFS over a source file's import graph. + * - `value` — packages reachable via value (non type-only) imports only. Used to decide + * whether a runtime reference can pull a forbidden runtime at execution time. + * - `all` — packages reachable via value OR type imports. Used to decide whether a + * type reference can leak a forbidden runtime through the public API surface. + * `value` is always a subset of `all`. + */ +interface Reach { + value: ReadonlySet; + all: ReadonlySet; +} + +/** + * Per-Program cache: source file path → reach sets transitively computed from that file. + * Both `value` and `all` sets are filled in a single DFS pass to share resolution work. * * Keyed by `ts.Program` identity so the cache is invalidated whenever * typescript-eslint rebuilds the Program. */ -const programCache = new WeakMap>>(); +const programCache = new WeakMap>(); export const rule = ESLintUtils.RuleCreator(() => __filename)({ name: RULE_NAME, @@ -80,7 +101,7 @@ export const rule = ESLintUtils.RuleCreator(() => __filename)Base_unstable`): a required `props` parameter and an optional `ref` parameter typed as `React.Ref<...>`, and disallow referencing any binding whose defining module transitively pulls a forbidden runtime package (default `tabster`).', + 'Enforce the API contract for v9 "base" hooks (`useBase_unstable`): a required `props` parameter and an optional `ref` parameter typed as `React.Ref<...>`, and disallow referencing any binding whose defining module transitively pulls a forbidden runtime package (default `tabster`) \u2014 both at value positions (runtime coupling) and at type positions (API surface coupling).', }, schema: [ { @@ -113,6 +134,8 @@ export const rule = ESLintUtils.RuleCreator(() => __filename) __filename)([...watchedPackages, ...forbiddenRuntimes]); - // Variable → import origin. Keyed by Variable identity so shadowing inside the base hook is handled. + // Map of locally-declared variable identity → original import origin metadata. Keyed by Variable + // identity (not name) so re-declarations / shadowing inside the base hook resolve correctly. const trackedImports = new Map(); - // Lazily-acquired typed services. Untyped lint silently skips transitive analysis. + // Tracks whether `computeSymbolReach` was invoked while typed services were unavailable. When set, + // we emit a single diagnostic on `Program:exit` so the user knows transitive analysis was skipped. + let typedServicesNeededButMissing = false; + + // Lazily-acquired typed services. Resolved once per file, cached in `typedServices` (undefined = + // not yet attempted, null = attempted and unavailable, value = available). let typedServices: ParserServicesWithTypeInformation | null | undefined; + + /** + * Returns typed services (TS Program + checker) for the current file, or `null` if untyped + * lint is in effect. Result is memoized for the lifetime of the per-file rule instance. + */ function getTypedServices(): ParserServicesWithTypeInformation | null { if (typedServices !== undefined) { return typedServices; @@ -142,22 +176,31 @@ export const rule = ESLintUtils.RuleCreator(() => __filename) MAX_PARAM_COUNT) { + /** + * Validates the base-hook signature: 1 or 2 positional params, first must be Identifier `props`, + * optional second must be Identifier `ref` typed as `React.Ref<...>` (verified to originate from + * the `react` package so collisions with same-named locals don't pass). + */ + function checkParameters(hookName: string, hookFn: BaseHookFunction, reportNode: TSESTree.Node): void { + if (hookFn.params.length < MIN_PARAM_COUNT || hookFn.params.length > MAX_PARAM_COUNT) { context.report({ node: reportNode, messageId: 'invalidParamCount', - data: { hookName, actual: fn.params.length }, + data: { hookName, actual: hookFn.params.length }, }); return; } - fn.params.forEach((param, index) => { + hookFn.params.forEach((param, index) => { const expected = EXPECTED_PARAM_NAMES[index]; if (param.type !== AST_NODE_TYPES.Identifier || param.name !== expected) { context.report({ @@ -177,16 +220,31 @@ export const rule = ESLintUtils.RuleCreator(() => __filename) __filename) __filename) __filename) visitScope(child, fn, hookName)); + scope.childScopes.forEach(child => visitScope(child, hookFn, hookName)); } - function computeSymbolReach(origin: TrackedImport): { reached: ReadonlySet; viaFile: string } | null { + /** + * Resolves the watched-package import to its defining module via TS `Program`, then queries the + * transitive import graph for forbidden runtimes (both value-only and value+type sets). + * Returns `null` (and flips the `typedServicesNeededButMissing` flag) when typed services + * aren't available, so the caller can silently skip and we can warn once on `Program:exit`. + */ + function computeSymbolReach( + origin: TrackedImport, + ): { value: ReadonlySet; all: ReadonlySet; viaFile: string } | null { const services = getTypedServices(); if (!services) { + typedServicesNeededButMissing = true; return null; } const checker = services.program.getTypeChecker(); @@ -269,7 +344,7 @@ export const rule = ESLintUtils.RuleCreator(() => __filename) __filename) { - const specTypeOnly = specifier.type === AST_NODE_TYPES.ImportSpecifier && specifier.importKind === 'type'; - if (specTypeOnly && (!isForbiddenPkg || allowTypeImports)) { + const specTypeOnly = + stmtTypeOnly || (specifier.type === AST_NODE_TYPES.ImportSpecifier && specifier.importKind === 'type'); + if (specTypeOnly && allowTypeImports) { return; } const importedName = getImportedName(specifier); @@ -310,7 +395,13 @@ export const rule = ESLintUtils.RuleCreator(() => __filename) __filename) { + // `export default function () {}` produces an anonymous FunctionDeclaration (id === null). + // The esquery selector above requires `id.name`, so this branch should be unreachable in + // practice — kept as a type-narrowing guard so TS treats `node.id` as non-null below. if (!node.id) { return; } @@ -326,9 +420,17 @@ export const rule = ESLintUtils.RuleCreator(() => __filename) { + // The declarator's `id` can be a destructuring pattern (e.g. `const { useFooBase_unstable } = mod`). + // Such patterns never define a function literal we can inspect, and the selector only matched + // because esquery probed the nested Identifier name. Skip anything that isn't a plain Identifier. if (node.id.type !== AST_NODE_TYPES.Identifier) { return; } + // We only check inline function literals (`= (props, ref) => {}` or `= function () {}`) because + // the param-shape / body-reference analysis needs a function node we can walk. Any other Right Hand Side + // — missing initializer (`let useFooBase_unstable;`, ambient `declare const ... : Hook`), a call + // expression (`= wrap(impl)`), an identifier alias (`= someOtherHook`), `require(...)`, etc. — + // is out of scope for this rule. const init = node.init; if ( !init || @@ -338,106 +440,151 @@ export const rule = ESLintUtils.RuleCreator(() => __filename) { +function transitiveReach(program: ts.Program, sourceFile: ts.SourceFile): Reach { let cache = programCache.get(program); if (!cache) { cache = new Map(); programCache.set(program, cache); } - return computeReach(program, sf, cache, new Set()); + return computeReach(program, sourceFile, cache, new Set()); } +/** + * Recursive worker for `transitiveReach`. Walks the import graph DFS, recording every bare + * specifier encountered (separately for value-only vs value+type follow modes) and recursing into + * each resolved source file. Uses `inProgress` to break cycles (cycle hits return empty sets + * without caching, so the originating call still commits the complete result). + */ function computeReach( program: ts.Program, - sf: ts.SourceFile, - cache: Map>, + sourceFile: ts.SourceFile, + cache: Map, inProgress: Set, -): ReadonlySet { - const cached = cache.get(sf.fileName); +): Reach { + const cached = cache.get(sourceFile.fileName); if (cached) { return cached; } - if (inProgress.has(sf.fileName)) { - // Cycle: return an empty set without caching so the eventual full result is committed by the originator. - return EMPTY_SET; + if (inProgress.has(sourceFile.fileName)) { + // Cycle: return empty sets without caching so the eventual full result is committed by the originator. + return EMPTY_REACH; } - inProgress.add(sf.fileName); - - const result = new Set(); - for (const { specifier, literal } of collectValueImports(sf)) { - if (isBareSpecifier(specifier)) { - result.add(packageNameOf(specifier)); + inProgress.add(sourceFile.fileName); + + const value = new Set(); + const all = new Set(); + for (const imp of collectImports(sourceFile)) { + if (isBareSpecifier(imp.specifier)) { + const pkg = packageNameOf(imp.specifier); + all.add(pkg); + if (!imp.typeOnly) { + value.add(pkg); + } } - const resolved = resolveModule(program, sf, specifier, literal); + const resolved = resolveModule(program, sourceFile, imp.specifier, imp.literal); if (!resolved) { continue; } - const childSf = program.getSourceFile(resolved); - if (!childSf) { + const childSourceFile = program.getSourceFile(resolved); + if (!childSourceFile) { continue; } - const childReach = computeReach(program, childSf, cache, inProgress); - for (const pkg of childReach) { - result.add(pkg); + const childReach = computeReach(program, childSourceFile, cache, inProgress); + for (const pkg of childReach.all) { + all.add(pkg); + } + if (!imp.typeOnly) { + // A type-only edge does not propagate runtime reach: it can only widen the `all` set. + for (const pkg of childReach.value) { + value.add(pkg); + } } } - inProgress.delete(sf.fileName); - cache.set(sf.fileName, result); + inProgress.delete(sourceFile.fileName); + const result: Reach = { value, all }; + cache.set(sourceFile.fileName, result); return result; } -const EMPTY_SET: ReadonlySet = new Set(); +const EMPTY_REACH: Reach = { value: new Set(), all: new Set() }; -interface ValueImport { +interface ImportEdge { specifier: string; literal: ts.StringLiteralLike; + /** `true` when this edge only carries type information (no runtime side-effect). */ + typeOnly: boolean; } -function collectValueImports(sf: ts.SourceFile): ValueImport[] { - const result: ValueImport[] = []; - for (const stmt of sf.statements) { +/** + * Enumerates every module specifier in `sourceFile`, tagging each edge as value (`typeOnly: false`) + * or type-only (`typeOnly: true`). `import type` / `export type`, fully type-only named import or + * export clauses, and `import type =` are emitted with `typeOnly: true`. Side-effect imports + * (no clause) are emitted as value edges. + */ +function collectImports(sourceFile: ts.SourceFile): ImportEdge[] { + const result: ImportEdge[] = []; + for (const stmt of sourceFile.statements) { if (ts.isImportDeclaration(stmt)) { + let typeOnly = false; if (stmt.importClause?.isTypeOnly) { - continue; - } - // `import './side-effect'` (no clause) IS a value import. - // For named imports, drop if every specifier is type-only AND there's no default/namespace. - if (stmt.importClause && stmt.importClause.namedBindings && ts.isNamedImports(stmt.importClause.namedBindings)) { + typeOnly = true; + } else if ( + stmt.importClause && + stmt.importClause.namedBindings && + ts.isNamedImports(stmt.importClause.namedBindings) + ) { const named = stmt.importClause.namedBindings; - const hasValue = !!stmt.importClause.name || named.elements.some(el => !el.isTypeOnly); - if (!hasValue) { - continue; - } + const hasValue = !!stmt.importClause.name || named.elements.some(element => !element.isTypeOnly); + typeOnly = !hasValue; } if (ts.isStringLiteralLike(stmt.moduleSpecifier)) { - result.push({ specifier: stmt.moduleSpecifier.text, literal: stmt.moduleSpecifier }); + result.push({ specifier: stmt.moduleSpecifier.text, literal: stmt.moduleSpecifier, typeOnly }); } continue; } if (ts.isExportDeclaration(stmt) && stmt.moduleSpecifier && ts.isStringLiteralLike(stmt.moduleSpecifier)) { - if (stmt.isTypeOnly) { - continue; - } - if (stmt.exportClause && ts.isNamedExports(stmt.exportClause)) { - const allTypeOnly = stmt.exportClause.elements.every(el => el.isTypeOnly); - if (allTypeOnly) { - continue; - } + let typeOnly = stmt.isTypeOnly; + if (!typeOnly && stmt.exportClause && ts.isNamedExports(stmt.exportClause)) { + typeOnly = stmt.exportClause.elements.every(element => element.isTypeOnly); } - result.push({ specifier: stmt.moduleSpecifier.text, literal: stmt.moduleSpecifier }); + result.push({ specifier: stmt.moduleSpecifier.text, literal: stmt.moduleSpecifier, typeOnly }); continue; } if ( @@ -445,21 +592,25 @@ function collectValueImports(sf: ts.SourceFile): ValueImport[] { ts.isExternalModuleReference(stmt.moduleReference) && ts.isStringLiteralLike(stmt.moduleReference.expression) ) { - if (stmt.isTypeOnly) { - continue; - } result.push({ specifier: stmt.moduleReference.expression.text, literal: stmt.moduleReference.expression, + typeOnly: stmt.isTypeOnly, }); } } return result; } +/** + * Resolves `specifier` (as used in `sourceFile`) to an absolute file path using the same algorithm + * the host TS Program uses. Prefers the (faster) `program.getResolvedModule` API exposed in TS ≥ 5.3 + * and falls back to `ts.resolveModuleName` for older toolchains. Returns `undefined` if the module + * cannot be resolved (e.g. ambient declarations, broken paths). + */ function resolveModule( program: ts.Program, - sf: ts.SourceFile, + sourceFile: ts.SourceFile, specifier: string, literal: ts.StringLiteralLike, ): string | undefined { @@ -477,12 +628,12 @@ function resolveModule( ts as unknown as { getModeForUsageLocation?: (file: ts.SourceFile, usage: ts.StringLiteralLike) => ts.ResolutionMode; } - ).getModeForUsageLocation?.(sf, literal); + ).getModeForUsageLocation?.(sourceFile, literal); if (typeof getResolvedModule === 'function') { - const r = getResolvedModule.call(program, sf, specifier, mode); - if (r?.resolvedModule) { - return r.resolvedModule.resolvedFileName; + const resolutionResult = getResolvedModule.call(program, sourceFile, specifier, mode); + if (resolutionResult?.resolvedModule) { + return resolutionResult.resolvedModule.resolvedFileName; } } @@ -492,7 +643,7 @@ function resolveModule( (program as unknown as { getCompilerHost?: () => ts.ModuleResolutionHost }).getCompilerHost?.() ?? ts.sys; const result = ts.resolveModuleName( specifier, - sf.fileName, + sourceFile.fileName, compilerOptions, host as ts.ModuleResolutionHost, undefined, @@ -502,10 +653,18 @@ function resolveModule( return result.resolvedModule?.resolvedFileName; } +/** + * `true` when the import specifier refers to a package (e.g. `react`, `@scope/pkg`, `pkg/sub`) + * rather than a relative or absolute path. + */ function isBareSpecifier(specifier: string): boolean { return !specifier.startsWith('.') && !specifier.startsWith('/'); } +/** + * Extracts the npm package name from a bare specifier. Handles both unscoped (`pkg/sub` → `pkg`) + * and scoped (`@scope/pkg/sub` → `@scope/pkg`) forms. + */ function packageNameOf(specifier: string): string { if (specifier.startsWith('@')) { const [scope, name] = specifier.split('/', 2); @@ -515,6 +674,11 @@ function packageNameOf(specifier: string): string { return slash === -1 ? specifier : specifier.slice(0, slash); } +/** + * Shortens an absolute file path for display in diagnostics: returns the part after the last + * `node_modules/` segment when present (so users see e.g. `tabster/dist/index.js`), or makes the + * path workspace-relative when inside the current working directory. + */ function shortenPath(absolute: string): string { const marker = '/node_modules/'; const idx = absolute.lastIndexOf(marker); @@ -552,6 +716,11 @@ function getImportedName(specifier: ImportSpecifierNode): string | undefined { } } +/** + * Returns a human-readable label for a function parameter, used in `invalidParamName` diagnostics + * so the user sees what they actually wrote (destructuring, rest, default value, …) instead of + * just `Identifier`. + */ function describeParam(param: TSESTree.Parameter): string { switch (param.type) { case AST_NODE_TYPES.Identifier: @@ -569,6 +738,11 @@ function describeParam(param: TSESTree.Parameter): string { } } +/** + * Returns `true` when `annotation` is `React.Ref<...>` (qualified) or `Ref<...>` (named) AND the + * referenced identifier was imported from the `react` package in the surrounding scope. The scope + * check guards against false positives when a local `Ref` shadows the React import. + */ function isReactRefTypeAnnotation( annotation: TSESTree.TSTypeAnnotation | undefined, scope: TSESLint.Scope.Scope, @@ -644,6 +818,11 @@ function isReactImportedIdentifier( }); } +/** + * Walks the scope chain looking for a variable with the given name. Plain `scope.set.get` only + * inspects the local scope, so this helper enables identifier resolution that matches JavaScript's + * lookup semantics. + */ function findVariableInScope(scope: TSESLint.Scope.Scope, name: string): TSESLint.Scope.Variable | undefined { let current: TSESLint.Scope.Scope | null = scope; while (current) { @@ -656,6 +835,10 @@ function findVariableInScope(scope: TSESLint.Scope.Scope, name: string): TSESLin return undefined; } +/** + * Renders the actual ref type annotation as a string for `invalidRefType` diagnostics, so users + * see what they wrote (`HTMLAttributes`, `Ref`, `MyType`…) instead of bare AST node types. + */ function describeRefType(annotation: TSESTree.TSTypeAnnotation | undefined): string { if (!annotation) { return ''; @@ -675,10 +858,15 @@ function describeRefType(annotation: TSESTree.TSTypeAnnotation | undefined): str return type.type; } -function isScopeWithinFunction(scope: TSESLint.Scope.Scope, fn: BaseHookFunction): boolean { +/** + * `true` when `scope` (or any of its ancestor scopes) is the function scope of `hookFn`. Used to + * confine the body-reference walk to the base hook itself — references in sibling functions are + * out of scope for this rule. + */ +function isScopeWithinFunction(scope: TSESLint.Scope.Scope, hookFn: BaseHookFunction): boolean { let current: TSESLint.Scope.Scope | null = scope; while (current) { - if (current.block === fn) { + if (current.block === hookFn) { return true; } current = current.upper; From f441aaf8f194a336236838552d16f9382c38b282 Mon Sep 17 00:00:00 2001 From: Martin Hochel Date: Tue, 26 May 2026 15:02:00 +0200 Subject: [PATCH 15/15] feat(eslint-rules): extend consistent-base-hook with paired state hook signature detection Wrapping state hooks (`use_unstable`) that pair with a base hook (`useBase_unstable`) must also follow the `(props, ref)` signature contract. Pair detection checks the same file first, then sibling files (`.ts`/`.tsx`) in the same directory. Suppress the violation on `useTagGroupBase_unstable`, which takes a third `options` argument used internally by `useTagGroup_unstable`. --- .../src/components/TagGroup/useTagGroup.ts | 1 + .../src/components/Sibling/useSiblingBase.ts | 7 + .../rules/consistent-base-hook.spec.ts | 70 +++++++- .../rules/consistent-base-hook.ts | 164 +++++++++++++++--- 4 files changed, 217 insertions(+), 25 deletions(-) create mode 100644 tools/eslint-rules/rules/__fixtures__/consistent-base-hook/src/components/Sibling/useSiblingBase.ts 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 e6b0b19aca97f..79ace1ae06ba8 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 @@ -27,6 +27,7 @@ type UseTagGroupBaseOptions = { * @param props - props from this instance of TagGroup (without appearance, size) * @param ref - reference to root HTMLDivElement of TagGroup */ +// eslint-disable-next-line @nx/workspace-consistent-base-hook -- accepts an extra `options` arg used internally by `useTagGroup_unstable` to coordinate focus after a tag is dismissed export const useTagGroupBase_unstable = ( props: TagGroupBaseProps, ref: React.Ref, diff --git a/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/src/components/Sibling/useSiblingBase.ts b/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/src/components/Sibling/useSiblingBase.ts new file mode 100644 index 0000000000000..52eaf50766185 --- /dev/null +++ b/tools/eslint-rules/rules/__fixtures__/consistent-base-hook/src/components/Sibling/useSiblingBase.ts @@ -0,0 +1,7 @@ +import * as React from 'react'; + +// Sibling base hook used to verify cross-file pair detection in the rule's spec. +// The wrapping state hook lives in `useSibling.ts` next to this file. +export const useSiblingBase_unstable = (props: { a: number }, ref: React.Ref) => { + return { props, ref }; +}; diff --git a/tools/eslint-rules/rules/consistent-base-hook.spec.ts b/tools/eslint-rules/rules/consistent-base-hook.spec.ts index 58ed8c66266e1..ab03e0222861e 100644 --- a/tools/eslint-rules/rules/consistent-base-hook.spec.ts +++ b/tools/eslint-rules/rules/consistent-base-hook.spec.ts @@ -4,6 +4,8 @@ import { rule, RULE_NAME } from './consistent-base-hook'; const FIXTURE_ROOT = path.join(__dirname, '__fixtures__/consistent-base-hook'); const TYPED_FILENAME = 'src/test.ts'; +const SIBLING_FILENAME = path.join(FIXTURE_ROOT, 'src/components/Sibling/useSibling.ts'); +const ORPHAN_FILENAME = path.join(FIXTURE_ROOT, 'src/components/Orphan/useOrphan.ts'); const typedLanguageOptions = { parserOptions: { @@ -62,12 +64,47 @@ ruleTester.run(RULE_NAME, rule, { }; `, }, + // Pair detection (same file): a state hook `useThing_unstable` next to its base hook + // `useThingBase_unstable` IS subject to the contract. Correct signature passes. + { + code: ` + import * as React from 'react'; + export const useThing_unstable = (props, ref: React.Ref) => { + return { props, ref }; + }; + export const useThingBase_unstable = (props, ref: React.Ref) => { + return { props, ref }; + }; + `, + }, + // Pair detection (no sibling base hook on disk): `useOrphanContextValues_unstable(state)` + // is NOT a paired wrapping hook and must NOT be flagged for its non-(props, ref) signature. + // The Orphan folder has no `useOrphanContextValuesBase.ts(x)` next to it. + { + filename: ORPHAN_FILENAME, + code: ` + export function useOrphanContextValues_unstable(state) { + return { state }; + } + `, + }, + // Pair detection (sibling file): wrapping state hook lives in `useSibling.ts`, paired with + // `useSiblingBase.ts` in the same folder. Correct (props, ref) signature passes. + { + filename: SIBLING_FILENAME, + code: ` + import * as React from 'react'; + export const useSibling_unstable = (props, ref: React.Ref) => { + return { props, ref }; + }; + `, + }, // Identifier with the same local name as a forbidden import alias does not collide via scope analysis. { code: ` import * as React from 'react'; import { useArrowNavigationGroup } from '@fluentui/react-tabster'; - export const useThing_unstable = (props, ref) => { + export const useThing_unstable = (props, ref: React.Ref) => { return useArrowNavigationGroup({}); }; export const useThingBase_unstable = (props, ref: React.Ref) => { @@ -226,6 +263,35 @@ ruleTester.run(RULE_NAME, rule, { }, ], }, + // Pair detection (same file): wrapping state hook with too many params is flagged because + // its sibling base hook in the same file marks it as a paired wrapper. + { + code: ` + import * as React from 'react'; + export const useThing_unstable = (props, ref, extra) => ({ props, ref, extra }); + export const useThingBase_unstable = (props, ref: React.Ref) => ({ props, ref }); + `, + errors: [{ messageId: 'invalidParamCount', data: { hookName: 'useThing_unstable', actual: 3 } }], + }, + // Pair detection (sibling file): wrapping state hook in `useSibling.ts` is paired with + // `useSiblingBase.ts` in the same folder. Wrong param names are flagged. + { + filename: SIBLING_FILENAME, + code: ` + import * as React from 'react'; + export const useSibling_unstable = (p, r: React.Ref) => ({ p, r }); + `, + errors: [ + { + messageId: 'invalidParamName', + data: { hookName: 'useSibling_unstable', index: 1, expected: 'props', actual: 'p' }, + }, + { + messageId: 'invalidParamName', + data: { hookName: 'useSibling_unstable', index: 2, expected: 'ref', actual: 'r' }, + }, + ], + }, ], }); @@ -293,7 +359,7 @@ typedRuleTester.run(`${RULE_NAME} (typed)`, rule, { export const useThingBase_unstable = (props: { a: number }, ref: React.Ref) => { return { props, ref }; }; - export const useThing_unstable = (props, ref) => { + export const useThing_unstable = (props, ref: React.Ref) => { return useHeavy(); }; `, diff --git a/tools/eslint-rules/rules/consistent-base-hook.ts b/tools/eslint-rules/rules/consistent-base-hook.ts index 165582b0b8be1..81da34002f77f 100644 --- a/tools/eslint-rules/rules/consistent-base-hook.ts +++ b/tools/eslint-rules/rules/consistent-base-hook.ts @@ -1,11 +1,20 @@ import type { TSESTree, TSESLint, ParserServicesWithTypeInformation } from '@typescript-eslint/utils'; import { ESLintUtils, AST_NODE_TYPES } from '@typescript-eslint/utils'; import * as ts from 'typescript'; +import * as fs from 'node:fs'; +import * as path from 'node:path'; // 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$/; +// State hook candidates are any `_unstable` hook NOT ending in `Base_unstable`. They are only +// subjected to the signature contract when they are paired with a sibling base hook (Option C — +// pair detection). See `hasPairedBaseHook`. +const STATE_HOOK_NAME_PATTERN = /^use[A-Z]\w*_unstable$/; +const BASE_SUFFIX = 'Base_unstable'; +const UNSTABLE_SUFFIX = '_unstable'; +const SIBLING_EXTENSIONS: ReadonlyArray = ['.ts', '.tsx']; const EXPECTED_PARAM_NAMES = ['props', 'ref'] as const; const MIN_PARAM_COUNT = 1; const MAX_PARAM_COUNT = 2; @@ -101,7 +110,7 @@ export const rule = ESLintUtils.RuleCreator(() => __filename)Base_unstable`): a required `props` parameter and an optional `ref` parameter typed as `React.Ref<...>`, and disallow referencing any binding whose defining module transitively pulls a forbidden runtime package (default `tabster`) \u2014 both at value positions (runtime coupling) and at type positions (API surface coupling).', + 'Enforce the API contract for v9 "base" hooks (`useBase_unstable`) and their paired wrapping state hooks (`use_unstable` declared in the same file or sibling component-folder file): a required `props` parameter and an optional `ref` parameter typed as `React.Ref<...>`. Additionally, disallow inside base hooks any binding whose defining module transitively pulls a forbidden runtime package (default `tabster`) \u2014 both at value positions (runtime coupling) and at type positions (API surface coupling).', }, schema: [ { @@ -126,10 +135,10 @@ export const rule = ESLintUtils.RuleCreator(() => __filename)`, got `{{actual}}`.', + 'Hook `{{hookName}}` parameter #{{index}} must be named `{{expected}}` (Identifier), got `{{actual}}`.', + invalidRefType: 'Hook `{{hookName}}` parameter `ref` must be typed as `React.Ref<...>`, got `{{actual}}`.', forbiddenRuntimeDirect: 'Base hook `{{hookName}}` cannot reference `{{importedName}}` from forbidden runtime package `{{package}}`. Move logic that depends on `{{package}}` to the wrapping `*_unstable` hook instead.', forbiddenRuntimeReach: @@ -152,6 +161,17 @@ export const rule = ESLintUtils.RuleCreator(() => __filename)(); + // Names of base hooks (`useBase_unstable`) declared at the top level of the current file. + // Populated on `Program` enter so the state-hook visitor can synchronously decide pairing for + // the same-file (82 / 85) case without re-scanning. + const baseHooksInCurrentFile = new Set(); + + // Caches sibling-file existence checks (`./useXBase.ts` / `./useXBase.tsx`) for the lifetime of + // this rule instance. The keys are absolute candidate paths; values are `true` if the file exists + // on disk. Used to cover the 3 outliers where the base hook and wrapping hook live in sibling + // files within the same component folder (Tooltip, Field, MenuItem). + const siblingFileExistsCache = new Map(); + // Tracks whether `computeSymbolReach` was invoked while typed services were unavailable. When set, // we emit a single diagnostic on `Program:exit` so the user knows transitive analysis was skipped. let typedServicesNeededButMissing = false; @@ -185,6 +205,50 @@ export const rule = ESLintUtils.RuleCreator(() => __filename)/useXBase.ts(x)`). + * + * The base hook is the structural marker for the `(props, ref)` contract. When found, the + * wrapping state hook is required to honor the same contract; otherwise it is left alone, + * which avoids false positives on unrelated `_unstable` hooks such as + * `useFooContextValues_unstable`, `useFooStyles_unstable`, etc. + */ + function hasPairedBaseHook(stateHookName: string): boolean { + const baseHookName = stateHookName.slice(0, -UNSTABLE_SUFFIX.length) + BASE_SUFFIX; + if (baseHooksInCurrentFile.has(baseHookName)) { + return true; + } + const filename = context.filename; + // ESLint passes synthetic filenames like `` for inline code; nothing to check. + if (!filename || !path.isAbsolute(filename)) { + return false; + } + const dir = path.dirname(filename); + // Sibling base-hook file (the wrapping hook lives in `useFoo.tsx`, the base in `useFooBase.tsx`). + const siblingBasename = baseHookName.slice(0, -UNSTABLE_SUFFIX.length); // e.g. `useFooBase` + for (const ext of SIBLING_EXTENSIONS) { + const candidate = path.join(dir, siblingBasename + ext); + if (candidate === filename) { + continue; + } + let exists = siblingFileExistsCache.get(candidate); + if (exists === undefined) { + try { + exists = fs.statSync(candidate).isFile(); + } catch { + exists = false; + } + siblingFileExistsCache.set(candidate, exists); + } + if (exists) { + return true; + } + } + return false; + } + /** * Validates the base-hook signature: 1 or 2 positional params, first must be Identifier `props`, * optional second must be Identifier `ref` typed as `React.Ref<...>` (verified to originate from @@ -406,39 +470,65 @@ export const rule = ESLintUtils.RuleCreator(() => __filename) { + // Broader selector matches both base hooks and state-hook candidates; the handler dispatches + // by name. State hooks only get the signature check when paired with a sibling base hook. + [`FunctionDeclaration[id.name=/${STATE_HOOK_NAME_PATTERN.source}/]`]: (node: TSESTree.FunctionDeclaration) => { // `export default function () {}` produces an anonymous FunctionDeclaration (id === null). // The esquery selector above requires `id.name`, so this branch should be unreachable in // practice — kept as a type-narrowing guard so TS treats `node.id` as non-null below. if (!node.id) { return; } - checkBaseHook(node.id.name, node, node.id); + const name = node.id.name; + if (BASE_HOOK_NAME_PATTERN.test(name)) { + checkBaseHook(name, node, node.id); + } else if (hasPairedBaseHook(name)) { + checkParameters(name, node, node.id); + } }, - [`VariableDeclarator[id.name=/${BASE_HOOK_NAME_PATTERN.source}/]`]: (node: TSESTree.VariableDeclarator) => { - // The declarator's `id` can be a destructuring pattern (e.g. `const { useFooBase_unstable } = mod`). - // Such patterns never define a function literal we can inspect, and the selector only matched - // because esquery probed the nested Identifier name. Skip anything that isn't a plain Identifier. - if (node.id.type !== AST_NODE_TYPES.Identifier) { + [`VariableDeclarator[id.name=/${STATE_HOOK_NAME_PATTERN.source}/]`]: (node: TSESTree.VariableDeclarator) => { + const init = getFunctionInit(node); + if (!init || node.id.type !== AST_NODE_TYPES.Identifier) { return; } - // We only check inline function literals (`= (props, ref) => {}` or `= function () {}`) because - // the param-shape / body-reference analysis needs a function node we can walk. Any other Right Hand Side - // — missing initializer (`let useFooBase_unstable;`, ambient `declare const ... : Hook`), a call - // expression (`= wrap(impl)`), an identifier alias (`= someOtherHook`), `require(...)`, etc. — - // is out of scope for this rule. - const init = node.init; - if ( - !init || - (init.type !== AST_NODE_TYPES.ArrowFunctionExpression && init.type !== AST_NODE_TYPES.FunctionExpression) - ) { - return; + const name = node.id.name; + if (BASE_HOOK_NAME_PATTERN.test(name)) { + checkBaseHook(name, init, node.id); + } else if (hasPairedBaseHook(name)) { + checkParameters(name, init, node.id); } - checkBaseHook(node.id.name, init, node.id); }, /** @@ -696,6 +786,34 @@ function shortenPath(absolute: string): string { // AST helpers (unchanged from previous version) // --------------------------------------------------------------------------- +/** + * Collects names of top-level declarations matching `BASE_HOOK_NAME_PATTERN` into `out`. + * Handles both `export const useFooBase_unstable = ...` (incl. `export const` chains) and + * `export function useFooBase_unstable() {}`, plus the unexported / `export { ... }` forms. + */ +function collectBaseHookNames(stmt: TSESTree.Node, out: Set): void { + // `export const useFooBase_unstable = ...` / `export function useFooBase_unstable() {}` + if (stmt.type === AST_NODE_TYPES.ExportNamedDeclaration && stmt.declaration) { + collectBaseHookNames(stmt.declaration, out); + return; + } + // `function useFooBase_unstable() {}` + if (stmt.type === AST_NODE_TYPES.FunctionDeclaration) { + if (stmt.id && BASE_HOOK_NAME_PATTERN.test(stmt.id.name)) { + out.add(stmt.id.name); + } + return; + } + // `const useFooBase_unstable = ...` (incl. multi-declarator forms) + if (stmt.type === AST_NODE_TYPES.VariableDeclaration) { + for (const decl of stmt.declarations) { + if (decl.id.type === AST_NODE_TYPES.Identifier && BASE_HOOK_NAME_PATTERN.test(decl.id.name)) { + out.add(decl.id.name); + } + } + } +} + /** * Resolves the original (imported) name from an import specifier. * Returns `'default'` for default imports, `'*'` for namespace imports,