fix: Resolve package-name-shaped tsconfig path aliases#11998
fix: Resolve package-name-shaped tsconfig path aliases#11998anthonyshew merged 8 commits intovercel:mainfrom
Conversation
When a tsconfig `paths` entry maps a bare specifier that looks like a package name (e.g. `features/feature-a` → `./src/features/feature-a`) `turbo boundaries` incorrectly flagged the import as an undeclared dependency. The root cause was an early-return guard in `check_import_as_tsconfig_path_alias` that skipped any import matching `is_potential_package_name`. This prevented the resolver from even attempting to resolve such imports as tsconfig aliases, causing them to fall through to `check_package_import` which then raised a violation. Fix: remove the `is_potential_package_name` guard so that all non-relative imports are tried against the resolver. If the import resolves to a local file the function returns `true` (no further checks); if it cannot be resolved it returns `false` and the call site falls through to `check_package_import` as before. Adds a regression test (`tsconfig_alias_resolves_package_name_shaped_path_alias`) that reproduces the exact setup reported in the issue. Fixes vercel#11906
|
@sleitor is attempting to deploy a commit to the Vercel Team on Vercel. A member of the Team first needs to authorize it. |
When check_import_as_tsconfig_path_alias resolves an import to a path inside node_modules, return false so the caller falls through to check_package_import for proper dependency validation.
|
Fixed in latest commit (83be5c2). The issue was that The fix adds a check after resolution: if the resolved path contains a |
anthonyshew
left a comment
There was a problem hiding this comment.
Hey, thanks for wanting to contribute.
- There's a failing regression test. We, of course, need for it to pass to be able to merge.
- The
node_modulescheck is a bit problematic. Depending on the package manager, the package might be in the root of the repository, behind a symlink, in a cache store somewhere else on disk, etc. Let's switch this to either:check_file_importlike at line 264resolution.package_json()fromunrs_resolver(probably the better choice)
is_potential_package_nameused to be an O(1) string check that skipped most imports. Now its called on every bare import, representing a massive performance regression
Let's get these cleaned up for a first review pass, and then we will see if we can merge. Thanks again!
… guard tsconfig check with is_potential_package_name - Replace node_modules path component check with resolution.package_json().is_some() per anthonyshew's suggestion: if the resolver found a package.json the resolution went through node_modules (a real npm package), not a tsconfig path alias. More robust than string path component matching. - Move check_import_as_tsconfig_path_alias inside the is_potential_package_name guard to restore O(1) filtering for non-package-name-shaped imports (node:fs, bun:test, etc.). Previously the resolver was invoked for every bare import which regressed performance. Now the resolver is only called for package-name-shaped imports.
|
Thanks for the review @anthonyshew! Addressed all three points in the latest commit: 1. Failing regression test — The only failing CI check now is "Validate PR title" (a format lint). All code tests pass locally ( 2. 3. |
…) presence The previous approach used `resolution.package_json().is_some()` to distinguish npm packages from local tsconfig path alias targets. However, oxc_resolver may return a package.json for any file inside a package directory — including files reached via tsconfig `paths` aliases in a package that has its own package.json. Switch to checking whether the resolved path contains a `node_modules` component, which is the canonical way to distinguish npm packages from local source files. Also adds a regression test that verifies alias resolution still works when a package.json is present in the package root.
|
Good catch from VADE — fixed in the latest commit (500e7d9). The The fix switches to checking whether the resolved path contains a |
Structured resolver error handling to distinguish expected failures (NotFound, Builtin) from unexpected ones (I/O, broken tsconfig) with tracing::debug logging for the latter. Added test coverage for the node_modules bail-out path and out-of-package alias boundary violations. Fixed pre-existing macOS test failures caused by /tmp symlink canonicalization mismatch. Added doc comments to key functions.
anthonyshew
left a comment
There was a problem hiding this comment.
Hey, thanks for getting us started. I made a few more changes and now we're good to go. Thanks!
…ig-path-aliases # Conflicts: # crates/turborepo-boundaries/src/imports.rs
Workspace packages symlinked in node_modules resolve to their real path (without node_modules in it), so the node_modules path check alone is insufficient. Added a package.json name check: if the resolution's package.json has a name matching the import's package name, it's an actual workspace/npm package, not a tsconfig alias. Also moved tsconfig alias resolution to cover all non-relative imports (not just package-name-shaped ones) so that aliases like `!` and `@/foo` are correctly resolved. Fixed Windows test failure by using dunce::canonicalize to avoid the \\?\ prefix that breaks path comparison.
## Release v2.8.17-canary.4 Versioned docs: https://v2-8-17-canary-4.turborepo.dev ### Changes - release(turborepo): 2.8.17-canary.3 (#12248) (`b3cfcdb`) - fix: Resolve package-name-shaped tsconfig path aliases (#11998) (`0a3166b`) Co-authored-by: Turbobot <turbobot@vercel.com>
Problem
When a
tsconfig.jsonmaps a bare specifier that looks like a package name to a local source path:{ "compilerOptions": { "paths": { "*": ["./src/*"] } } }an import like:
resolves locally (TypeScript handles it correctly), but
turbo boundariesincorrectly reports:This was introduced in v2.8.7. See #11906 for a reproduction.
Root cause
check_import_as_tsconfig_path_aliashad an early-return guard:This prevented the resolver from attempting to resolve any import whose specifier looks like a package name (e.g.
features/feature-a). Those imports then fell through tocheck_package_import, which flagged them as undeclared dependencies.Fix
Remove the
is_potential_package_nameguard so all non-relative imports are first tried against the tsconfig resolver:trueand the check stops (no false positive).node_modules→ returnsfalse(real npm package, falls through to dependency check).false(import falls through tocheck_package_importas before).Additionally:
Err(_) => Ok(false)with explicit matching on expected errors (NotFound,Builtin, etc.) andtracing::debug!logging for unexpected errors (I/O, broken tsconfig) to aid debugging.check_import,check_file_import,get_package_name, andis_potential_package_name./tmp→/private/tmp).Tests
tsconfig_alias_check_skips_bare_package_names→tsconfig_alias_check_returns_false_for_unresolvable_package_importsto reflect the corrected semantics (the behaviour is unchanged: unresolvable package imports still returnfalse).tsconfig_alias_resolves_package_name_shaped_path_alias: regression test that mirrors the exact scenario inturbo boundariesdoes not understand tsconfig path aliases, incorrectly flags local imports as undeclared dependencies #11906 ("*": ["./src/*"]alias,features/feature-aimport).tsconfig_alias_check_returns_false_for_node_modules_resolution: verifies thenode_modulesbail-out path with a realnode_modules/fixture on disk.tsconfig_alias_flags_boundary_violation_for_out_of_package_resolution: verifies a tsconfig alias pointing outside the package root produces anImportLeavesPackagediagnostic.warnings.is_empty()assertions to regression tests.Fixes #11906