feat(eslint-rules): add base-hook-no-forbidden-runtime rule#36253
feat(eslint-rules): add base-hook-no-forbidden-runtime rule#36253Hotell wants to merge 3 commits into
Conversation
📊 Bundle size report✅ No changes found |
|
Pull request demo site: URL |
There was a problem hiding this comment.
Pull request overview
Adds a new workspace ESLint rule (@nx/workspace-base-hook-no-forbidden-runtime) intended to prevent v9 use*Base_unstable hooks from (directly or transitively) depending on forbidden runtime packages, with a dedicated typed + untyped test/fixture setup.
Changes:
- Introduces the
base-hook-no-forbidden-runtimerule with transitive import-graph reach analysis (value vs type reach) and a one-shot warning when type info is missing. - Adds RuleTester coverage plus a self-contained fixture workspace (stub packages + tsconfig) to validate transitive reach and cycle handling.
- Updates eslint-rules workspace config to exclude fixture files from lint compilation, and registers the rule in the workspace rules index.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/eslint-rules/tsconfig.lint.json | Excludes __fixtures__ from lint tsconfig to avoid fixture compilation/lint noise. |
| tools/eslint-rules/rules/base-hook-no-forbidden-runtime.ts | Implements the new rule, including import tracking, scope analysis, and transitive reach computation. |
| tools/eslint-rules/rules/base-hook-no-forbidden-runtime.spec.ts | Adds untyped + typed RuleTester suites covering direct forbidden imports, transitive reach, and cycles. |
| tools/eslint-rules/rules/fixtures/base-hook-no-forbidden-runtime/tsconfig.json | Fixture TS project config with path mappings to stub packages for typed tests. |
| tools/eslint-rules/rules/fixtures/base-hook-no-forbidden-runtime/stubs/watched-pkg/light.ts | Stub “light” module that should not reach the forbidden runtime. |
| tools/eslint-rules/rules/fixtures/base-hook-no-forbidden-runtime/stubs/watched-pkg/index.ts | Stub package entrypoint with mixed value/type re-exports to exercise reach semantics. |
| tools/eslint-rules/rules/fixtures/base-hook-no-forbidden-runtime/stubs/watched-pkg/heavy.ts | Stub “heavy” module that imports the forbidden runtime. |
| tools/eslint-rules/rules/fixtures/base-hook-no-forbidden-runtime/stubs/light-helper/index.ts | Stub helper package used by the “light” module. |
| tools/eslint-rules/rules/fixtures/base-hook-no-forbidden-runtime/stubs/heavy-runtime/index.ts | Stub forbidden runtime package used to validate direct + transitive blocking. |
| tools/eslint-rules/rules/fixtures/base-hook-no-forbidden-runtime/stubs/cyclic-pkg/index.ts | Stub cyclic package entrypoint to exercise cycle safety. |
| tools/eslint-rules/rules/fixtures/base-hook-no-forbidden-runtime/stubs/cyclic-pkg/b.ts | Part of a cyclic import graph in the fixture. |
| tools/eslint-rules/rules/fixtures/base-hook-no-forbidden-runtime/stubs/cyclic-pkg/a.ts | Part of a cyclic import graph in the fixture. |
| tools/eslint-rules/rules/fixtures/base-hook-no-forbidden-runtime/src/test.ts | Placeholder file so the typed Program contains the filename used in RuleTester cases. |
| tools/eslint-rules/rules/fixtures/base-hook-no-forbidden-runtime/src/dummy.ts | Additional placeholder/anchor file in the fixture project. |
| tools/eslint-rules/index.ts | Registers the new rule in the exported workspace rules map. |
47ccc8e to
18d8549
Compare
| type BaseHookFunction = TSESTree.FunctionDeclaration | TSESTree.FunctionExpression | TSESTree.ArrowFunctionExpression; | ||
|
|
||
| const DEFAULT_WATCHED_PACKAGES: ReadonlyArray<string> = ['@fluentui/react-tabster']; | ||
| const DEFAULT_FORBIDDEN_RUNTIMES: ReadonlyArray<string> = ['tabster']; |
There was a problem hiding this comment.
should we add @floating-ui/dom, @floating-ui/react to DEFAULT_FORBIDDEN_RUNTIMES and @fluentui/react-positioning to DEFAULT_WATCHED_PACKAGES? Or you plan to add on config level via options? (two base hooks still have react-positioning: useTooltipBase_unstable & useMenuBase_unstable, but they are not used in headless, so we should revert it)
There was a problem hiding this comment.
- floating - not atm, we might need it as progressive enhancement pattern for headless.
so we should revert it
what should we revert ?
There was a problem hiding this comment.
the base hooks that are not used in headless (with react-positioning, in Menu and Tooltip)
There was a problem hiding this comment.
IMO we can keep them to follow our "base hook" established pattern, the thing that they use internally fui/r-positioning/floating ui is ok for now. if we will come to the point that we wanna use them in headless we will refactor. wdyt ?
Registers @nx/workspace-base-hook-no-forbidden-runtime. Rule is not yet enabled in any project config and is fully inert until wired up in a follow-up PR.
Cache in-progress reach results to preserve transitive closure across import cycles, normalize diagnostic paths cross-platform, and add cyclic regression fixtures/tests.
…er review feedback Address reviewer feedback from PR microsoft#36253: - Add SymbolReach type alias for return value clarity (mainframev suggestion) - Extract getImportSymbolNameNode helper to reduce nested import logic (mainframev suggestion) - Remove redundant casts to ImportSpecifierNode in tracked imports (mainframev suggestion) - Remove legacy TypeScript fallback in resolveModule; require TS >=5.3 behavior only (mainframev suggestion) All changes maintain existing behavior and pass the full eslint-rules test suite (85 tests). getFunctionInit remains duplicated to avoid creating a new shared module per user preference.
18d8549 to
a1c78dd
Compare
Summary
Introduces a new workspace ESLint rule
@nx/workspace-base-hook-no-forbidden-runtimethat preventsuseFooBase/useFooBase_unstablehooks from pulling in heavy runtime dependencies. It walks the imports of the base-hook module and flags transitive dependencies on packages outside an allowlist.The rule is fully inert in this PR — it is registered with
@nx/workspacebut not enabled in any project config. Wiring happens in a follow-up PR.Plan reference
Part of the split of #36251
Fixtures
Self-contained fixture workspace under
tools/eslint-rules/rules/__fixtures__/base-hook-no-forbidden-runtime/includes a localtsconfig.jsonplus stub packages:light-helper,heavy-runtime,watched-pkg(withlight.ts/heavy.tspaths), andcyclic-pkg(a → b → index → a cycle) to verify cycle handling.Verification
→ 4 suites / 56 tests pass.
ESLint CLI perf (TIMING=200)
Measured on a combined branch (PR1 + PR2 + PR3 wiring) so the rule actually runs.
@nx/workspace-base-hook-no-forbidden-runtimereact-button(no*Basehook)react-combobox(1*Basehook)react-tags(1*Basehook)Negligible — the rule short-circuits on non-base-hook files and the import graph walk is bounded + cycle-safe.
Out of scope
@fluentui/eslint-plugin(separate PR).