Uh oh!
There was an error while loading. Please reload this page.
fix: add defineSlots to all components for proper slot type inference - #684
Conversation
All 19 components with slots were missing defineSlots, meaning slot props were typed as any. This adds proper type definitions so IDEs provide autocomplete and type checking for slot bindings.
The latest updates on your projects. Learn more about Vercel for GitHub.
|
commit: |
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughThis pull request adds explicit Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/script/src/runtime/components/ScriptVimeoPlayer.vue (1)
70-77: Placeholder slot type is incomplete — missingv-bind="payload"props.Line 301 spreads
payload(the oEmbed response) in addition to:placeholder:<slotv-if="!ready"v-bind="payload" :placeholder="placeholder" name="placeholder">The slot type only declares
placeholder, so consumers won't get autocomplete for other oEmbed properties likethumbnail_url,title,author_name, etc.🔧 Suggested fix to include payload properties
You could either type the full oEmbed response or at minimum document that additional props are available:
defineSlots<{ default?: () => any - placeholder?: (props: { placeholder: string | undefined }) => any+ placeholder?: (props: { placeholder: string | undefined } & Partial<VimeoOEmbedResponse>) => any loading?: () => any awaitingLoad?: () => any error?: () => any }>()Where
VimeoOEmbedResponsecaptures the relevant oEmbed fields, or use a looser type likeRecord<string, unknown>if the exact shape isn't worth maintaining.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/script/src/runtime/components/ScriptVimeoPlayer.vue` around lines 70 - 77, The placeholder slot typing is too narrow: update the defineSlots generic so the placeholder slot includes the full oEmbed payload (or a loose map) in addition to placeholder; specifically modify the defineSlots declaration that currently types placeholder as (props: { placeholder: string | undefined }) => any to accept props: { placeholder?: string | undefined } & VimeoOEmbedResponse (or Record<string, unknown>) so consumers get properties from payload (refer to defineSlots, placeholder slot, and the payload variable used in the template).
🤖 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/script/src/runtime/components/ScriptVimeoPlayer.vue`:
- Around line 70-77: The placeholder slot typing is too narrow: update the
defineSlots generic so the placeholder slot includes the full oEmbed payload (or
a loose map) in addition to placeholder; specifically modify the defineSlots
declaration that currently types placeholder as (props: { placeholder: string |
undefined }) => any to accept props: { placeholder?: string | undefined } &
VimeoOEmbedResponse (or Record<string, unknown>) so consumers get properties
from payload (refer to defineSlots, placeholder slot, and the payload variable
used in the template).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 786151e4-e31f-4821-9f1e-aa09fad50cde
📒 Files selected for processing (19)
packages/script/src/runtime/components/GoogleMaps/ScriptGoogleMaps.vuepackages/script/src/runtime/components/GoogleMaps/ScriptGoogleMapsInfoWindow.vuepackages/script/src/runtime/components/GoogleMaps/ScriptGoogleMapsMarker.vuepackages/script/src/runtime/components/GoogleMaps/ScriptGoogleMapsMarkerClusterer.vuepackages/script/src/runtime/components/GoogleMaps/ScriptGoogleMapsOverlayView.vuepackages/script/src/runtime/components/GoogleMaps/ScriptGoogleMapsStaticMap.vuepackages/script/src/runtime/components/ScriptBlueskyEmbed.vuepackages/script/src/runtime/components/ScriptCarbonAds.vuepackages/script/src/runtime/components/ScriptCrisp.vuepackages/script/src/runtime/components/ScriptGoogleAdsense.vuepackages/script/src/runtime/components/ScriptInstagramEmbed.vuepackages/script/src/runtime/components/ScriptIntercom.vuepackages/script/src/runtime/components/ScriptLemonSqueezy.vuepackages/script/src/runtime/components/ScriptPayPalButtons.vuepackages/script/src/runtime/components/ScriptPayPalMessages.vuepackages/script/src/runtime/components/ScriptStripePricingTable.vuepackages/script/src/runtime/components/ScriptVimeoPlayer.vuepackages/script/src/runtime/components/ScriptXEmbed.vuepackages/script/src/runtime/components/ScriptYouTubePlayer.vue
There was a problem hiding this comment.
Copilot reviewed 19 out of 19 changed files in this pull request and generated no comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Adds slot extraction to generate-registry-types.ts, producing
{Component}Slots declarations and schema fields. Updates
ScriptTypes.vue to render a Slots tab alongside Props and Events.
🔗 Linked issue
N/A (user-reported: clusterer renderer slot typed as
any)❓ Type of change
📚 Description
All 19 components with slots were missing
defineSlots, so slot props were typed asanyin consuming templates. This adds properdefineSlotstype definitions to every component, giving IDEs autocomplete and type checking for slot bindings like#renderer="{ cluster, stats, map }".Components with typed slot props:
ScriptGoogleMapsMarkerClusterer(renderer),ScriptXEmbed,ScriptBlueskyEmbed,ScriptInstagramEmbed(default + error),ScriptGoogleMapsStaticMap(default),ScriptCrisp,ScriptIntercom(default),ScriptPayPalButtons,ScriptPayPalMessages(default),ScriptYouTubePlayer,ScriptVimeoPlayer(placeholder), plus 7 components with no-prop slots.