diff --git a/change/@fluentui-react-avatar-6892f968-8be8-4e9c-9fc6-b747d59db983.json b/change/@fluentui-react-avatar-6892f968-8be8-4e9c-9fc6-b747d59db983.json new file mode 100644 index 00000000000000..456a1485aaf822 --- /dev/null +++ b/change/@fluentui-react-avatar-6892f968-8be8-4e9c-9fc6-b747d59db983.json @@ -0,0 +1,7 @@ +{ + "type": "none", + "comment": "chore: forbid react-icons, react-motion and Griffel in v9 base hooks", + "packageName": "@fluentui/react-avatar", + "email": "martinhochel@microsoft.com", + "dependentChangeType": "none" +} diff --git a/change/@fluentui-react-combobox-55618c76-2dae-4993-a52c-d0269f1f96de.json b/change/@fluentui-react-combobox-55618c76-2dae-4993-a52c-d0269f1f96de.json new file mode 100644 index 00000000000000..e9b8caf347c6b9 --- /dev/null +++ b/change/@fluentui-react-combobox-55618c76-2dae-4993-a52c-d0269f1f96de.json @@ -0,0 +1,7 @@ +{ + "type": "none", + "comment": "chore: forbid react-icons, react-motion and Griffel in v9 base hooks", + "packageName": "@fluentui/react-combobox", + "email": "martinhochel@microsoft.com", + "dependentChangeType": "none" +} diff --git a/change/@fluentui-react-headless-components-preview-9d9458fe-d4dc-4b37-90a0-e5e2c5fe7807.json b/change/@fluentui-react-headless-components-preview-9d9458fe-d4dc-4b37-90a0-e5e2c5fe7807.json new file mode 100644 index 00000000000000..cb72f343ae73c0 --- /dev/null +++ b/change/@fluentui-react-headless-components-preview-9d9458fe-d4dc-4b37-90a0-e5e2c5fe7807.json @@ -0,0 +1,7 @@ +{ + "type": "none", + "comment": "chore: forbid react-icons, react-motion and Griffel in v9 base hooks", + "packageName": "@fluentui/react-headless-components-preview", + "email": "martinhochel@microsoft.com", + "dependentChangeType": "none" +} diff --git a/change/@fluentui-react-menu-639031a5-ca32-46af-9da2-9f57d8fc50d6.json b/change/@fluentui-react-menu-639031a5-ca32-46af-9da2-9f57d8fc50d6.json new file mode 100644 index 00000000000000..1a1ec54bd574d2 --- /dev/null +++ b/change/@fluentui-react-menu-639031a5-ca32-46af-9da2-9f57d8fc50d6.json @@ -0,0 +1,7 @@ +{ + "type": "none", + "comment": "chore: forbid react-icons, react-motion and Griffel in v9 base hooks", + "packageName": "@fluentui/react-menu", + "email": "martinhochel@microsoft.com", + "dependentChangeType": "none" +} diff --git a/change/@fluentui-react-message-bar-9726a17c-0433-4787-b72b-90ba58ebfe61.json b/change/@fluentui-react-message-bar-9726a17c-0433-4787-b72b-90ba58ebfe61.json new file mode 100644 index 00000000000000..b0420f085ce704 --- /dev/null +++ b/change/@fluentui-react-message-bar-9726a17c-0433-4787-b72b-90ba58ebfe61.json @@ -0,0 +1,7 @@ +{ + "type": "none", + "comment": "chore: forbid react-icons, react-motion and Griffel in v9 base hooks", + "packageName": "@fluentui/react-message-bar", + "email": "martinhochel@microsoft.com", + "dependentChangeType": "none" +} diff --git a/packages/eslint-plugin/src/internal.js b/packages/eslint-plugin/src/internal.js index 323c79840c04d8..2eb42d2232cef4 100644 --- a/packages/eslint-plugin/src/internal.js +++ b/packages/eslint-plugin/src/internal.js @@ -12,6 +12,26 @@ function shouldRegisterInternal() { const shouldRegister = shouldRegisterInternal(); +/** + * Runtime packages that must not be reachable from v9 base hooks (`useBase_unstable`). + * + * Configured explicitly instead of relying on the rule default so the boundary enforced at lint + * time stays in sync with, and visible next to, the `forbiddenPackages` asserted at build time by + * `bundle-isolation.config.json`. + * + * Names are matched exactly, so a package and its satellites have to be listed separately — + * `@fluentui/react-motion-components-preview` is not covered by `@fluentui/react-motion`, and the + * `@griffel/*` glob the bundle config uses has to be spelled out here. + */ +const baseHookForbiddenRuntimes = [ + 'tabster', + '@fluentui/react-icons', + '@fluentui/react-motion', + '@fluentui/react-motion-components-preview', + '@griffel/react', + '@griffel/core', +]; + /** * * this will be removed after https://github.com/microsoft/fluentui/issues/30332 @@ -36,7 +56,7 @@ const __internal = { rules: { '@nx/workspace-consistent-callback-type': 'error', '@nx/workspace-base-hook-signature': 'error', - '@nx/workspace-base-hook-no-forbidden-runtime': 'error', + '@nx/workspace-base-hook-no-forbidden-runtime': ['error', { forbiddenRuntimes: baseHookForbiddenRuntimes }], '@nx/workspace-no-restricted-globals': restrictedGlobals.react, '@nx/workspace-no-missing-jsx-pragma': ['error', { runtime: 'automatic' }], }, diff --git a/packages/react-components/react-avatar/library/src/components/AvatarGroupPopover/useAvatarGroupPopover.tsx b/packages/react-components/react-avatar/library/src/components/AvatarGroupPopover/useAvatarGroupPopover.tsx index fdbbfa9ca6c64c..46f42419544ba4 100644 --- a/packages/react-components/react-avatar/library/src/components/AvatarGroupPopover/useAvatarGroupPopover.tsx +++ b/packages/react-components/react-avatar/library/src/components/AvatarGroupPopover/useAvatarGroupPopover.tsx @@ -71,7 +71,14 @@ export const useAvatarGroupPopover_unstable = (props: AvatarGroupPopoverProps): * @param props - AvatarGroupPopover props * @returns AvatarGroupPopover state */ -export const useAvatarGroupPopoverBase_unstable = (props: AvatarGroupPopoverBaseProps): AvatarGroupPopoverBaseState => { +export const useAvatarGroupPopoverBase_unstable = ( + // Tracked debt: unlike the other base types these subtract only `size`, so the `root` slot keeps + // `PopoverProps` — and with it `surfaceMotion` — in the base API surface. Type-only, so nothing + // is emitted; clearing it means dropping the motion slot from the base props/state. + // eslint-disable-next-line @nx/workspace-base-hook-no-forbidden-runtime + props: AvatarGroupPopoverBaseProps, + // eslint-disable-next-line @nx/workspace-base-hook-no-forbidden-runtime +): AvatarGroupPopoverBaseState => { const layout = useAvatarGroupContext_unstable(ctx => ctx.layout); const { indicator = 'count', count = React.Children.count(props.children), children, ...restOfProps } = props; diff --git a/packages/react-components/react-combobox/library/src/components/Combobox/useCombobox.tsx b/packages/react-components/react-combobox/library/src/components/Combobox/useCombobox.tsx index 1dba0dfeffc572..092aa1da42eee4 100644 --- a/packages/react-components/react-combobox/library/src/components/Combobox/useCombobox.tsx +++ b/packages/react-components/react-combobox/library/src/components/Combobox/useCombobox.tsx @@ -65,6 +65,7 @@ export const useComboboxBase_unstable = ( const triggerRef = React.useRef(null); + // eslint-disable-next-line @nx/workspace-base-hook-no-forbidden-runtime -- tracked debt: styled slot pulls Griffel const listbox = useListboxSlot(props.listbox, useMergedRefs(comboboxPopupRef, activeDescendantListboxRef), { state: comboboxInternalState, triggerRef, @@ -97,6 +98,7 @@ export const useComboboxBase_unstable = ( const showClearIcon = selectedOptions.length > 0 && !disabled && clearable && !multiselect; const state: BaseComboboxState = { + // eslint-disable-next-line @nx/workspace-base-hook-no-forbidden-runtime -- tracked debt: styled slot pulls Griffel components: { root: 'div', input: 'input', expandIcon: 'span', listbox: Listbox, clearIcon: 'span' }, root: rootSlot, input: triggerSlot, diff --git a/packages/react-components/react-combobox/library/src/components/Dropdown/useDropdown.tsx b/packages/react-components/react-combobox/library/src/components/Dropdown/useDropdown.tsx index 135b1296141b02..48c4c1039a5132 100644 --- a/packages/react-components/react-combobox/library/src/components/Dropdown/useDropdown.tsx +++ b/packages/react-components/react-combobox/library/src/components/Dropdown/useDropdown.tsx @@ -56,6 +56,7 @@ export const useDropdownBase_unstable = ( const [comboboxPopupRef, comboboxTargetRef] = useComboboxPositioning(props); const triggerRef = React.useRef(null); + // eslint-disable-next-line @nx/workspace-base-hook-no-forbidden-runtime -- tracked debt: styled slot pulls Griffel const listbox = useListboxSlot(props.listbox, useMergedRefs(comboboxPopupRef, activeDescendantListboxRef), { state: dropdownInternalState, triggerRef, @@ -98,6 +99,7 @@ export const useDropdownBase_unstable = ( const showClearButton = selectedOptions.length > 0 && !disabled && clearable && !multiselect; const state: DropdownBaseState = { + // eslint-disable-next-line @nx/workspace-base-hook-no-forbidden-runtime -- tracked debt: styled slot pulls Griffel components: { root: 'div', button: 'button', clearButton: 'button', expandIcon: 'span', listbox: Listbox }, root: rootSlot, button: trigger, diff --git a/packages/react-components/react-headless-components-preview/library/bundle-isolation.config.json b/packages/react-components/react-headless-components-preview/library/bundle-isolation.config.json index 66d5e19222b520..fa5c6d1a98297f 100644 --- a/packages/react-components/react-headless-components-preview/library/bundle-isolation.config.json +++ b/packages/react-components/react-headless-components-preview/library/bundle-isolation.config.json @@ -2,6 +2,12 @@ "$schema": "../../../../tools/verify-bundle-isolation/schema.json", "fixturesRoot": "./bundle-size", "externals": ["react", "react-dom", "react/jsx-runtime", "react/compiler-runtime"], - "forbiddenPackages": ["tabster", "@griffel/*", "@fluentui/react-icons"], + "forbiddenPackages": [ + "tabster", + "@griffel/*", + "@fluentui/react-icons", + "@fluentui/react-motion", + "@fluentui/react-motion-components-preview" + ], "allowedViolations": {} } diff --git a/packages/react-components/react-menu/library/src/components/Menu/useMenu.tsx b/packages/react-components/react-menu/library/src/components/Menu/useMenu.tsx index 72123147238176..df6c1ab5b488d6 100644 --- a/packages/react-components/react-menu/library/src/components/Menu/useMenu.tsx +++ b/packages/react-components/react-menu/library/src/components/Menu/useMenu.tsx @@ -150,6 +150,7 @@ export const useMenuBase_unstable = ( mouseCoordinates: { x: 0, y: 0 }, }); + // eslint-disable-next-line @nx/workspace-base-hook-no-forbidden-runtime -- tracked debt: SafeZoneArea pulls Griffel const safeZoneHandle = useSafeZoneArea({ disabled: !enableSafeZone, timeout: typeof safeZone === 'object' ? safeZone.timeout : 300, diff --git a/tools/eslint-rules/rules/__fixtures__/base-hook-no-forbidden-runtime/stubs/cyclic-heavy-pkg/package.json b/tools/eslint-rules/rules/__fixtures__/base-hook-no-forbidden-runtime/stubs/cyclic-heavy-pkg/package.json new file mode 100644 index 00000000000000..3a620c685ad6d1 --- /dev/null +++ b/tools/eslint-rules/rules/__fixtures__/base-hook-no-forbidden-runtime/stubs/cyclic-heavy-pkg/package.json @@ -0,0 +1,5 @@ +{ + "name": "cyclic-heavy-pkg", + "version": "1.0.0", + "main": "index.ts" +} diff --git a/tools/eslint-rules/rules/__fixtures__/base-hook-no-forbidden-runtime/stubs/heavy-runtime/package.json b/tools/eslint-rules/rules/__fixtures__/base-hook-no-forbidden-runtime/stubs/heavy-runtime/package.json new file mode 100644 index 00000000000000..c0d1eed26d32ea --- /dev/null +++ b/tools/eslint-rules/rules/__fixtures__/base-hook-no-forbidden-runtime/stubs/heavy-runtime/package.json @@ -0,0 +1,5 @@ +{ + "name": "heavy-runtime", + "version": "1.0.0", + "main": "index.ts" +} diff --git a/tools/eslint-rules/rules/__fixtures__/base-hook-no-forbidden-runtime/stubs/watched-pkg/heavy.ts b/tools/eslint-rules/rules/__fixtures__/base-hook-no-forbidden-runtime/stubs/watched-pkg/heavy.ts index fc570732bb31d5..02c9ade6a8d6d4 100644 --- a/tools/eslint-rules/rules/__fixtures__/base-hook-no-forbidden-runtime/stubs/watched-pkg/heavy.ts +++ b/tools/eslint-rules/rules/__fixtures__/base-hook-no-forbidden-runtime/stubs/watched-pkg/heavy.ts @@ -10,3 +10,16 @@ export type CleanTag = { tag: 'clean' }; export function useHeavy(): { tag: 'heavy' } { return runHeavy(); } + +// Mirrors the v9 shape: a styled props bag carrying a forbidden-runtime slot, and the base bag +// derived from it by subtracting exactly that member. +export type StyledProps = { tag: 'styled'; motion?: HeavyOptions; label: string }; + +export type DerivedBaseProps = Omit; + +// Only coupling is the constraint of a type parameter — the parameter itself is declared here, +// not in the forbidden runtime, so the reach is invisible unless constraints are followed. +export type HeavyConstrainedCallback = (value: T) => void; + +// Same shape, but with a constraint that stays inside this package. +export type CleanConstrainedCallback = (value: T) => void; diff --git a/tools/eslint-rules/rules/__fixtures__/base-hook-no-forbidden-runtime/stubs/watched-pkg/index.ts b/tools/eslint-rules/rules/__fixtures__/base-hook-no-forbidden-runtime/stubs/watched-pkg/index.ts index 324be823ed5554..0142943d89d505 100644 --- a/tools/eslint-rules/rules/__fixtures__/base-hook-no-forbidden-runtime/stubs/watched-pkg/index.ts +++ b/tools/eslint-rules/rules/__fixtures__/base-hook-no-forbidden-runtime/stubs/watched-pkg/index.ts @@ -8,3 +8,7 @@ export type { HeavyType } from './heavy'; export type { CleanTag } from './heavy'; export type HeavyWrapper = { tag: 'heavy-wrapper'; inner: HeavyType }; +export type { StyledProps } from './heavy'; +export type { DerivedBaseProps } from './heavy'; +export type { HeavyConstrainedCallback } from './heavy'; +export type { CleanConstrainedCallback } from './heavy'; diff --git a/tools/eslint-rules/rules/__fixtures__/base-hook-no-forbidden-runtime/stubs/workspace-relay-pkg/index.ts b/tools/eslint-rules/rules/__fixtures__/base-hook-no-forbidden-runtime/stubs/workspace-relay-pkg/index.ts new file mode 100644 index 00000000000000..afbc7109d96a46 --- /dev/null +++ b/tools/eslint-rules/rules/__fixtures__/base-hook-no-forbidden-runtime/stubs/workspace-relay-pkg/index.ts @@ -0,0 +1,5 @@ +// Pure re-export of a workspace forbidden runtime. `getAliasedSymbol` collapses the whole chain +// in one hop, so the intermediate `workspace-runtime` specifier is never visited and ownership +// has to be recovered from the leaf declaration's own package manifest. +export { runWorkspaceHeavy } from 'workspace-runtime'; +export type { WorkspaceHeavyOptions } from 'workspace-runtime'; diff --git a/tools/eslint-rules/rules/__fixtures__/base-hook-no-forbidden-runtime/stubs/workspace-runtime/index.ts b/tools/eslint-rules/rules/__fixtures__/base-hook-no-forbidden-runtime/stubs/workspace-runtime/index.ts new file mode 100644 index 00000000000000..117eba3dd31a38 --- /dev/null +++ b/tools/eslint-rules/rules/__fixtures__/base-hook-no-forbidden-runtime/stubs/workspace-runtime/index.ts @@ -0,0 +1,7 @@ +// A forbidden runtime that ships as a *workspace* package: it is path-mapped straight to source, +// so its files carry no `node_modules` segment to read a package name from. +export function runWorkspaceHeavy(): { tag: 'workspace-heavy' } { + return { tag: 'workspace-heavy' }; +} + +export type WorkspaceHeavyOptions = { kind: 'workspace-heavy' }; diff --git a/tools/eslint-rules/rules/__fixtures__/base-hook-no-forbidden-runtime/stubs/workspace-runtime/package.json b/tools/eslint-rules/rules/__fixtures__/base-hook-no-forbidden-runtime/stubs/workspace-runtime/package.json new file mode 100644 index 00000000000000..bcb4679cfa2dd6 --- /dev/null +++ b/tools/eslint-rules/rules/__fixtures__/base-hook-no-forbidden-runtime/stubs/workspace-runtime/package.json @@ -0,0 +1,5 @@ +{ + "name": "workspace-runtime", + "version": "1.0.0", + "main": "index.ts" +} diff --git a/tools/eslint-rules/rules/__fixtures__/base-hook-no-forbidden-runtime/tsconfig.json b/tools/eslint-rules/rules/__fixtures__/base-hook-no-forbidden-runtime/tsconfig.json index 0e0e24ef90756f..eeb2d405aa00c9 100644 --- a/tools/eslint-rules/rules/__fixtures__/base-hook-no-forbidden-runtime/tsconfig.json +++ b/tools/eslint-rules/rules/__fixtures__/base-hook-no-forbidden-runtime/tsconfig.json @@ -18,6 +18,8 @@ "relay-pkg": ["./stubs/relay-pkg/index.ts"], "component-pkg": ["./stubs/component-pkg/index.ts"], "typed-dist-pkg": ["./stubs/typed-dist-pkg/index.d.ts"], + "workspace-runtime": ["./stubs/workspace-runtime/index.ts"], + "workspace-relay-pkg": ["./stubs/workspace-relay-pkg/index.ts"], "heavy-runtime": ["./stubs/heavy-runtime/index.ts"], "heavy-runtime/*": ["./stubs/heavy-runtime/*"], "light-helper": ["./stubs/light-helper/index.ts"], diff --git a/tools/eslint-rules/rules/base-hook-no-forbidden-runtime.spec.ts b/tools/eslint-rules/rules/base-hook-no-forbidden-runtime.spec.ts index ccd9b8c3d58f01..6ec3d07b31716b 100644 --- a/tools/eslint-rules/rules/base-hook-no-forbidden-runtime.spec.ts +++ b/tools/eslint-rules/rules/base-hook-no-forbidden-runtime.spec.ts @@ -100,6 +100,15 @@ const transitiveOptionsAllowTypeImports: readonly [{ forbiddenRuntimes: string[] }, ]; +// `workspace-runtime` stands in for a forbidden runtime that lives in the repo rather than in +// `node_modules` (e.g. `@fluentui/react-motion`), which TypeScript resolves through `paths` +// straight to source. +const workspaceRuntimeOptions: readonly [{ forbiddenRuntimes: string[] }] = [ + { + forbiddenRuntimes: ['workspace-runtime'], + }, +]; + typedRuleTester.run(`${RULE_NAME} (typed)`, rule, { valid: [ // The defining file of \`useLight\` only reaches \`light-helper\`, not \`heavy-runtime\`. @@ -129,6 +138,33 @@ typedRuleTester.run(`${RULE_NAME} (typed)`, rule, { }; `, }, + // A base props bag derived from a styled one by subtracting the forbidden-runtime member is + // clean: `Omit` really removes it, so the resolved type has no coupling left. The declaration + // still *mentions* `StyledProps`, which is why this has to be answered structurally rather + // than by following declaration syntax. + { + languageOptions: typedLanguageOptions, + filename: TYPED_FILENAME, + options: transitiveOptions, + code: ` + import type { DerivedBaseProps } from 'watched-pkg'; + export const useThingBase_unstable = (props: DerivedBaseProps, ref) => { + return { props, ref }; + }; + `, + }, + // A type parameter whose constraint stays inside the watched package couples nothing. + { + languageOptions: typedLanguageOptions, + filename: TYPED_FILENAME, + options: transitiveOptions, + code: ` + import type { CleanConstrainedCallback } from 'watched-pkg'; + export const useThingBase_unstable = (props: { cb: CleanConstrainedCallback }, ref) => { + return { props, ref }; + }; + `, + }, // Watched-package import exists but only used by a non-base hook in the same file. { languageOptions: typedLanguageOptions, @@ -460,7 +496,7 @@ typedRuleTester.run(`${RULE_NAME} (typed)`, rule, { importedName: 'HeavyType', package: 'watched-pkg', runtime: 'heavy-runtime', - viaFile: 'rules/__fixtures__/base-hook-no-forbidden-runtime/stubs/watched-pkg/heavy.ts', + viaFile: 'rules/__fixtures__/base-hook-no-forbidden-runtime/stubs/heavy-runtime/index.ts', }, }, ], @@ -485,7 +521,57 @@ typedRuleTester.run(`${RULE_NAME} (typed)`, rule, { importedName: 'HeavyType', package: 'watched-pkg', runtime: 'heavy-runtime', - viaFile: 'rules/__fixtures__/base-hook-no-forbidden-runtime/stubs/watched-pkg/heavy.ts', + viaFile: 'rules/__fixtures__/base-hook-no-forbidden-runtime/stubs/heavy-runtime/index.ts', + }, + }, + ], + }, + // The counterpart of the `Omit` case: while the member is still present the coupling is real + // and must be reported, so the structural check is not simply blind to subtraction. + { + languageOptions: typedLanguageOptions, + filename: TYPED_FILENAME, + options: transitiveOptions, + code: ` + import type { StyledProps } from 'watched-pkg'; + export const useThingBase_unstable = (props: StyledProps, ref) => { + return { props, ref }; + }; + `, + errors: [ + { + messageId: 'forbiddenRuntimeReach', + data: { + hookName: 'useThingBase_unstable', + importedName: 'StyledProps', + package: 'watched-pkg', + runtime: 'heavy-runtime', + viaFile: 'rules/__fixtures__/base-hook-no-forbidden-runtime/stubs/heavy-runtime/index.ts', + }, + }, + ], + }, + // A type parameter is not an object type, so its constraint is only seen if the walk resolves + // it explicitly — yet the constraint is part of the signature the base hook exposes. + { + languageOptions: typedLanguageOptions, + filename: TYPED_FILENAME, + options: transitiveOptions, + code: ` + import type { HeavyConstrainedCallback } from 'watched-pkg'; + export const useThingBase_unstable = (props: { cb: HeavyConstrainedCallback }, ref) => { + return { props, ref }; + }; + `, + errors: [ + { + messageId: 'forbiddenRuntimeReach', + data: { + hookName: 'useThingBase_unstable', + importedName: 'HeavyConstrainedCallback', + package: 'watched-pkg', + runtime: 'heavy-runtime', + viaFile: 'rules/__fixtures__/base-hook-no-forbidden-runtime/stubs/heavy-runtime/index.ts', }, }, ], @@ -510,7 +596,7 @@ typedRuleTester.run(`${RULE_NAME} (typed)`, rule, { importedName: 'HeavyWrapper', package: 'watched-pkg', runtime: 'heavy-runtime', - viaFile: 'rules/__fixtures__/base-hook-no-forbidden-runtime/stubs/watched-pkg/heavy.ts', + viaFile: 'rules/__fixtures__/base-hook-no-forbidden-runtime/stubs/heavy-runtime/index.ts', }, }, ], @@ -540,6 +626,56 @@ typedRuleTester.run(`${RULE_NAME} (typed)`, rule, { }, ], }, + // A workspace forbidden runtime reached through a pure re-export barrel. The alias collapse + // skips the intermediate `workspace-runtime` specifier, and the leaf declaration is + // path-mapped to source, so ownership can only be recovered from its package manifest. + { + languageOptions: typedLanguageOptions, + filename: TYPED_FILENAME, + options: workspaceRuntimeOptions, + code: ` + import { runWorkspaceHeavy } from 'workspace-relay-pkg'; + export const useThingBase_unstable = (props: { a: number }, ref) => { + return { props, ref, x: runWorkspaceHeavy() }; + }; + `, + errors: [ + { + messageId: 'forbiddenRuntimeReach', + data: { + hookName: 'useThingBase_unstable', + importedName: 'runWorkspaceHeavy', + package: 'workspace-relay-pkg', + runtime: 'workspace-runtime', + viaFile: 'rules/__fixtures__/base-hook-no-forbidden-runtime/stubs/workspace-runtime/index.ts', + }, + }, + ], + }, + // Same for a type re-exported from a workspace forbidden runtime — API coupling, not just runtime. + { + languageOptions: typedLanguageOptions, + filename: TYPED_FILENAME, + options: workspaceRuntimeOptions, + code: ` + import type { WorkspaceHeavyOptions } from 'workspace-relay-pkg'; + export const useThingBase_unstable = (props: { a: WorkspaceHeavyOptions }, ref) => { + return { props, ref }; + }; + `, + errors: [ + { + messageId: 'forbiddenRuntimeReach', + data: { + hookName: 'useThingBase_unstable', + importedName: 'WorkspaceHeavyOptions', + package: 'workspace-relay-pkg', + runtime: 'workspace-runtime', + viaFile: 'rules/__fixtures__/base-hook-no-forbidden-runtime/stubs/workspace-runtime/index.ts', + }, + }, + ], + }, // Relative imports are analyzed too: `./local-trigger` -> `./local-heavy` -> `heavy-runtime`. // This is the shape that let `@fluentui/react-tabster` leak into `useDropdownBase_unstable`. { @@ -634,7 +770,7 @@ typedRuleTester.run(`${RULE_NAME} (typed)`, rule, { importedName: 'DistHeavy', package: 'typed-dist-pkg', runtime: 'heavy-runtime', - viaFile: 'rules/__fixtures__/base-hook-no-forbidden-runtime/stubs/typed-dist-pkg/index.d.ts', + viaFile: 'rules/__fixtures__/base-hook-no-forbidden-runtime/stubs/heavy-runtime/index.ts', }, }, ], @@ -687,3 +823,73 @@ typedRuleTester.run(`${RULE_NAME} (typed)`, rule, { }, ], }); + +// --------------------------------------------------------------------------- +// Cache partitioning by forbidden-runtime set. +// +// An answer is only true relative to the set it was computed for, while the Program (and the +// symbols and types in it) is shared by every configuration pointed at the same tsconfig. These +// runs are ordered on purpose: each one re-asks about `useHeavy` after a previous run has already +// cached an answer for it under a *different* set. +// --------------------------------------------------------------------------- +const heavyReference = ` + import { useHeavy } from 'watched-pkg'; + export const useThingBase_unstable = (props, ref) => { + return { props, ref, x: useHeavy() }; + }; +`; + +const heavyReferenceError = [ + { + messageId: 'forbiddenRuntimeReach' as const, + data: { + hookName: 'useThingBase_unstable', + importedName: 'useHeavy', + package: 'watched-pkg', + runtime: 'heavy-runtime', + viaFile: 'rules/__fixtures__/base-hook-no-forbidden-runtime/stubs/watched-pkg/heavy.ts', + }, + }, +]; + +// 1. Seeds the `heavy-runtime` bucket with a hit. +new RuleTester().run(`${RULE_NAME} (typed, cache seeded for heavy-runtime)`, rule, { + valid: [], + invalid: [ + { + languageOptions: typedLanguageOptions, + filename: TYPED_FILENAME, + options: transitiveOptions, + code: heavyReference, + errors: heavyReferenceError, + }, + ], +}); + +// 2. A configuration that does not ban `heavy-runtime` must not inherit that hit — otherwise it +// is told about a package it explicitly allows. +new RuleTester().run(`${RULE_NAME} (typed, hit not reused by a set that allows it)`, rule, { + valid: [ + { + languageOptions: typedLanguageOptions, + filename: TYPED_FILENAME, + options: workspaceRuntimeOptions, + code: heavyReference, + }, + ], + invalid: [], +}); + +// 3. ...and the clean answer just cached for that set must not suppress the real one. +new RuleTester().run(`${RULE_NAME} (typed, clean result not reused by a set that forbids it)`, rule, { + valid: [], + invalid: [ + { + languageOptions: typedLanguageOptions, + filename: TYPED_FILENAME, + options: transitiveOptions, + code: heavyReference, + errors: heavyReferenceError, + }, + ], +}); diff --git a/tools/eslint-rules/rules/base-hook-no-forbidden-runtime.ts b/tools/eslint-rules/rules/base-hook-no-forbidden-runtime.ts index d58d28edcf6428..771bba2bcbff4b 100644 --- a/tools/eslint-rules/rules/base-hook-no-forbidden-runtime.ts +++ b/tools/eslint-rules/rules/base-hook-no-forbidden-runtime.ts @@ -1,5 +1,6 @@ import type { TSESTree, TSESLint, ParserServicesWithTypeInformation } from '@typescript-eslint/utils'; import { ESLintUtils, AST_NODE_TYPES } from '@typescript-eslint/utils'; +import * as fs from 'node:fs'; import * as path from 'node:path'; import * as ts from 'typescript'; @@ -98,15 +99,23 @@ interface Hit { } /** - * Per-Program memo of symbol-level results, split by whether type positions were followed. + * Per-Program memo of symbol-level results, split by the kind of coupling being asked about. * Keyed by `ts.Program` identity so the cache dies with the Program that produced the symbols. */ interface AnalysisCache { value: Map; - all: Map; + type: Map; } -const programCache = new WeakMap(); +/** + * Every cached answer is only true *relative to a forbidden-runtime set*, so the memo is + * partitioned by that set as well as by Program. Sharing one bucket across configurations lets a + * result computed for one ban list be served to another that bans different packages — producing + * both spurious diagnostics (a runtime the second config allows) and missed ones (a `null` cached + * before the package was banned). Configurations with identical ban lists still share a bucket, + * which is where the win of this cache actually comes from. + */ +const programCache = new WeakMap>(); export const rule = ESLintUtils.RuleCreator(() => __filename)({ name: RULE_NAME, @@ -298,7 +307,9 @@ export const rule = ESLintUtils.RuleCreator(() => __filename), ): Hit | null { - const caches = getAnalysisCache(program); - const cache = followTypes ? caches.all : caches.value; + const followTypes = false; + const cache = getAnalysisCache(program, forbiddenRuntimes).value; const inProgress = new Set(); function visitSymbol(current: ts.Symbol): Hit | null { @@ -591,15 +601,226 @@ function findForbiddenRuntime( return visitSymbol(symbol); } -function getAnalysisCache(program: ts.Program): AnalysisCache { - let cache = programCache.get(program); +/** + * Answers "does the *resolved* type of `symbol` expose a forbidden runtime in its API surface?" + * + * Unlike the runtime analysis this cannot work on declaration syntax. A base type is routinely + * derived from a styled one by subtracting the motion/focus members — + * `Omit` — and syntactically that declaration still + * mentions `ProgressBarProps`, which mentions the forbidden slot type. Following those references + * reports coupling the resolved type does not have. Asking the type checker instead makes the + * subtraction visible: the erased member is simply not among the type's properties. + * + * The walk therefore follows what the type actually consists of - its alias/declaration symbols, + * type arguments, union and intersection constituents, properties, signatures, constraints and + * index types - and reports the first constituent declared inside a forbidden package. + * + * Memoized per Program × symbol. Cycle-safe, and bounded so a pathological type cannot stall lint. + */ +function findForbiddenTypeReach( + program: ts.Program, + checker: ts.TypeChecker, + symbol: ts.Symbol, + forbiddenRuntimes: ReadonlySet, +): Hit | null { + const cache = getAnalysisCache(program, forbiddenRuntimes).type; + const cached = cache.get(symbol); + if (cached !== undefined) { + return cached; + } + + const target = symbol.flags & ts.SymbolFlags.Alias ? aliasTargetOf(checker, symbol) : symbol; + const result = target ? walkType(program, checker, declaredTypeOf(checker, target), forbiddenRuntimes) : null; + + cache.set(symbol, result); + return result; +} + +/** + * React prop bags reach thousands of distinct types, so the walk is bounded on both axes. + * Exhausting either limit degrades to "no hit" rather than to a slow lint run; the direct-import + * and runtime checks are unaffected. + */ +const TYPE_WALK_BUDGET = 20_000; +const TYPE_WALK_MAX_DEPTH = 10; + +function walkType( + program: ts.Program, + checker: ts.TypeChecker, + root: ts.Type, + forbiddenRuntimes: ReadonlySet, +): Hit | null { + const seen = new Set(); + let budget = TYPE_WALK_BUDGET; + + function visit(type: ts.Type, depth: number): Hit | null { + if (budget-- <= 0 || depth > TYPE_WALK_MAX_DEPTH || seen.has(type)) { + return null; + } + seen.add(type); + + // `aliasSymbol` catches `type X = ...` written in a forbidden package even when the resolved + // shape is a plain object literal; `symbol` catches interfaces and classes declared there. + for (const owner of [type.aliasSymbol, type.symbol]) { + const hit = owner && owningForbiddenPackage(owner, forbiddenRuntimes); + if (hit) { + return hit; + } + } + + if (type.isUnionOrIntersection()) { + for (const constituent of type.types) { + const hit = visit(constituent, depth + 1); + if (hit) { + return hit; + } + } + return null; + } + + // Type parameters, indexed accesses and conditionals are `Instantiable`, not `Object`, so the + // primitive bail below would discard them along with `string` and `number` — and with them the + // constraint, which is part of the public API. `(value: T) => void` + // exposes the forbidden package through its signature even though `T` itself has no members. + // Unconstrained parameters answer `undefined` and fall through to the bail unchanged. + if (type.flags & ts.TypeFlags.Instantiable) { + const constraint = checker.getBaseConstraintOfType(type); + const hit = constraint && visit(constraint, depth + 1); + if (hit) { + return hit; + } + } + + // Primitives and literals answer `getPropertiesOfType` with their apparent members from the + // standard library (`String`, `Number`, ...). Following those explodes the walk into the whole + // lib without ever crossing into first-party code, so stop here. + if (!(type.flags & ts.TypeFlags.Object)) { + return null; + } + + // Deliberately *not* following `aliasTypeArguments`: the erasure this analysis exists to + // respect lives exactly there. `Omit` keeps + // `ProgressBarProps` as an alias argument, so following it would resurrect the member the + // resolved type no longer has. Instantiated references such as `Array` are still followed. + for (const argument of checker.getTypeArguments(type as ts.TypeReference)) { + const hit = visit(argument, depth + 1); + if (hit) { + return hit; + } + } + + // A DOM prop bag pulls in the whole of `@types/react` and the standard library. Those members + // cannot lead back to a first-party runtime, and expanding them exhausts the budget before the + // interesting members are reached — which previously hid real coupling. The filter is applied + // per property rather than per containing type on purpose: a base type is usually an + // `Omit<...>`, whose alias symbol belongs to `lib.es5.d.ts` even though every member of the + // resolved shape is first-party. + for (const property of checker.getPropertiesOfType(type)) { + const declaration = property.valueDeclaration ?? property.declarations?.[0]; + if (!declaration) { + continue; + } + // A property *declared* in a forbidden package couples the API even when its own type is a + // primitive, so check ownership before deciding whether to descend. + const owned = owningForbiddenPackage(property, forbiddenRuntimes); + if (owned) { + return owned; + } + if (isLibraryFile(program, declaration.getSourceFile())) { + continue; + } + const hit = visit(checker.getTypeOfSymbolAtLocation(property, declaration), depth + 1); + if (hit) { + return hit; + } + } + + for (const signature of [...type.getCallSignatures(), ...type.getConstructSignatures()]) { + for (const parameter of signature.getParameters()) { + const declaration = parameter.valueDeclaration ?? parameter.declarations?.[0]; + const hit = declaration && visit(checker.getTypeOfSymbolAtLocation(parameter, declaration), depth + 1); + if (hit) { + return hit; + } + } + const hit = visit(signature.getReturnType(), depth + 1); + if (hit) { + return hit; + } + } + + for (const indexInfo of checker.getIndexInfosOfType(type)) { + const hit = visit(indexInfo.type, depth + 1); + if (hit) { + return hit; + } + } + + return null; + } + + return visit(root, 0); +} + +/** + * The type a symbol stands for: the declared type for aliases, interfaces and classes, falling + * back to the type at the declaration site for everything else. + */ +function declaredTypeOf(checker: ts.TypeChecker, symbol: ts.Symbol): ts.Type { + const declared = checker.getDeclaredTypeOfSymbol(symbol); + if (!(declared.flags & ts.TypeFlags.Any) || !symbol.declarations?.length) { + return declared; + } + return checker.getTypeOfSymbolAtLocation(symbol, symbol.declarations[0]); +} + +function aliasTargetOf(checker: ts.TypeChecker, symbol: ts.Symbol): ts.Symbol | undefined { + try { + return checker.getAliasedSymbol(symbol); + } catch { + return undefined; + } +} + +/** + * `true` for the TypeScript standard library and ambient `@types` packages — the two sources whose + * members are large, generic and incapable of referencing a first-party runtime. + */ +function isLibraryFile(program: ts.Program, sourceFile: ts.SourceFile): boolean { + return program.isSourceFileDefaultLibrary(sourceFile) || toPosixPath(sourceFile.fileName).includes('/@types/'); +} + +function getAnalysisCache(program: ts.Program, forbiddenRuntimes: ReadonlySet): AnalysisCache { + let byRuntimeSet = programCache.get(program); + if (!byRuntimeSet) { + byRuntimeSet = new Map(); + programCache.set(program, byRuntimeSet); + } + const key = cacheKeyOf(forbiddenRuntimes); + let cache = byRuntimeSet.get(key); if (!cache) { - cache = { value: new Map(), all: new Map() }; - programCache.set(program, cache); + cache = { value: new Map(), type: new Map() }; + byRuntimeSet.set(key, cache); } return cache; } +/** + * Memo of forbidden-runtime set → its cache key. The set is built once per rule instance, so this + * keeps the sort out of the per-symbol path while still letting two instances configured + * identically resolve to the same key and therefore share a cache bucket. + */ +const cacheKeyBySet = new WeakMap, string>(); + +function cacheKeyOf(forbiddenRuntimes: ReadonlySet): string { + let key = cacheKeyBySet.get(forbiddenRuntimes); + if (key === undefined) { + key = [...forbiddenRuntimes].sort().join('\u0000'); + cacheKeyBySet.set(forbiddenRuntimes, key); + } + return key; +} + /** * `true` when the leaf declaration of `symbol` lives inside one of the forbidden packages, which * covers re-export chains that `getAliasedSymbol` collapses in a single hop. @@ -607,7 +828,7 @@ function getAnalysisCache(program: ts.Program): AnalysisCache { function owningForbiddenPackage(symbol: ts.Symbol, forbiddenRuntimes: ReadonlySet): Hit | null { for (const declaration of symbol.declarations ?? []) { const fileName = declaration.getSourceFile().fileName; - const owner = packageFromNodeModulesPath(fileName); + const owner = packageOf(fileName); if (owner !== undefined && forbiddenRuntimes.has(owner)) { return { runtime: owner, via: shortenPath(fileName) }; } @@ -616,20 +837,58 @@ function owningForbiddenPackage(symbol: ts.Symbol, forbiddenRuntimes: ReadonlySe } /** - * The npm package a file belongs to, when the file sits under a `node_modules` directory. + * Memo of directory → owning package name, so the upward `package.json` walk runs once per + * directory instead of once per declaration. Package identity of a directory is stable for the + * lifetime of a lint run. */ -function packageFromNodeModulesPath(fileName: string): string | undefined { - const segments = toPosixPath(fileName).split('/'); - const index = segments.lastIndexOf('node_modules'); - if (index === -1) { - return undefined; +const packageNameByDirectory = new Map(); + +/** + * The package a file belongs to, resolved by walking up to the nearest named `package.json`. + * + * Deriving the name from a `node_modules` path segment is not enough: workspace packages are + * resolved by TypeScript through `paths` mappings (or symlink real paths) straight to their + * source, so their file names contain no `node_modules` segment to read a name from. Walking up + * to the manifest handles both — and is also more accurate for dependencies whose directory name + * differs from their declared name. + */ +function packageOf(fileName: string): string | undefined { + let dir = path.dirname(path.resolve(fileName)); + const visited: string[] = []; + + const memoize = (owner: string | undefined) => { + visited.forEach(seen => packageNameByDirectory.set(seen, owner)); + return owner; + }; + + while (dir !== path.dirname(dir)) { + if (packageNameByDirectory.has(dir)) { + return memoize(packageNameByDirectory.get(dir)); + } + visited.push(dir); + + const owner = readPackageName(path.join(dir, 'package.json')); + if (owner !== undefined) { + return memoize(owner); + } + + dir = path.dirname(dir); } - const first = segments[index + 1]; - if (first === undefined) { + + return memoize(undefined); +} + +/** + * The `name` of a manifest, or `undefined` when it is absent, unreadable, or nameless — nested + * manifests such as `{ "type": "module" }` markers are not package roots, so the walk continues. + */ +function readPackageName(manifestPath: string): string | undefined { + try { + const { name } = JSON.parse(fs.readFileSync(manifestPath, 'utf-8')); + return typeof name === 'string' && name.length > 0 ? name : undefined; + } catch { return undefined; } - const second = segments[index + 2]; - return first.startsWith('@') && second !== undefined ? `${first}/${second}` : first; } interface ModuleEdge {