chore: forbid react-icons, griffel and react-motion in v9 base hooks

[Martin Hochel (Hotell)]

Previous Behavior

The base-hook-no-forbidden-runtime lint rule only guarded tabster, while the headless verify-bundle-isolation config already asserted tabster, @griffel/* and @fluentui/react-icons. The two boundaries had drifted, so a v9 base hook could reach @fluentui/react-icons, the motion packages or Griffel without lint noticing.

Two defects in the rule also surfaced while enabling the wider ban:

1. Workspace packages were invisible to the transitive check. Ownership of a leaf declaration was derived from a node_modules path segment, but TypeScript resolves workspace packages through tsconfig paths straight to source. A forbidden runtime reached through a collapsed re-export chain went unreported.
2. Type coupling was decided from declaration syntax. The walk followed ProgressBarBaseProps -> ProgressBarProps -> ProgressBarSlots -> MotionSlotProps and reported coupling the resolved type does not have — the base types subtract exactly that member (Omit<ProgressBarProps, 'indeterminateMotion'>), and syntax cannot see a subtraction.

New Behavior

Ban list now 1:1 with bundle verification. @fluentui/react-icons, @fluentui/react-motion, @fluentui/react-motion-components-preview, @griffel/react and @griffel/core join tabster, configured explicitly in packages/eslint-plugin/src/internal.js rather than relying on the rule default, so the lint-time boundary sits next to the build-time forbiddenPackages it mirrors. The rule matches names exactly, so satellites and the @griffel/* glob have to be spelled out.

Workspace-aware ownership. Package identity is resolved by walking up to the nearest named package.json, mirroring what verify-bundle-isolation already did. This also handles dependencies whose directory name differs from their declared name.

Structural type analysis. Type references are now answered by the type checker — walking alias/declaration symbols, type arguments, union and intersection constituents, properties, signatures and index types — so an erased member is simply absent. Two subtleties are encoded in the implementation:

• aliasTypeArguments are deliberately not followed: that is exactly where Omit<T, K> retains the pre-subtraction type, so following it resurrects the erased member.
• Library pruning is applied per property, not per containing type. Pruning by containing type hid real coupling, because 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.

Value references keep the existing syntactic analysis, which is the right model for runtime dependencies.

On the motion ban alone this turns 20 reports in 12 packages into 2 in one — Accordion, Drawer, Menu, Nav, Progress and TeachingPopover were all false positives.

Coupling found and recorded as debt

All of it pre-existing. Note that the two checks share a package list but cover different surfaces: the lint rule inspects use*Base_unstable hooks wherever they are declared, while bundle verification measures what survives into the headless entry points. Only MessageBar is common to both — the headless package consumes useMessageBarBase_unstable directly, which is exactly why @fluentui/react-motion shows up in the bundle report. The Griffel rows below are source-level only today: the headless package reimplements useMenu/useCombobox and borrows just MENU_ENTER_EVENT and useInputTriggerSlot, so those chains do not currently reach a bundle. (Griffel used to leak into the bundle through @fluentui/react-portal's usePortalMountNode; #36553 has since closed that on master, so after rebasing, @fluentui/react-motion via MessageBar is the only bundle-level debt left — and #36549 removes that one too.)

Runtime	Where	Kind
@fluentui/react-motion	useMessageBarBase_unstable calls useMotionForwardedRef — the only case that actually reaches the bundle	value
@fluentui/react-motion	AvatarGroupPopover base types omit only size, keeping the popover root slot and with it surfaceMotion	type
@griffel/react	useComboboxBase_unstable / useDropdownBase_unstable compose the styled Listbox slot	value
@griffel/react	useMenuBase_unstable calls useSafeZoneArea, which renders <SafeZoneArea>; that component calls mergeClasses and its styles module calls makeStyles at module scope	value

@fluentui/react-icons and @fluentui/react-motion-components-preview came back completely clean — nothing to record.

Validation

• 109 eslint-rules tests pass, including new fixtures for the workspace-package shape, an Omit-erased / not-erased pair, and a coupled / clean type-parameter-constraint pair.
• Swept all 2714 source files across every package that declares a base hook: 0 rule violations and 0 unused suppressions — every disable is load-bearing.
• Tests and type-check pass on all touched packages.
• verify-bundle-isolation: PASS WITH DEBT — 3 fixtures, 0 regressions, 1 allowed violation, kept out: tabster, @griffel/*, @fluentui/react-icons, @fluentui/react-motion-components-preview. The single allowlist entry is @fluentui/react-motion via useMessageBarBase_unstable; TagPicker and TeachingPopover are CLEAN.
• beachball check passes; change files are type: none since every change to a published package is comments or lint config only.

Review follow-ups

Two further rule defects were raised in review and are fixed here as separate commits:

• Analysis cache was keyed by Program × symbol, but every answer depends on the configured forbiddenRuntimes. ESLint shares one TypeScript Program across configurations pointed at the same tsconfig, so a result computed for one ban list could be served to another that bans different packages. Reproduced (a config banning only workspace-runtime was told about heavy-runtime), then fixed by partitioning the memo by a normalized key of the set. No current result changes — the repo configures the rule at one site — so this is insurance against a second configuration.
• Type-parameter constraints were silently dropped. Instantiable types are not Object, so they were discarded by the bail that keeps primitives from expanding into their standard-library members — taking their constraints with them. An imported <T extends MotionSlotProps>(value: T) => void was reported clean despite exposing the forbidden package in its signature. Now resolved via getBaseConstraintOfType before the bail. This makes the rule stricter, so the whole-repo sweep was re-run: still 0 violations, 0 unused suppressions.

A third comment — that the type walk resolves the imported symbol rather than the type at the reference site, so an inline props: Omit<StyledProps, 'motion'> still reports the erased member — is accurate and deliberately not fixed here. The erasure is handled when written as a declaration (the ProgressBar case above) but not inline. I implemented the fix, got it green, and dropped it: it was ~83% of the change's complexity and 7 new functions, surfaced five separate precision defects (four only after an earlier version passed the full suite), and addresses a pattern that occurs zero times across the 39 packages declaring a base hook. Details in the review thread; it can land as its own reviewable change if the pattern appears.

Notes for reviewers

• The suppressions are per-line rather than block-level on purpose, so each hook body keeps being checked — a block disable would mask a future runtime leak, which is the case that actually costs bundle size.
• The structural walk has a depth cap (10) and a traversal budget (20k) as hang protection; exhausting either degrades to "no hit". Real types currently use 20–200 visits, so there is ample headroom, but it is a silent-failure mode worth knowing about.
• Every recorded violation is pre-existing coupling this PR makes visible, not new debt. Removing an entry is the goal; adding one is a regression.

Related Issue(s)

[github-actions]

📊 Bundle size report

✅ No changes found

[github-actions]

Pull request demo site: URL

[Copilot]

Pull request overview

Aligns v9 base-hook linting with bundle-isolation restrictions and records existing dependency debt.

Changes:

• Adds workspace-aware package ownership and structural type analysis.
• Expands forbidden runtimes and bundle checks.
• Adds fixtures, tests, suppressions, and change records.

Reviewed changes

Copilot reviewed 22 out of 22 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
tools/eslint-rules/rules/base-hook-no-forbidden-runtime.ts	Implements structural type and workspace ownership analysis.
tools/eslint-rules/rules/base-hook-no-forbidden-runtime.spec.ts	Tests workspace and omitted-type behavior.
tools/eslint-rules/rules/__fixtures__/base-hook-no-forbidden-runtime/tsconfig.json	Maps new fixture packages.
tools/eslint-rules/rules/__fixtures__/base-hook-no-forbidden-runtime/stubs/workspace-runtime/package.json	Defines the workspace runtime fixture.
tools/eslint-rules/rules/__fixtures__/base-hook-no-forbidden-runtime/stubs/workspace-runtime/index.ts	Provides fixture runtime exports.
tools/eslint-rules/rules/__fixtures__/base-hook-no-forbidden-runtime/stubs/workspace-relay-pkg/index.ts	Re-exports workspace runtime symbols.
tools/eslint-rules/rules/__fixtures__/base-hook-no-forbidden-runtime/stubs/watched-pkg/index.ts	Exports structural type fixtures.
tools/eslint-rules/rules/__fixtures__/base-hook-no-forbidden-runtime/stubs/watched-pkg/heavy.ts	Adds styled and omitted props fixtures.
tools/eslint-rules/rules/__fixtures__/base-hook-no-forbidden-runtime/stubs/heavy-runtime/package.json	Adds fixture package identity.
tools/eslint-rules/rules/__fixtures__/base-hook-no-forbidden-runtime/stubs/cyclic-heavy-pkg/package.json	Adds cyclic fixture package identity.
packages/react-components/react-message-bar/library/src/components/MessageBar/useMessageBar.ts	Records existing motion coupling.
packages/react-components/react-menu/library/src/components/Menu/useMenu.tsx	Records existing Griffel coupling.
packages/react-components/react-headless-components-preview/library/bundle-isolation.config.json	Extends bundle restrictions and debt allowance.
packages/react-components/react-combobox/library/src/components/Dropdown/useDropdown.tsx	Suppresses recorded Dropdown coupling.
packages/react-components/react-combobox/library/src/components/Combobox/useCombobox.tsx	Suppresses recorded Combobox coupling.
packages/react-components/react-avatar/library/src/components/AvatarGroupPopover/useAvatarGroupPopover.tsx	Records type-level motion coupling.
packages/eslint-plugin/src/internal.js	Configures the expanded runtime ban list.
change/@fluentui-react-message-bar-9726a17c-0433-4787-b72b-90ba58ebfe61.json	Records the MessageBar change.
change/@fluentui-react-menu-639031a5-ca32-46af-9da2-9f57d8fc50d6.json	Records the Menu change.
change/@fluentui-react-headless-components-preview-9d9458fe-d4dc-4b37-90a0-e5e2c5fe7807.json	Records the headless package change.
change/@fluentui-react-combobox-55618c76-2dae-4993-a52c-d0269f1f96de.json	Records the Combobox change.
change/@fluentui-react-avatar-6892f968-8be8-4e9c-9fc6-b747d59db983.json	Records the Avatar change.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Martin Hochel (Hotell)]

Correct diagnosis, but not fixing it in this PR — deliberately. Recording the reasoning since the limitation is real.

I confirmed the defect first: props: Omit<StyledProps, 'motion'> written inline does report the erased member, exactly as you describe. So the PR description overstates its own claim — the structural analysis handles erasure written as a declaration (type Base = Omit<Props, 'motion'>, the ProgressBar case), not at the reference site.

I then implemented the fix you suggest — thread the reference into the type path, resolve the enclosing type expression, re-key the cache by the resolved type — and got it fully green. I dropped it anyway. What it cost:

• ~83% of the change's complexity: ~219 lines and 7 new functions, against 13 lines for the constraint fix and 33 for the cache fix.
• Five separate precision defects, four of which surfaced only after an earlier version passed the whole suite:
  1. Resolving the enclosing expression let a clean import inherit a sibling's coupling (Styled | Clean blamed Clean).
  2. Gating on "enclosing has any hit" reintroduced the original false positive whenever a second member was coupled (Omit<Styled,'motion'> & { w: Heavy } blamed Styled).
  3. Walk-budget exhaustion became proof of cleanliness: a clean enclosing result now vetoes an established hit, so hitting the depth cap silently deleted real coupling at nesting depth ≥10. This inverted the safety direction of the existing limits.
  4. Narrowing the climb to fix (2) broke erasure applied around a combining node — Omit<A & B, 'k'>, idiomatic here, e.g. Omit<TreeState & SubtreeContextValue, 'treeType'> in react-tree.
  5. ParameterDeclaration is not a TypeNode, so Omit<{ onChange: (e, d: Styled) => void }, 'onChange'> — a props bag dropping a handler whose payload carries the runtime — still misfired.

Each was individually plausible; the point is that the erasure-scope question has no primitive in the TypeScript API, so it becomes hand-rolled AST classification of which node kinds erase, which merely combine, and which are non-TypeNode nodes that nonetheless live inside a type. That is a lot of surface for a lint rule to own.

Against that: the inline pattern occurs zero times across all 39 packages that declare a base hook. The fix prevents a future false positive; it removes no current one.

So this PR ships the two fixes that are small, local and verifiable, and leaves this one open rather than smuggling the risky part in behind them. If the inline form does appear, the design is worked out and can land as its own reviewable change — which is the right size for it.

[Martin Hochel (Hotell)]

Confirmed and fixed in ce0f7890a8.

Reproduced before fixing: analysing useHeavy under forbiddenRuntimes: ['heavy-runtime'] and then under ['workspace-runtime'] over the same fixture Program made the second configuration report heavy-runtime — a package it does not ban. The rule's own spec was already running two different option sets against one tsconfig, so the condition was live.

The memo is now partitioned by a normalized key of the set, and configurations with identical ban lists still share a bucket. Both directions are regression-tested in ordered RuleTester runs: a hit cached for one set must not be served to a set that allows it, and the clean result that produces must not then suppress the real one.

This changes no current result — the repo configures the rule at a single site — so it is insurance against a second configuration rather than a fix for observed noise.

[Martin Hochel (Hotell)]

Confirmed and fixed in 39d5c30302.

Reproduced before fixing with a fixture matching your example: type HeavyConstrainedCallback = <T extends HeavyOptions>(value: T) => void was reported clean, because Instantiable types are not Object and so were discarded by the primitive bail along with string and number.

walkType now resolves checker.getBaseConstraintOfType before that bail. Unconstrained parameters answer undefined and fall through unchanged, and the existing seen set and depth cap bound the extra edge. Both the coupled and the clean-constraint case are covered by fixtures.

Worth noting since this one makes the rule stricter: I re-swept every package that declares a base hook (39 packages) with --report-unused-disable-directives. Zero new violations and zero unused suppressions, so it closes the blind spot without adding debt.

[Dmytro Kirpa (dmytrokirpa)]

fixed in #36549

[Copilot]

Pull request overview

Copilot reviewed 22 out of 22 changed files in this pull request and generated no new comments.

Suppressed comments (4)

tools/eslint-rules/rules/base-hook-no-forbidden-runtime.ts:660

• Using a single seen set with a depth limit can produce false negatives in shared type graphs. If a type is first reached at depth 10, its children are cut off at 11, but a later shallower path to that same type is skipped as already seen; a forbidden child that was within range of the shallower path is therefore never checked. Record the shallowest visited depth and revisit a type when the new path has more remaining depth.

    if (budget-- <= 0 || depth > TYPE_WALK_MAX_DEPTH || seen.has(type)) {
      return null;
    }
    seen.add(type);

packages/eslint-plugin/src/internal.js:32

• This list is not yet 1:1 with the bundle boundary's @griffel/* ban: @griffel/shadow-dom is an installed runtime package (yarn.lock:6766) but is absent here. A base hook can therefore reach it without a lint error even though bundle isolation rejects the same dependency. Add the known runtime satellite (or support the same package pattern semantics in the rule).

  '@griffel/react',
  '@griffel/core',

tools/eslint-rules/rules/base-hook-no-forbidden-runtime.ts:750

• The signature walk omits API-bearing signature elements. The constraint test passes only because T is reused as a value parameter, but <T extends HeavyOptions>() => void has no edge to T; similarly, (this: HeavyOptions) => void and (value: unknown) => value is HeavyOptions are missed because explicit this parameters and predicate target types are not included in getParameters()/getReturnType(). Traverse these signature types explicitly so forbidden coupling cannot hide in a valid public callback signature.

    for (const signature of [...type.getCallSignatures(), ...type.getConstructSignatures()]) {

tools/eslint-rules/rules/base-hook-no-forbidden-runtime.ts:757

• Only the index value type is traversed, so coupling in the index key type is missed. For example, a public { [key: HeavyKey]: string } shape is treated as clean when HeavyKey comes from a forbidden package. IndexInfo exposes both keyType and type; visit both.

    for (const indexInfo of checker.getIndexInfosOfType(type)) {
      const hit = visit(indexInfo.type, depth + 1);