fix(config): support tsconfig extends array form - #2054
Conversation
## Summary - Handle the `extends` array form (TypeScript 5.0+) when resolving tsconfig/jsconfig path aliases and `baseUrl` for `next.config.ts` loading. - Normalize `extends` (string | string[] | undefined) into an ordered list at both loader sites; iterate parents so later entries override earlier ones. - Add array-extends regression tests (single parent, multi-parent precedence, child override). ## Root Cause Both tsconfig loaders only handled the string form of `extends` (`typeof parsed.extends === "string"`). When `extends` was an array (`["./a.json", "./b.json"]`), it was silently dropped, so inherited `compilerOptions.paths`/`baseUrl` were lost and path-aliased imports in `next.config.ts` failed to resolve. Next.js normalizes via `Array.isArray(config.extends) ? config.extends : [config.extends]` and merges parents in order; this change mirrors that behavior at both vinext sites (packages/vinext/src/config/tsconfig-paths.ts and packages/vinext/src/index.ts). ## References - Fixes cloudflare#1974 - Next.js parity: packages/next/src/build/next-config-ts/transpile-config.ts (loadTsConfigFile) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
commit: |
|
/bigbonk review for issues |
There was a problem hiding this comment.
Review: tsconfig array-form extends
The fix is correct and the merge semantics match Next.js parity. I verified against the canary source packages/next/src/build/next-config-ts/transpile-config.ts:
- Parents merged in order via
{ ...mergedOptions, ...parentOptions }→ later array entries override earlier ones ✅ paths: currentOptions.paths ?? mergedOptions.paths→ child overrides parents ✅baseUrlhandling intsconfig-paths.ts(parent.baseUrl ?? resolution.baseUrl) correctly lets a later parent's baseUrl win while falling back across entries ✅
The shared seen set is threaded correctly through both recursion sites so cyclic extends can't loop, and each parent resolves relative to its own directory. I ran tests/tsconfig-paths-config-loader.test.ts (12 passed) and tests/tsconfig-paths-vite8.test.ts (7 passed) — both green.
One gap worth addressing
The PR fixes two separate, duplicated implementations (config/tsconfig-paths.ts and index.ts::loadTsconfigPathAliases), but the new tests only exercise tsconfig-paths.ts via loadTsconfigPathAliasesForRoot. The index.ts site has no test coverage for the array form — the only extends test touching that path (tsconfig-paths-vite8.test.ts:122) still uses the string form (extends: "./tsconfig.base.json"). So the index.ts half of this fix is untested. Adding an array-form case to tsconfig-paths-vite8.test.ts (or the alias-build suite) would lock in the behavior at that call site too.
Pre-existing (not blocking)
This is the third+ copy of resolveTsconfigExtends / resolveTsconfigPathCandidate / the new normalize*Extends helpers across index.ts and config/tsconfig-paths.ts. The duplication predates this PR, but it's exactly the kind of divergence-prone copy AGENTS.md warns about — a future cleanup could consolidate these into the shared config/tsconfig-paths.ts module and have index.ts import them.
Nice work porting the Next.js test cases and linking them.
| const extendedPath = resolveTsconfigExtends(normalizedPath, extendsSpecifier); | ||
| if (extendedPath) { | ||
| aliases = loadTsconfigPathAliases(extendedPath, projectRoot, seen); | ||
| aliases = { ...aliases, ...loadTsconfigPathAliases(extendedPath, projectRoot, seen) }; |
There was a problem hiding this comment.
This array-form extends path in index.ts is the second site fixed, but it has no test coverage for the array form — tests/tsconfig-paths-vite8.test.ts:122 (the only extends test exercising loadTsconfigPathAliases) still uses the string form. Consider adding an array-form case to that suite so the index.ts half of this fix is locked in, not just the config/tsconfig-paths.ts half.
|
Review posted. Summary of my findings on PR #2054: Verdict: Correct fix, with one test-coverage gap. What's good:
Gap flagged (inline comment):
Pre-existing (non-blocking):
|
Summary
extends("extends": ["./a.json", "./b.json"]) when loading tsconfig path aliases.extendswas silently ignored and inheritedpaths/baseUrlwere dropped.Root Cause
Two sites resolved
extendsas a string only:config/tsconfig-paths.ts:const extendsList = typeof parsed.extends === "string" ? [parsed.extends] : [];index.ts:if (typeof parsed.extends === "string") { … }When a project used
extends: [...](common in monorepos with shared base configs), both produced an empty parent list, so path aliases /baseUrldefined in the extended configs were never loaded — andnext.config.tsimports through those aliases failed at config-load time.The fix normalizes
extends(string | string[] | undefined) into an ordered list at both sites and iterates the parents, merging so that later array entries override earlier ones and the child config overrides all parents — matching Next.js'loadTsConfigFile. The sharedseenset is threaded through every entry so cyclicextendscan't loop, and each parent's aliases continue to resolve relative to its own directory. Non-string array entries are filtered out; the single-string form is unchanged.References
extendsarray form ignored — path aliases dropped when loading next.config.ts #1974packages/next/src/build/next-config-ts/transpile-config.ts(Array.isArray(config.extends) ? config.extends : [config.extends], parents merged in order)Verification
CI=true pnpm test tests/tsconfig-paths-config-loader.test.ts— new cases (ported from Next.js): array-formextendsresolves inherited paths; later array entries override earlier; child overrides array-extended paths. Red before, green after; single-string regression and child-override tests still pass (12 passed).CI=true pnpm test tests/tsconfig-path-alias-build.test.ts tests/tsconfig-paths-vite8.test.ts— theindex.tscall-site suites pass (no regression).CI=true pnpm test— full battery green (only the pre-existing env flakesdeploy.test.ts > resolveWranglerBinandoxlint-prefer-shared-utils, reproduced identically onorigin/main).CI=true npx vp check— clean on all changed files.