fix(eslint-plugin): use project root as path-normalization anchor in monorepos#365
Merged
Merged
Conversation
…monorepos The `/src/` heuristic in `normalizePath` and `resolveImportPath` collapses `<monorepo>/packages/<x>/src/foo.ts` to `src/foo.ts`, destroying the package prefix and making `no-forbidden-imports` / `no-layer-violation` patterns that reference `packages/<x>/**` unable to match files inside `<package>/src/**`. Add an optional `projectRoot` parameter to both utilities. When provided and the file lives under it, the project-root-relative path is returned; otherwise the existing `/src/` fallback is used. Rules resolve the anchor via a new `getConfigRoot` helper that walks up to `harness.config.json`. Existing tests pass unmodified — they don't thread a `projectRoot` and hit the fallback path.
…tPath fallback Add tests for two branches introduced by the projectRoot widening that weren't exercised by the existing suite: getConfigRoot returning null when no harness.config.json ancestor exists, and resolveImportPath returning the import path unchanged when the resolved location has no /src/ boundary and no projectRoot anchor.
`path.resolve` on Windows prepends a drive letter to Unix-style absolute paths (e.g. '/abs/repo/...' becomes 'D:/abs/repo/...'), which makes the synthetic Unix paths in three monorepo resolveImportPath cases fall through the `startsWith(root + '/')` check. This is a test-fixture limitation, not a production bug — real Windows ESLint usage receives filenames and projectRoot values that share a drive prefix. The integration test (`monorepo-path-anchor.test.ts`) uses real `path.join` to construct fixture paths, so it exercises the actual cross-platform code path on all OSes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
<package>/src/**files normalized tosrc/<...>, destroying the package prefix and making layer-based rules withfrom: "packages/<x>/**"patterns unable to match files inside<package>/src/**.projectRootparameter tonormalizePath/resolveImportPathinpackages/eslint-plugin/src/utils/path-utils.ts. Rules resolve the anchor via a newgetConfigRoot(filePath)helper that walks up toharness.config.json(wraps the existingfindConfigFile).projectRootis provided and the file lives under it, return the project-root-relative path (preserves package identity). Otherwise fall back to the legacy/src/heuristic — byte-identical to current behavior.Backward compatibility
projectRootand hit the legacy fallback path.projectRootis implicit (derived from config file location), not a config field.Repro & consumer impact
Before this fix, in a monorepo:
so a
forbiddenImportsrule withfrom: "packages/types/**"never matched. After this fix, the same call with aprojectRootof/abs/reporeturns'packages/types/src/foo.ts', and layer-internal rules forpackages/<x>/src/**start firing as intended.Out of scope
importMatchesDisallowedmatcher is unchanged — workspace-package specifier matching is a separate config-side concern.Test plan
tests/utils/path-utils.test.tsforwith projectRoot (monorepo)covering: anchoring, files without/src/, outside-root fallback, trailing-slash equivalence, legacy preservation.tests/utils/config-loader.test.tsforgetConfigRoot(resolved directory + null branch).tests/integration/monorepo-path-anchor.test.tswith atests/fixtures/monorepo/harness.config.jsonfixture:forbiddenImportsrule withfrom: "packages/a/**"fires on a file atpackages/a/src/index.ts(it does not onmain).pnpm test,pnpm typecheck,pnpm lint,pnpm buildall clean inpackages/eslint-plugin.pnpm -w lintandpnpm test:ciclean.