Skip to content

Conversation

edison1105
Copy link
Member

@edison1105 edison1105 commented Jun 18, 2025

close #13484

Summary by CodeRabbit

  • New Features

    • Support for the ${configDir} variable in TypeScript path mappings and include/exclude patterns when resolving multi-config projects on TypeScript 5.5+.
  • Tests

    • Added a test verifying ${configDir} handling in complex tsconfig setups with project references and path aliases, ensuring correct symbol resolution and dependency tracking.

Copy link

coderabbitai bot commented Jun 18, 2025

Walkthrough

A test was added validating TypeScript ${configDir} resolution with extended tsconfigs and project references. The resolver now conditionally substitutes ${configDir} in include/exclude and path patterns when TypeScript version is 5.5+ during multi-config resolution.

Changes

Cohort / File(s) Change Summary
New test
packages/compiler-sfc/__tests__/compileScript/resolveType.spec.ts
Added a test covering TS project references, extended tsconfigs, and ${configDir} usage in paths/includes.
Resolver logic
packages/compiler-sfc/src/script/resolveType.ts
In multi-config resolution, detect TS major/minor, introduce getPattern(base, p) to substitute ${configDir} only for TS >= 5.5 and use it for include/exclude matching.

Sequence Diagram(s)

sequenceDiagram
    participant TestRunner
    participant SFCCompiler
    participant TSResolver
    participant FS

    TestRunner->>SFCCompiler: compile SFC with extended tsconfigs + ${configDir}
    SFCCompiler->>TSResolver: load tsconfig(s), request ts.versionMajorMinor
    TSResolver-->>SFCCompiler: ts version (major.minor) and merged config
    alt TS >= 5.5
        SFCCompiler->>FS: resolve patterns by substituting ${configDir} with config dir
    else TS < 5.5
        SFCCompiler->>FS: resolve patterns via joinPaths(base, pattern) (no ${configDir} substitution)
    end
    FS-->>SFCCompiler: resolved include/exclude file list
    SFCCompiler-->>TestRunner: resolved types and dependency list
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Assessment against linked issues

