Skip to content

Conversation

edison1105
Copy link
Member

@edison1105 edison1105 commented Sep 4, 2025

only for the strict pattern {[K in keyof T]: T[K]}

close #13847

Summary by CodeRabbit

  • New Features
    • Improved runtime inference for certain TypeScript mapped types in component props, producing more accurate runtime prop types and better support for advanced TS patterns.
  • Tests
    • Added tests covering mapped types with indexed access in defineProps to ensure correct prop type resolution.

…ith indexed access

only for the strict pattern {[K in keyof T]: T[K]}

close #13847
Copy link

coderabbitai bot commented Sep 4, 2025

Walkthrough

Adds support in inferRuntimeType for a specific TypeScript mapped-type pattern that uses indexed access and keyof constraints, and adds tests verifying resolution of a mapped type with indexed access in defineProps. Tests were added twice (duplicated). No public API changes.

Changes

Cohort / File(s) Summary
SFC compiler runtime type inference
packages/compiler-sfc/src/script/resolveType.ts
Adds handling in inferRuntimeType for a constrained TSMappedType pattern ({ [K in keyof T]: T[K] }) that validates indexed-access and keyof constraints and resolves to the referenced target type from typeParameters; returns UNKNOWN_TYPE for unsupported patterns.
CompileScript type resolution tests (duplicated additions)
packages/compiler-sfc/__tests__/compileScript/resolveType.spec.ts
Adds the test "TSMappedType with indexed access" (appears inserted twice) asserting defineProps<{ placement?: Placement }>() resolves placement to String and Object.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Dev as SFC Type Annotations
  participant SFC as compiler-sfc
  participant RT as inferRuntimeType
  participant MT as TSMappedType Branch

  Dev->>SFC: defineProps<{ placement?: Placement }>()
  SFC->>RT: inferRuntimeType(node, typeParameters)
  alt node is TSMappedType
    RT->>MT: validate mapped pattern (K in keyof T => T[K])
    alt pattern & refs match
      MT->>RT: retrieve target type from typeParameters
      RT-->>SFC: inferred runtime type (from target)
    else pattern mismatch
      MT-->>RT: UNKNOWN_TYPE
      RT-->>SFC: UNKNOWN_TYPE
    end
  else other node kinds
    RT-->>SFC: existing inference paths
  end
  SFC-->>Dev: prop runtime type(s) for validation
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested labels

regression

Poem

I nibbled through mapped keys tonight,
Chased K and T by lantern light.
I found the index, traced its art,
Props now know their runtime part.
Hoppity tests — a joyful bite! 🐇

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "fix(compiler-sfc): enhance inferRuntimeType to support TSMappedType with indexed access" is concise and accurately summarizes the primary change in the patch: adding support in inferRuntimeType for TSMappedType patterns that use indexed access. It is specific to the affected subsystem (compiler-sfc) and omits noisy or irrelevant information.
Linked Issues Check ✅ Passed The change implements handling for the restricted mapped-type pattern {[K in keyof T]: T[K]} inside inferRuntimeType and adds tests exercising TSMappedType with indexed access, which directly targets the regression described in issue #13847 (prop default-value type checking for imported literal unions). This appears to restore the intended resolution path by resolving the referenced type from typeParameters and therefore meets the primary coding objective, though the implementation is deliberately narrow in scope and the test was accidentally duplicated in the test file.
Out of Scope Changes Check ✅ Passed All code changes are limited to packages/compiler-sfc (resolveType implementation and related tests) and directly relate to the inferRuntimeType enhancement required by the linked issue; there are no modifications to public APIs or unrelated packages. The only anomalous item is the duplicated test insertion in the spec file, which looks accidental but is still within the testing scope rather than an unrelated change.
✨ 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/fix/TSMappedType

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8ec49bb and c7121d2.

📒 Files selected for processing (1)
  • packages/compiler-sfc/src/script/resolveType.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/compiler-sfc/src/script/resolveType.ts

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

github-actions bot commented Sep 4, 2025

