fix(compiler-sfc): resolve type re-exports inside declare global#14766
Merged
edison1105 merged 6 commits intovuejs:mainfrom May 6, 2026
Merged
fix(compiler-sfc): resolve type re-exports inside declare global#14766edison1105 merged 6 commits intovuejs:mainfrom
edison1105 merged 6 commits intovuejs:mainfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds eager resolution and re-export propagation for declarations inside ambient ChangesGlobal re-export handling & tests
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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: |
Member
Author
|
I don't believe the test failure is related 🤔 |
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/compiler-sfc/src/script/resolveType.ts`:
- Around line 1340-1376: The code currently copies AST nodes by reference from
sourceScope (via sourceScope.exportedTypes/exportedDeclares) into the local
types/declares maps in the ExportNamedDeclaration re-export handling and the
ExportAllDeclaration branch, which allows later owner-scope normalization to
mutate the original sourceScope nodes; instead, clone the nodes before storing
them locally (e.g. when assigning types[exported] =
sourceScope.exportedTypes[local] and declares[exported] =
sourceScope.exportedDeclares[local], and when merging Object.assign(types,
sourceScope.exportedTypes)/declares) so the local scope gets fresh copies; use
the project’s existing node-cloning helper (or a lightweight deep/structured
clone) to produce new node instances so owner/parent normalization won’t corrupt
the cached source file state.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0c25b506-3292-41c5-a7e5-e4a6555cbe08
📒 Files selected for processing (2)
packages/compiler-sfc/__tests__/compileScript/resolveType.spec.tspackages/compiler-sfc/src/script/resolveType.ts
Member
Author
|
thank you dear rabbit 🐇 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/compiler-sfc/__tests__/compileScript/resolveType.spec.ts (1)
1500-1534: Consider asserting transitive dependency tracking for re-exported globals.This test validates type resolution well, but adding a
depsassertion would also lock in cache/invalidation behavior for./foo,./bar, and./bazloaded via/global.d.ts.Suggested test hardening
- expect( - resolve(`defineProps<Foo>()`, files, globalTypeFiles).props, - ).toStrictEqual({ + const fooRes = resolve(`defineProps<Foo>()`, files, globalTypeFiles) + expect(fooRes.props).toStrictEqual({ foo: ['Number'], }) + expect(fooRes.deps && [...fooRes.deps]).toEqual( + expect.arrayContaining(['/global.d.ts', '/foo.ts']), + ) - expect( - resolve(`defineProps<Bar>()`, files, globalTypeFiles).props, - ).toStrictEqual({ + const barRes = resolve(`defineProps<Bar>()`, files, globalTypeFiles) + expect(barRes.props).toStrictEqual({ bar: ['Boolean'], }) + expect(barRes.deps && [...barRes.deps]).toEqual( + expect.arrayContaining(['/global.d.ts', '/bar.ts']), + ) - expect( - resolve(`defineProps<Baz>()`, files, globalTypeFiles).props, - ).toStrictEqual({ + const bazRes = resolve(`defineProps<Baz>()`, files, globalTypeFiles) + expect(bazRes.props).toStrictEqual({ baz: ['String'], }) + expect(bazRes.deps && [...bazRes.deps]).toEqual( + expect.arrayContaining(['/global.d.ts', '/baz.ts']), + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/compiler-sfc/__tests__/compileScript/resolveType.spec.ts` around lines 1500 - 1534, The test "global types with re-exports" verifies props but doesn't assert dependency tracking; update the test to also assert resolve(...) returns the expected deps so cache/invalidation is locked in: after each resolve(...) call (or once per file), assert the returned deps includes '/global.d.ts' plus the transitive source(s) ('/foo.ts', '/bar.ts', '/baz.ts') as appropriate; modify the assertions around resolve in this test to check both .props and .deps (using the resolve helper and the file names above) so re-exported globals are tracked as dependencies.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/compiler-sfc/__tests__/compileScript/resolveType.spec.ts`:
- Around line 1500-1534: The test "global types with re-exports" verifies props
but doesn't assert dependency tracking; update the test to also assert
resolve(...) returns the expected deps so cache/invalidation is locked in: after
each resolve(...) call (or once per file), assert the returned deps includes
'/global.d.ts' plus the transitive source(s) ('/foo.ts', '/bar.ts', '/baz.ts')
as appropriate; modify the assertions around resolve in this test to check both
.props and .deps (using the resolve helper and the file names above) so
re-exported globals are tracked as dependencies.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cc9fea78-4d83-405c-9c81-dc15cfc0f0bf
📒 Files selected for processing (2)
packages/compiler-sfc/__tests__/compileScript/resolveType.spec.tspackages/compiler-sfc/src/script/resolveType.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/compiler-sfc/src/script/resolveType.ts
edison1105
added
scope: sfc
🔨 p3-minor-bug
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/compiler-sfc/src/script/resolveType.ts`:
- Around line 1352-1380: The code calls importSourceToScope while indexing
`declare global`, which mutates ctx.deps and causes spurious dependency records;
fix this by preventing dependency tracking during those calls (either add an
options param to importSourceToScope like {recordDeps:false} and pass it from
the `ExportNamedDeclaration`/`ExportAllDeclaration` branches inside the
declare-global handling, or temporarily save/restore ctx.deps around the call),
ensuring only the real resolved source is recorded by the normal lookup path;
reference importSourceToScope, ctx.deps, and the export handling blocks that
handle ExportNamedDeclaration/ExportAllDeclaration in declare global.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 246cb6ca-752a-48d4-b8ca-22388d859fa5
📒 Files selected for processing (2)
packages/compiler-sfc/__tests__/compileScript/resolveType.spec.tspackages/compiler-sfc/src/script/resolveType.ts
Member
|
/ecosystem-ci run |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/compiler-sfc/src/script/resolveType.ts`:
- Around line 782-785: Cache reuse short-circuits by returning previously
resolved entries (via _resolvedElements / _resolvedReference) but only adds the
immediate owner (resolved._ownerScope) to ctx.deps, so transitive source deps
(e.g., global.d.ts -> foo.ts -> bar.ts or foo.ts -> types.ts) are lost; update
the resolution path in resolveType.ts to walk the chain of owner scopes for a
resolved entry (starting from resolved and following resolved._ownerScope links
until null or s) and add each ownerScope.filename into ctx.deps (in addition to
the current immediate-owner logic that checks resolved._ownerScope !== s),
ensuring you also handle cached _resolvedElements / _resolvedReference cases by
collecting and merging their transitive owner filenames into ctx.deps so deeper
files aren’t dropped on subsequent compiles.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 30c9d70f-ede7-4406-abaf-95d77c8eb757
📒 Files selected for processing (2)
packages/compiler-sfc/__tests__/compileScript/resolveType.spec.tspackages/compiler-sfc/src/script/resolveType.ts
Contributor
|
📝 Ran ecosystem CI: Open
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/compiler-sfc/src/script/resolveType.ts (1)
155-176:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReplay cached call-signature deps too.
ResolvedElementsalso backsdefineEmits(), butrecordResolvedElementDeps()only re-adds prop owners. On a cache hit, an imported interface that contributes onlycallsviaextendswill lose its transitive source files fromctx.deps, so emit-type edits can stop invalidating HMR correctly.Possible fix
interface ResolvedElements { props: Record< string, (TSPropertySignature | TSMethodSignature) & { // resolved props always has ownerScope attached _ownerScope: TypeScope } > - calls?: (TSCallSignatureDeclaration | TSFunctionType)[] + calls?: ((TSCallSignatureDeclaration | TSFunctionType) & MaybeWithScope)[] } function recordResolvedElementDeps( ctx: TypeResolveContext, - { props }: ResolvedElements, + { props, calls }: ResolvedElements, ): void { for (const key in props) { recordScopeDep(ctx, props[key]._ownerScope) } + for (const call of calls || []) { + recordScopeDep(ctx, call._ownerScope) + } }case 'TSFunctionType': { + ;(node as MaybeWithScope)._ownerScope ??= scope return { props: {}, calls: [node] } }} else if (e.type === 'TSCallSignatureDeclaration') { + ;(e as MaybeWithScope)._ownerScope = scope ;(res.calls || (res.calls = [])).push(e) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/compiler-sfc/src/script/resolveType.ts` around lines 155 - 176, The cache hit path in resolveTypeElements only replays deps for props via recordResolvedElementDeps, which ignores call-signature contributors, causing their _ownerScope to not be re-recorded; update recordResolvedElementDeps (or the cache-hit handling in resolveTypeElements) to also iterate over ResolvedElements.calls and call recordScopeDep(ctx, call._ownerScope) for each call entry (in addition to props) so imported interfaces that only contribute call signatures still have their transitive source files added to ctx.deps.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@packages/compiler-sfc/src/script/resolveType.ts`:
- Around line 155-176: The cache hit path in resolveTypeElements only replays
deps for props via recordResolvedElementDeps, which ignores call-signature
contributors, causing their _ownerScope to not be re-recorded; update
recordResolvedElementDeps (or the cache-hit handling in resolveTypeElements) to
also iterate over ResolvedElements.calls and call recordScopeDep(ctx,
call._ownerScope) for each call entry (in addition to props) so imported
interfaces that only contribute call signatures still have their transitive
source files added to ctx.deps.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4dbfd0c7-e63e-4fe8-b310-2c22cfb2211b
📒 Files selected for processing (2)
packages/compiler-sfc/__tests__/compileScript/resolveType.spec.tspackages/compiler-sfc/src/script/resolveType.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
resolves nuxt/nuxt#30213
this was an omission in my earlier pr #13789
this now handles both named exports and star exports
Summary by CodeRabbit
Bug Fixes
Tests