Objective Addressed Explanation
Correctly resolve ${configDir} in extended tsconfig files (paths/include) for Vue SFC type resolution (#13484)

Out-of-scope changes

No out-of-scope functional changes detected.

Suggested reviewers

  • Doctor-wu
  • LittleSound

Poem

"I nibbled paths where configs hide,
${configDir} now fits inside.
Tests applaud with carrot cheer,
Types resolved — the coast is clear! 🐇"


📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 7588b4d and 06b1037.

📒 Files selected for processing (2)
  • packages/compiler-sfc/__tests__/compileScript/resolveType.spec.ts (1 hunks)
  • packages/compiler-sfc/src/script/resolveType.ts (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • packages/compiler-sfc/tests/compileScript/resolveType.spec.ts
  • packages/compiler-sfc/src/script/resolveType.ts
✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch edison/feat/resolveConfigDir

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore or @coderabbit ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

github-actions bot commented Jun 18, 2025

Size Report

Bundles

File Size Gzip Brotli
runtime-dom.global.prod.js 101 kB 38.4 kB 34.6 kB
vue.global.prod.js 159 kB 58.5 kB 52.1 kB

Usages

Name Size Gzip Brotli
createApp (CAPI only) 46.5 kB 18.2 kB 16.7 kB
createApp 54.5 kB 21.2 kB 19.4 kB
createSSRApp 58.7 kB 22.9 kB 20.9 kB
defineCustomElement 59.5 kB 22.8 kB 20.8 kB
overall 68.5 kB 26.4 kB 24 kB

Copy link

pkg-pr-new bot commented Jun 18, 2025

Open in StackBlitz

@vue/compiler-core

npm i https://pkg.pr.new/@vue/compiler-core@13491

@vue/compiler-dom

npm i https://pkg.pr.new/@vue/compiler-dom@13491

@vue/compiler-sfc

npm i https://pkg.pr.new/@vue/compiler-sfc@13491

@vue/compiler-ssr

npm i https://pkg.pr.new/@vue/compiler-ssr@13491

@vue/reactivity

npm i https://pkg.pr.new/@vue/reactivity@13491

@vue/runtime-core

npm i https://pkg.pr.new/@vue/runtime-core@13491

@vue/runtime-dom

npm i https://pkg.pr.new/@vue/runtime-dom@13491

@vue/server-renderer

npm i https://pkg.pr.new/@vue/server-renderer@13491

@vue/shared

npm i https://pkg.pr.new/@vue/shared@13491

vue

npm i https://pkg.pr.new/vue@13491

@vue/compat

npm i https://pkg.pr.new/@vue/compat@13491

commit: 06b1037

@edison1105 edison1105 marked this pull request as ready for review June 18, 2025 06:41
@edison1105 edison1105 added scope: sfc 🔨 p3-minor-bug Priority 3: this fixes a bug, but is an edge case that only affects very specific usage. labels Jun 18, 2025
@edison1105 edison1105 changed the title feat(compiler-sfc): support ${configDir} in paths for TypeScript 5.5+ fix(compiler-sfc): support ${configDir} in paths for TypeScript 5.5+ Jun 18, 2025
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
packages/compiler-sfc/src/script/resolveType.ts (1)

1048-1054: Minor style improvement – use optional chaining

Static-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?.some above.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 532cfae and 5fab826.

📒 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 test

The new case accurately reproduces #13484 and guards the compiler against future regressions in ${configDir} handling. No concerns.

edison1105 and others added 2 commits June 18, 2025 14:46
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
@edison1105 edison1105 force-pushed the edison/feat/resolveConfigDir branch from 1024d53 to 7588b4d Compare June 18, 2025 06:50
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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] = … and function getPattern… are recreated every time resolveWithTS is invoked. They only depend on ts.versionMajorMinor and configPath, both of which are invariant during a single compiler run. Hoisting them to the outer scope of resolveWithTS (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 streamline excluded check

Small 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

📥 Commits

Reviewing files that changed from the base of the PR and between 5fab826 and 1024d53.

📒 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 good

The new supportsConfigDir check (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.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (2)
packages/compiler-sfc/src/script/resolveType.ts (2)

1032-1039: Protect against pre-release version strings & compute once

Good call switching to major > 5 || (major === 5 && minor >= 5) – it future-proofs the guard.
Two minor nits:

  1. Number('5.5-beta') becomes NaN, disabling the feature for nightly/beta drops.
    Use parseInt (or a regex) to strip non-digits first.

  2. supportsConfigDir is recalculated on every getPattern invocation 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 readability

Biome 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

📥 Commits

Reviewing files that changed from the base of the PR and between 1024d53 and 7588b4d.

📒 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

@edison1105 edison1105 added the ready to merge The PR is ready to be merged. label Jun 18, 2025
@edison1105
Copy link
Member Author

/ecosystem-ci run

@vue-bot
Copy link
Contributor

vue-bot commented Sep 1, 2025

📝 Ran ecosystem CI: Open

suite result latest scheduled
radix-vue success success
primevue success success
pinia success success
language-tools success success
vue-i18n success success
vueuse success success
vue-macros failure failure
vuetify success success
quasar success success
nuxt success success
test-utils success success
router success success
vant failure failure
vue-simple-compiler success success
vitepress success success
vite-plugin-vue success success

@edison1105 edison1105 merged commit 8696e34 into main Sep 2, 2025
16 checks passed
@edison1105 edison1105 deleted the edison/feat/resolveConfigDir branch September 2, 2025 09:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🔨 p3-minor-bug Priority 3: this fixes a bug, but is an edge case that only affects very specific usage. ready to merge The PR is ready to be merged. scope: sfc

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Vue compiler does not correctly resolve ${configDir} from extended tsconfigs

2 participants