Size Report

Bundles

File Size Gzip Brotli
runtime-dom.global.prod.js 101 kB 38.5 kB 34.6 kB
vue.global.prod.js 159 kB 58.6 kB 52.2 kB

Usages

Name Size Gzip Brotli
createApp (CAPI only) 46.6 kB 18.2 kB 16.7 kB
createApp 54.6 kB 21.3 kB 19.5 kB
createSSRApp 58.9 kB 23 kB 21 kB
defineCustomElement 59.6 kB 22.8 kB 20.9 kB
overall 68.8 kB 26.4 kB 24.1 kB

Copy link

pkg-pr-new bot commented Sep 4, 2025

Open in StackBlitz

@vue/compiler-core

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

@vue/compiler-dom

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

@vue/compiler-sfc

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

@vue/compiler-ssr

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

@vue/reactivity

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

@vue/runtime-core

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

@vue/runtime-dom

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

@vue/server-renderer

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

@vue/shared

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

vue

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

@vue/compat

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

commit: af6f254

@edison1105 edison1105 marked this pull request as ready for review September 4, 2025 14:55
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 (3)
packages/compiler-sfc/__tests__/compileScript/resolveType.spec.ts (1)

734-748: Good regression coverage; add an import-based case mirroring #13847.

This validates the mapped-identity path via inline types. To guard against future regressions that involve imported aliases (the original failure mode), add a companion test that defines Prettify/Placement in an external file and imports it in the SFC before calling defineProps. This ensures cross-file resolution and caching paths participate in the fix.

I can draft the import-based test using the local files map used elsewhere in this suite.

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

1819-1819: Preserve isKeyOf when delegating to the target type.

If TSMappedType appears under a keyof operator, the current call drops the isKeyOf context, which can change the inferred runtime type. Propagate the flag.

-                return inferRuntimeType(ctx, targetType, scope)
+                return inferRuntimeType(ctx, targetType, scope, isKeyOf)

1784-1824: Optional: accept trivially-equivalent variants.

If you want slightly broader compatibility without losing the “strict pattern” intent, consider also accepting readonly/optional modifiers that don’t affect value inference (e.g., { readonly [K in keyof T]-?: T[K] }). This would remain conservative, but reduce surprises from common utility aliases.

I can follow up with a tiny helper (e.g., isIdentityMappedType(node)) and a couple of focused tests if desired.

📜 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 75220c7 and 99bbec5.

📒 Files selected for processing (2)
  • packages/compiler-sfc/__tests__/compileScript/resolveType.spec.ts (1 hunks)
  • packages/compiler-sfc/src/script/resolveType.ts (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/compiler-sfc/src/script/resolveType.ts (1)
packages/compiler-sfc/src/script/utils.ts (1)
  • UNKNOWN_TYPE (13-13)
🔇 Additional comments (1)
packages/compiler-sfc/src/script/resolveType.ts (1)

1784-1824: Mapped-identity recognition LGTM for { [K in keyof T]: T[K] }.

The structural checks are tight and limited to the declared scope; this should correctly restore string inference for Prettify<T> without widening unrelated mapped types.

@edison1105 edison1105 marked this pull request as draft September 5, 2025 00:01
@edison1105 edison1105 marked this pull request as ready for review September 18, 2025 14:16
@edison1105 edison1105 added ready to merge The PR is ready to be merged. scope: sfc 🔨 p3-minor-bug Priority 3: this fixes a bug, but is an edge case that only affects very specific usage. labels Sep 18, 2025
@edison1105
Copy link
Member Author

/ecosystem-ci run

@vue-bot
Copy link
Contributor

vue-bot commented Sep 24, 2025

📝 Ran ecosystem CI: Open

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

@edison1105 edison1105 merged commit e388f1a into main Sep 24, 2025
16 checks passed
@edison1105 edison1105 deleted the edison/fix/TSMappedType branch September 24, 2025 09:29
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.

Regression in prop type checking of imported type literal unions
2 participants