fix(compiler-sfc): support ${configDir} in paths for TypeScript 5.5+ #13491
Conversation
WalkthroughA new test case was added to verify TypeScript's Changes
Sequence Diagram(s)sequenceDiagram
participant TestRunner
participant Compiler
participant TypeScript
participant FileSystem
TestRunner->>Compiler: Compile Vue SFC with extended tsconfig using ${configDir}
Compiler->>TypeScript: Load tsconfig(s) and parse version
TypeScript-->>Compiler: Return config with include/exclude patterns
Compiler->>FileSystem: Resolve paths with ${configDir} if TS >= 5.5
FileSystem-->>Compiler: Return normalized file paths
Compiler-->>TestRunner: Return resolved types and dependency info
Assessment against linked issues
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Size ReportBundles
Usages
|
@vue/compiler-core
@vue/compiler-dom
@vue/compiler-sfc
@vue/compiler-ssr
@vue/reactivity
@vue/runtime-core
@vue/runtime-dom
@vue/server-renderer
@vue/shared
vue
@vue/compat
commit: |
edison1105
added
scope: sfc
🔨 p3-minor-bug
edison1105
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/compiler-sfc/src/script/resolveType.ts (1)
1048-1054: Minor style improvement – use optional chainingStatic-analysis already hints at it. The manual
excluded && …can be trimmed:- if ( - excluded && - excluded.some(p => isMatch(containingFile, getPattern(base, p))) - ) { + if ( + excluded?.some(p => isMatch(containingFile, getPattern(base, p))) + ) { continue }Touches no logic, just terser & consistent with the
included?.someabove.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/compiler-sfc/__tests__/compileScript/resolveType.spec.ts(1 hunks)packages/compiler-sfc/src/script/resolveType.ts(2 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
packages/compiler-sfc/src/script/resolveType.ts
[error] 1052-1053: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Redirect rules
- GitHub Check: Header rules
- GitHub Check: Pages changed
🔇 Additional comments (1)
packages/compiler-sfc/__tests__/compileScript/resolveType.spec.ts (1)
1158-1195: 👍 Robust regression testThe new case accurately reproduces #13484 and guards the compiler against future regressions in
${configDir}handling. No concerns.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
edison1105
force-pushed
the
edison/feat/resolveConfigDir
branch
from
1024d53 to
7588b4d
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/compiler-sfc/src/script/resolveType.ts (2)
1032-1039: Minor refactor opportunity: lift helper + version parsing out of the loop
const [major, minor] = …andfunction getPattern…are recreated every timeresolveWithTSis invoked. They only depend onts.versionMajorMinorandconfigPath, both of which are invariant during a single compiler run. Hoisting them to the outer scope ofresolveWithTS(or caching the parsed tuple) would avoid unnecessary re-allocation and keep the loop body slimmer.Not critical, just a readability / micro-perf improvement.
1050-1055: Use optional chaining to streamlineexcludedcheckSmall readability tweak: the explicit null check can be replaced with optional chaining.
- if ( - excluded && - excluded.some(p => isMatch(containingFile, getPattern(base, p))) - ) { + if ( + excluded?.some(p => isMatch(containingFile, getPattern(base, p))) + ) {No behavioural change; simply reduces nesting.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/compiler-sfc/src/script/resolveType.ts(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/compiler-sfc/src/script/resolveType.ts (1)
packages/compiler-sfc/src/script/utils.ts (2)
normalizePath(107-109)joinPaths(111-112)
🪛 Biome (1.9.4)
packages/compiler-sfc/src/script/resolveType.ts
[error] 1053-1054: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (1)
packages/compiler-sfc/src/script/resolveType.ts (1)
1032-1038: Version-guard correctly future-proofed — looks goodThe new
supportsConfigDircheck (major > 5 || (major === 5 && minor >= 5)) removes the earlier logical gap for TS 6.x+ and resolves the concern raised in the previous review.
Implementation is clear and side-effect-free.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/compiler-sfc/src/script/resolveType.ts (2)
1032-1039: Protect against pre-release version strings & compute onceGood call switching to
major > 5 || (major === 5 && minor >= 5)– it future-proofs the guard.
Two minor nits:
Number('5.5-beta')becomesNaN, disabling the feature for nightly/beta drops.
UseparseInt(or a regex) to strip non-digits first.
supportsConfigDiris recalculated on everygetPatterninvocation although it’s invariant – lift it outside the arrow for micro clarity.-const [major, minor] = ts.versionMajorMinor.split('.').map(Number) +const [majorRaw, minorRaw] = ts.versionMajorMinor.split('.') +const major = parseInt(majorRaw, 10) +const minor = parseInt(minorRaw, 10) -const getPattern = (base: string, p: string) => { - // ts 5.5+ supports ${configDir} in paths - const supportsConfigDir = major > 5 || (major === 5 && minor >= 5) +// ts 5.5+ supports ${configDir} in paths +const supportsConfigDir = major > 5 || (major === 5 && minor >= 5) + +const getPattern = (base: string, p: string) => {
1049-1055: Use optional chaining to quiet lint & improve readabilityBiome flags this pattern check; a terse optional-chain keeps the meaning while removing an explicit null-guard:
- if ( - (!included && (!base || containingFile.startsWith(base))) || - included?.some(p => isMatch(containingFile, getPattern(base, p))) - ) { + if ( + (!included && (!base || containingFile.startsWith(base))) || + included?.some(p => isMatch(containingFile, getPattern(base, p))) + ) { if ( - excluded && - excluded.some(p => isMatch(containingFile, getPattern(base, p))) + excluded?.some(p => + isMatch(containingFile, getPattern(base, p)), + ) ) {This is purely stylistic – feel free to keep the existing guard if you prefer explicit checks.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/compiler-sfc/src/script/resolveType.ts(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/compiler-sfc/src/script/resolveType.ts (1)
packages/compiler-sfc/src/script/utils.ts (2)
normalizePath(107-109)joinPaths(111-112)
🪛 Biome (1.9.4)
packages/compiler-sfc/src/script/resolveType.ts
[error] 1053-1054: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: continuous-release
- GitHub Check: test / e2e-test
- GitHub Check: test / unit-test-windows
close #13484
Summary by CodeRabbit
New Features
${configDir}variable in TypeScript path mappings and include/exclude patterns when using TypeScript 5.5 or higher.Tests
${configDir}in complex tsconfig setups with project references and path aliases.