Repository navigation
Conversation
…declares them TsconfigResolver merged baseUrl and paths from an extended config but resolved them against the directory of the nearest config. tsc resolves both relative to the file that declares them, so in an Nx/Turborepo layout (root tsconfig.base.json, apps/<app>/tsconfig.json extending it) every alias was probed under apps/<app>/ and came back None. The IMPORTS_FROM edge kept the raw alias string, and importers_of / impact radius on a shared library missed every app that inherits its paths. _resolve_extends() now records the absolute anchor where baseUrl (and paths, for configs without baseUrl) is declared and passes it down the chain unchanged until a config redeclares it. resolve_alias() uses baseUrl's anchor first, then the paths anchor, then the nearest config's directory as before. The existing extends test kept base and child in one directory, where both anchors coincide. Four new tests cover a base two levels up, paths without baseUrl, a child that redeclares paths, and a child baseUrl that rebases inherited paths. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Inherited baseUrl/paths anchoring through tsconfig extends chains: - middle config declaring neither option, suffix-less extends - paths declared by a middle config without baseUrl - baseUrl pointing out of a sibling config directory - inherited baseUrl anchoring paths the child declares - child paths without any baseUrl, child empty paths - inherited wildcards, extension and directory-index probing - unicode and spaces in directories, jsconfig children - several apps sharing one base, repeated lookups (cache) - missing, array-form and package extends keep the child anchor - extends cycles, invalid base JSON, malformed child options - internal anchor key names in user configs are ignored - 40-level extends chain - full build, incremental update and rebuild store resolved targets Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Thank you for this PR.
The failing tests are on the branch After you push the fix, I will test it again. |
# Conflicts: # CHANGELOG.md Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The review on tirth8205#1111 found two extends shapes where the anchors disagreed with tsc: - A base config reached through a symlink was anchored at the directory of the file the link points to, because the extends path was resolved before its directory was taken. tsc locates a config by the path it opened it through, normalized without following symlinks. The extends path, the config's directory and baseUrl are now normalized lexically, and an alias target is normalized before it is resolved, so a `..` in baseUrl or paths climbs from the link, not from its target. - "baseUrl": null in an extending config kept the inherited baseUrl anchor. tsc treats null as unset, so paths fall back to the config that declares them. Drops the xfail marks from the two tests on review/pr-1111 that pin these, and adds one for a base config inside a symlinked directory, with "baseUrl": ".." and with a paths-only "../src/*", both checked against tsc 5.9.3 and 6.0.3. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Thanks for the review and for the tests. Both points are fixed in f638eb6, and the branch is up to date with
Each shape checked with
|
Pull Request
Linked issue
Closes #1110
What & why
TsconfigResolvermergedbaseUrlandpathsfrom an extended config but resolved them against the directory of the nearest config (_tsconfig_dir). tsc resolves both relative to the config file that declares them. In an Nx or Turborepo layout (roottsconfig.base.json,apps/<app>/tsconfig.jsonextending it) every alias was probed underapps/<app>/,resolve_alias()returnedNone, and theIMPORTS_FROMedge kept the raw alias string.importers_ofand the impact radius of a shared library then missed every app that inherits itspaths.All changes are in
code_review_graph/tsconfig_resolver.py:_resolve_extends()records the absolute anchor wherebaseUrlis declared (_base_url_dir) and wherepathsis declared (_paths_dir), and passes both down the chain unchanged until a config redeclares them."baseUrl": nullin an extending config drops the inheritedbaseUrlanchor, since tsc treatsnullas unset.extendspath, the config's directory andbaseUrlare normalized withos.path.abspath/os.path.normpath, which do not follow symlinks, so a base config reached through a symlink, to the file or to a directory above it, anchors where the link sits.resolve_alias()resolvespathsagainst_base_url_dir, then_paths_dir, then the nearest config's directory as before._match_and_probe()normalizes the joined path before resolving it, so a..inpathsclimbs from the anchor as written. The joined path is still resolved before probing, so a file it finds is returned in the same form as before.A config that declares its own
baseUrlandpaths, the only shape that worked before, resolves as it did. The one exception is a..that climbs out of a symlinked directory, which now climbs from the link, as in tsc. The scope is unchanged: only a relative stringextendsis followed; array-form and packageextendsare still not.On the monorepo where this showed up (5.3k files), 0 of 430 imports of the shared types library were linked from the apps and libraries that inherit
paths. With this change the resolver maps the aliases from each of those apps and libraries (checked on eight representative imports, one per app or library).Review follow-up
The review found two shapes where the first version disagreed with tsc: a symlinked base config was anchored at the link's target, and a child
"baseUrl": nullkept the inherited anchor. Both are fixed in f638eb6. The tests fromreview/pr-1111are on this branch with their original commits (1893de6, eeca314); f638eb6 removes their twoxfailmarks. Checking the symlink fix against tsc found the same problem for a base config inside a symlinked directory, now covered bytest_base_config_in_a_symlinked_directory_climbs_from_the_link.Each shape, checked with
ts.resolveModuleNamefrom TypeScript 5.9.3 and 6.0.3 on Linux (both versions agree):staging"baseUrl": "."apps/api/src/util.tsapps/api/src/util.tstools/src/util.tsapps/api/src/util.tsapps/api/src/util.tsapps/api/src/util.tstools/src/util.tsapps/api/src/util.ts"baseUrl": nullfoo.tsNonebase/foo.tsfoo.ts"baseUrl": ".."apps/api/src/util.tsNonetools/src/util.tsapps/api/src/util.ts"../src/*"apps/api/src/util.tsNonetools/src/util.tsapps/api/src/util.tsHow it was tested
Branch merged with
stagingd72e8fb. Two environments:uv sync --extra dev.uv.lockinstalled for cp310 and the tests run withpython3 -m pytest. The symlink tests need it: on Windows they skip, because creating a symlink needs a privilege.tests/test_tsconfig_resolver.pyrun against the resolver from 6d148c5 (the merge, before f638eb6) has 5 failures on Linux: both cases of the symlinked base config test, both cases of the symlinked directory test and thenullcase. With only the_resolve_extends()half of the fix, the paths-only"../src/*"case still fails, which is why_match_and_probe()changed.stagingd72e8fb against this branch:staging5005 passed and 0 failed; this branch 5038 passed and 0 failed. Both have 920 skipped, 3 xfailed and 2 xpassed. The 33 extra passes are the new tests intests/test_tsconfig_resolver.py.test_tsconfig_resolver.py). Passed goes from 4923 to 4952 and skipped from 899 to 903, the 4 extra skips being the symlink cases.staging, all indaemon.py(fcntl.flock,LOCK_EXandLOCK_NBare POSIX-only).Checklist
uv run pytest tests/ --tb=short -q(Linux, run withpython3 -m pytestas described above; on Windows the same 103 tests fail onstaging)uv run ruff check code_review_graph/uv run mypy code_review_graph/ --ignore-missing-imports --no-strict-optional(Linux; on Windows the same 3 POSIX-only errors indaemon.pyasstaging)docs/, docstrings):_resolve_extends()docstring and aCHANGELOG.mdentry under[Unreleased]🤖 Generated with Claude Code