mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-10-05 02:43:32 +00:00
fix(typescript): resolve tsconfig paths aliases on Windows (#3203)
* fix(typescript): resolve tsconfig `paths` aliases on Windows `rebaseTarget` turns a `paths` target that `path.resolve` produced back into a repo-relative one, and it recognised the wildcard suffix only as `/*`. On Windows the resolved target is `C:\repo\src\*`, so the check never matched, the bare-`*` branch stripped the star, `path.relative` ate the trailing backslash, and every alias target came back as `src*`. `substituteStar` then built `srclib/date` from `@/lib/date`, nothing resolved, and every alias import was dropped as external — no IMPORTS/CALLS edge for anything reached through `@/`, so `impact` answered UNKNOWN for a repo's whole shared layer. Normalise the separator before looking at the suffix. `tsconfig-index.test.ts` already asserts the `src/*` shape and fails 4 of 13 cases on Windows without this; it only ever ran on Ubuntu, so add it to the cross-platform lane. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * test(typescript): make `paths` rebasing assertable from any runner `rebaseTarget` reads the platform separator, but the only suite covering it builds real directories — so on Ubuntu it sees `/` whatever the code does with `\`, and only the windows-latest lane can fail it. That is how an alias bug affecting every Windows install reached a green CI in the first place. Thread an injectable `pathApi` through `rebaseTarget` and `repoRelative` — the seam `isInside` and the `\\?\` prefix guard already use — and add a fixture-free suite that pins the win32 and POSIX branches explicitly. Deleting the separator normalisation now fails on every runner rather than only on windows-latest. Registered beside its fixture sibling in the cross-platform lane so the two halves of the same rule stay discoverable as one group. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(typescript): emit a bare `*` for a repo-root `paths` target A target naming the repo ROOT — `"*": ["./*"]` under `"baseUrl": "."` — rebases to an EMPTY repo-relative prefix, so only the suffix survived and the encoding came out as `/*`. `substituteStar` turns that into `/lib/date`, and `resolveFile` matches repo-relative keys without stripping a leading slash, so the indexed `lib/date.ts` misses and the alias is dropped as external. Emit the bare `*` for that case instead, which substitutes to `lib/date` and resolves. This is the shipped POSIX encoding as much as the Windows one, and it is what the Windows path emitted by accident before the separator was normalised — so the normalisation does not narrow what already resolved there. The common `"@/*": ["./src/*"]` is untouched: it has a real prefix and stays `src/*`. Pinned at all three levels the encoding passes through: the rebaser, on both path flavours; the loader, through a real fixture; and resolution, where `*` finds the file and `/*` does not. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(test): say why the root-wildcard fixture declares no `baseUrl` The comment introduced the fixture as `"baseUrl": "."`, which the loader encodes as `''`, while the fixture passes `null` — a different scope, since `null` means the config declares no baseUrl at all and takes the paths arm alone. `null` is the right fixture and the comment was the wrong half: with `''` the baseUrl arm resolves `packages/utils/src/index` by itself, so the negative case would return null for a reason unrelated to the target encoding. Verified by substitution — `''` makes that assertion report the file instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com> Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>
This commit is contained in:
parent
2f7e192add
commit
ab25b86807
5 changed files with 165 additions and 6 deletions
|
|
@ -36,6 +36,18 @@ const PLATFORM_LOGIC = [
|
|||
// must exercise the Windows backslash branch, so run it on the OS matrix (#2394).
|
||||
'test/unit/cli-entry.test.ts',
|
||||
'test/unit/platform-capabilities.test.ts',
|
||||
// The tsconfig loader rebases `paths` targets through `path.resolve`, so the
|
||||
// wildcard suffix it must recognise is `/*` on POSIX and `\*` on Windows. It
|
||||
// only looked for `/*`, and every alias target came back as `src*` on
|
||||
// Windows while the Ubuntu run stayed green — so this file has to run where
|
||||
// the separator differs.
|
||||
'test/unit/tsconfig-index.test.ts',
|
||||
// The unit half of the same rebasing rule. Fixture-free and pathApi-injectable
|
||||
// (every separator assertion passes an explicit `path.win32` / `path.posix`),
|
||||
// so unlike the fixture suite above it fails on EVERY runner when the
|
||||
// normalisation is removed rather than only on windows-latest. Registered
|
||||
// beside its fixture sibling so the two halves stay discoverable as one group.
|
||||
'test/unit/tsconfig-rebase-target.test.ts',
|
||||
// The gitnexus-plan safe writer resolves every name through a per-platform
|
||||
// backend: Linux anchors through /proc/self/fd, macOS resolves lexically and
|
||||
// verifies each step against descriptors it holds open. Publication is link(2)
|
||||
|
|
|
|||
|
|
@ -318,8 +318,8 @@ function parseJsonc(raw: string): Record<string, unknown> {
|
|||
return JSON.parse(withoutComments) as Record<string, unknown>;
|
||||
}
|
||||
|
||||
function repoRelative(repoRoot: string, absDir: string): string {
|
||||
const rel = path.relative(repoRoot, absDir).split(path.sep).join('/');
|
||||
function repoRelative(repoRoot: string, absDir: string, pathApi: typeof path = path): string {
|
||||
const rel = pathApi.relative(repoRoot, absDir).split(pathApi.sep).join('/');
|
||||
return rel === '.' || rel === '' ? '' : rel;
|
||||
}
|
||||
|
||||
|
|
@ -329,11 +329,34 @@ function repoRelative(repoRoot: string, absDir: string): string {
|
|||
* `path.resolve` swallows the wildcard into a path segment, so it is stripped
|
||||
* before resolving and re-appended after — the `*` is a substitution marker,
|
||||
* not a directory named `*`.
|
||||
*
|
||||
* `pathApi` is injectable so the win32 separator branch is unit-testable from a
|
||||
* POSIX runner; production callers always use the platform-bound `path`.
|
||||
*/
|
||||
function rebaseTarget(repoRoot: string, absTarget: string): string {
|
||||
function rebaseTarget(repoRoot: string, absTarget: string, pathApi: typeof path = path): string {
|
||||
// `/repo/src/*` must come back as `src/*`, not `src*`: stripping only the
|
||||
// star leaves a trailing slash that `path.relative` then eats.
|
||||
const suffix = absTarget.endsWith('/*') ? '/*' : absTarget.endsWith('*') ? '*' : '';
|
||||
const base = suffix === '' ? absTarget : absTarget.slice(0, -suffix.length);
|
||||
return `${repoRelative(repoRoot, base)}${suffix}`;
|
||||
//
|
||||
// The target arrives from `path.resolve`, so on Windows it is `C:\repo\src\*`
|
||||
// and an `endsWith('/*')` check never matches. It fell through to the bare
|
||||
// `*` branch, `path.relative` ate the trailing backslash, and every alias
|
||||
// target came back as `src*` — which `substituteStar` turns into `srclib/x`,
|
||||
// so nothing an alias reached was ever resolved on Windows. Normalise the
|
||||
// separator before looking at the suffix.
|
||||
const normalized = absTarget.split(pathApi.sep).join('/');
|
||||
const suffix = normalized.endsWith('/*') ? '/*' : normalized.endsWith('*') ? '*' : '';
|
||||
const base = suffix === '' ? normalized : normalized.slice(0, -suffix.length);
|
||||
const prefix = repoRelative(repoRoot, base, pathApi);
|
||||
// A target naming the repo ROOT (`"*": ["./*"]` under `baseUrl: "."`) leaves
|
||||
// an empty prefix, and `${''}${'/*'}` is `/*`. `substituteStar` turns that
|
||||
// into `/lib/date`, but `resolveFile` matches repo-relative keys and never
|
||||
// strips a leading slash, so `lib/date.ts` misses and the alias goes
|
||||
// external. The bare `*` is the encoding that substitutes correctly — and it
|
||||
// is what the pre-#3203 Windows path emitted by accident, so this keeps the
|
||||
// separator fix from narrowing what already resolved there.
|
||||
if (prefix === '' && suffix === '/*') return '*';
|
||||
return `${prefix}${suffix}`;
|
||||
}
|
||||
|
||||
/** Test seam for {@link rebaseTarget} (see `test/unit/tsconfig-rebase-target.test.ts`). */
|
||||
export const _rebaseTargetForTests = rebaseTarget;
|
||||
|
|
|
|||
|
|
@ -181,6 +181,29 @@ describe('tsconfig paths', () => {
|
|||
expect(resolve('~/index', { tsconfigs: twoTargets })).toBe('packages/utils/src/index.ts');
|
||||
});
|
||||
|
||||
it('substitutes a repo-root target written as the bare `*`', () => {
|
||||
// A `"*": ["./*"]` target resolves to the repo root, so `rebaseTarget`
|
||||
// leaves an EMPTY repo-relative prefix and the suffix is the whole
|
||||
// encoding. This is the pair that says why it must be `*` and not `/*`.
|
||||
//
|
||||
// `baseUrl` is `null` — no baseUrl declared — rather than the `''` the
|
||||
// loader emits for a real `"baseUrl": "."`, and deliberately so: `''`
|
||||
// resolves `packages/utils/src/index` through the baseUrl arm on its own,
|
||||
// which would answer the negative case for a reason that has nothing to do
|
||||
// with the target encoding. `paths` is tried first either way, so cutting
|
||||
// the fallback is what leaves this asserting only what it names.
|
||||
const rootWildcard = (target: string): TsconfigIndex => ({
|
||||
scopes: [{ dir: '', baseUrl: null, paths: [{ pattern: '*', targets: [target] }] }],
|
||||
});
|
||||
|
||||
expect(resolve('packages/utils/src/index', { tsconfigs: rootWildcard('*') })).toBe(
|
||||
'packages/utils/src/index.ts',
|
||||
);
|
||||
// `substituteStar('/*', …)` yields `/packages/utils/src/index`, and
|
||||
// `resolveFile` matches repo-relative keys without a leading slash.
|
||||
expect(resolve('packages/utils/src/index', { tsconfigs: rootWildcard('/*') })).toBeNull();
|
||||
});
|
||||
|
||||
it('applies the nearest config, not the root one', () => {
|
||||
const nested: TsconfigIndex = {
|
||||
scopes: [
|
||||
|
|
|
|||
|
|
@ -231,6 +231,24 @@ describe('parsing', () => {
|
|||
expect(scope?.paths[0]?.targets).toEqual(['src/*', 'generated/*']);
|
||||
});
|
||||
|
||||
it('encodes a repo-root target as the bare `*`', async () => {
|
||||
// `"*": ["./*"]` resolves to the repo root itself, so the repo-relative
|
||||
// prefix is empty and the suffix is the whole target. `/*` substitutes to
|
||||
// `/lib/date`, which `resolveFile` never matches — see the unit assertions
|
||||
// in `tsconfig-rebase-target.test.ts` for both path flavours.
|
||||
const root = repo({
|
||||
'tsconfig.json': JSON.stringify({
|
||||
compilerOptions: { baseUrl: '.', paths: { '*': ['./*'], '@/*': ['./src/*'] } },
|
||||
}),
|
||||
});
|
||||
|
||||
const scope = tsconfigFor(await loadTsconfigIndex(root), 'src/a.ts');
|
||||
|
||||
expect(scope?.paths.find((mapping) => mapping.pattern === '*')?.targets).toEqual(['*']);
|
||||
// The common alias is unaffected — only the empty-prefix case changes.
|
||||
expect(scope?.paths.find((mapping) => mapping.pattern === '@/*')?.targets).toEqual(['src/*']);
|
||||
});
|
||||
|
||||
it('returns null for a repo with no config at all', async () => {
|
||||
expect(await loadTsconfigIndex(repo({ 'src/a.ts': '' }))).toBeNull();
|
||||
});
|
||||
|
|
|
|||
83
gitnexus/test/unit/tsconfig-rebase-target.test.ts
Normal file
83
gitnexus/test/unit/tsconfig-rebase-target.test.ts
Normal file
|
|
@ -0,0 +1,83 @@
|
|||
/**
|
||||
* `rebaseTarget` — `paths` target rebasing, across both path flavours.
|
||||
*
|
||||
* The fixture suite (`tsconfig-index.test.ts`) builds real directories, so it
|
||||
* only ever sees the HOST separator: its `…/*` assertions are green on Ubuntu
|
||||
* whatever `rebaseTarget` does with a backslash, and only the windows-latest
|
||||
* lane can fail them. That is how the alias bug survived a green CI in the
|
||||
* first place. Injecting `pathApi` — the seam `isInside` and the `\\?\` prefix
|
||||
* guard already use — makes both platform branches assertable from any runner,
|
||||
* so deleting the separator normalisation fails here on Ubuntu too.
|
||||
*
|
||||
* Two behaviours are pinned:
|
||||
*
|
||||
* - `path.resolve` emits `C:\repo\src\*` on Windows, so the `endsWith('/*')`
|
||||
* check never matched, the bare-`*` branch ate the trailing separator, and
|
||||
* every alias target came back as `src*` — `substituteStar` then produced
|
||||
* `srclib/date`, which matches no file, so the common Vite/shadcn
|
||||
* `"@/*": ["./src/*"]` resolved to nothing on Windows.
|
||||
* - a repo-ROOT target (`"*": ["./*"]` under `baseUrl: "."`) rebases to an
|
||||
* EMPTY prefix, and `${''}${'/*'}` is `/*`. `substituteStar('/*', 'lib/date')`
|
||||
* yields `/lib/date`, and `resolveFile` matches repo-relative keys without
|
||||
* a leading slash, so the alias went external. The bare `*` is the encoding
|
||||
* that substitutes correctly, on both platforms.
|
||||
*/
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import path from 'node:path';
|
||||
import { _rebaseTargetForTests as rebaseTarget } from '../../src/core/ingestion/languages/typescript/tsconfig.js';
|
||||
|
||||
describe('rebaseTarget — Windows separator normalisation', () => {
|
||||
it('rebases the Vite/shadcn `@/*` target to `src/*`, not `src*`', () => {
|
||||
// What `path.win32.resolve('C:\\repo', './src/*')` hands the rebaser.
|
||||
expect(rebaseTarget('C:\\repo', 'C:\\repo\\src\\*', path.win32)).toBe('src/*');
|
||||
});
|
||||
|
||||
it('rebases a nested monorepo target', () => {
|
||||
expect(rebaseTarget('C:\\repo', 'C:\\repo\\packages\\ui\\src\\*', path.win32)).toBe(
|
||||
'packages/ui/src/*',
|
||||
);
|
||||
});
|
||||
|
||||
it('leaves a starless target alone', () => {
|
||||
expect(rebaseTarget('C:\\repo', 'C:\\repo\\src\\exact.ts', path.win32)).toBe('src/exact.ts');
|
||||
});
|
||||
|
||||
it('produces the identical result on POSIX', () => {
|
||||
// The normalisation is a no-op here — `split('/').join('/')` is identity —
|
||||
// so these are the OLD values as much as the new ones. Pinning them keeps
|
||||
// the Windows fix from being paid for with a POSIX regression.
|
||||
expect(rebaseTarget('/repo', '/repo/src/*', path.posix)).toBe('src/*');
|
||||
expect(rebaseTarget('/repo', '/repo/packages/ui/src/*', path.posix)).toBe('packages/ui/src/*');
|
||||
expect(rebaseTarget('/repo', '/repo/src/exact.ts', path.posix)).toBe('src/exact.ts');
|
||||
});
|
||||
|
||||
it('defaults to the platform-bound path module', () => {
|
||||
const root = path.resolve('repo');
|
||||
expect(rebaseTarget(root, path.resolve(root, './src/*'))).toBe('src/*');
|
||||
});
|
||||
});
|
||||
|
||||
describe('rebaseTarget — repo-root wildcard', () => {
|
||||
it('emits a bare `*` rather than `/*` for a root target', () => {
|
||||
// `"baseUrl": "."` with `"*": ["./*"]` — the target resolves to the repo
|
||||
// root itself, so the repo-relative prefix is empty and only the suffix is
|
||||
// left. `/*` substitutes to `/lib/date`, which no indexed key matches.
|
||||
expect(rebaseTarget('/repo', '/repo/*', path.posix)).toBe('*');
|
||||
expect(rebaseTarget('C:\\repo', 'C:\\repo\\*', path.win32)).toBe('*');
|
||||
});
|
||||
|
||||
it('still emits `/*` once the target is one directory in', () => {
|
||||
// The bare `*` is the empty-prefix case ONLY; a real prefix keeps its
|
||||
// separator or `src*` comes back, which is the bug above wearing a
|
||||
// different hat.
|
||||
expect(rebaseTarget('/repo', '/repo/src/*', path.posix)).toBe('src/*');
|
||||
expect(rebaseTarget('C:\\repo', 'C:\\repo\\src\\*', path.win32)).toBe('src/*');
|
||||
});
|
||||
|
||||
it('leaves a starless root target as the empty prefix', () => {
|
||||
// `resolveFile('')` is `null`, which is the honest answer for a target
|
||||
// naming the repo root and no file. Unchanged by the wildcard rule.
|
||||
expect(rebaseTarget('/repo', '/repo', path.posix)).toBe('');
|
||||
expect(rebaseTarget('C:\\repo', 'C:\\repo', path.win32)).toBe('');
|
||||
});
|
||||
});
|
||||
Loading…
Add table
Reference in a new issue