Skip to content

fix(types): Register built-in components Teleport and Suspense should work #4955

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(types): Register built-in components Teleport and Suspense should work #4955

wants to merge 1 commit into from

Conversation

yuwu9145
Copy link
Contributor

@yuwu9145 yuwu9145 commented Nov 17, 2021 •
edited
Loading

fix #5833 & #4950 (built-in components like Teleport do not work)

Cause is
image

which results in Teleport & Suspense are not legitimate Component type.

However, as per office doc (usage with built-in components for rendering dynamic components), <component :is="isGroup ? 'Teleport' : 'Suspense'"> ... </component> will require to register built-in components

@posva
Copy link
Member

posva commented Nov 17, 2021

Thanks but this wouldn't work for library custom components like RouterView or RouterLink in vue router. It would also be great to add a d-ts test to check for externally added components

posva
posva reviewed Nov 17, 2021
View reviewed changes
packages/runtime-core/src/componentOptions.ts Outdated
@@ -138,7 +138,7 @@ export interface ComponentOptionsBase<
// Luckily `render()` doesn't need any arguments nor does it care about return
// type.
render?: Function
components?: Record<string, Component>
components?: Record<string, Component | { __isTeleport: true } | { __isSuspense: true } | { __isKeepAlive: true }>
Copy link
Member

Choose a reason for hiding this comment

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

Same problem 😅

Copy link
Contributor Author

@yuwu9145 yuwu9145 Nov 17, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I think components from user's land (including RouterView or RouterLink ) shall serve as Component type, only built-in components Teleport, Suspense and KeepAlive need special treatment

@yuwu9145
Copy link
Contributor Author

yuwu9145 commented Nov 17, 2021 •
edited
Loading

This issue actually has nothing to do with $props, and as per my testing, both RouterView & RouterLink work, as they serve as legitimate Component type. The reason why Teleport breaks type Inference is because it has __isTeleport property which does not exist in Component, same to Suspense.

Edit: RouterView works in my place
image

I will add tests soon

@yuwu9145
Copy link
Contributor Author

d-ts test has been added, without changes in this pull request, it will fail

@yuwu9145 yuwu9145 changed the title Add types for component with $prop safely Allow Teleport, Suspens and KeepAlive in components options Nov 17, 2021
@yuwu9145 yuwu9145 changed the title safely Allow Teleport, Suspens and KeepAlive in components options Safely allow Teleport, Suspens and KeepAlive in components options Nov 17, 2021
@yuwu9145 yuwu9145 changed the title Safely allow Teleport, Suspens and KeepAlive in components options fix(types): Safely allow Teleport, Suspense in components options Nov 23, 2021
@yuwu9145 yuwu9145 changed the title fix(types): Safely allow Teleport, Suspense in components options fix(types): Register built-in components Teleport and Suspense should work Nov 23, 2021
@yuwu9145 yuwu9145 requested a review from posva November 24, 2021 04:16
@yuwu9145 yuwu9145 force-pushed the fix-4950 branch 2 times, most recently from 7e9b02b to 657af7d Compare November 26, 2021 08:50
@netlify
Copy link

netlify bot commented Apr 29, 2022 •
edited
Loading

✅ Deploy Preview for vue-sfc-playground ready!

Name Link
🔨 Latest commit f9fc790
🔍 Latest deploy log https://app.netlify.com/sites/vue-sfc-playground/deploys/626bda943a44460009c56288
😎 Deploy Preview https://deploy-preview-4955--vue-sfc-playground.netlify.app/
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@netlify
Copy link

netlify bot commented Apr 29, 2022 •
edited
Loading

✅ Deploy Preview for vuejs-coverage ready!

Name Link
🔨 Latest commit f9fc790
🔍 Latest deploy log https://app.netlify.com/sites/vuejs-coverage/deploys/626bda941c79ad00082920e7
😎 Deploy Preview https://deploy-preview-4955--vuejs-coverage.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@netlify
Copy link

netlify bot commented Apr 29, 2022 •
edited
Loading

✅ Deploy Preview for vue-next-template-explorer ready!

Name Link
🔨 Latest commit f9fc790
🔍 Latest deploy log https://app.netlify.com/sites/vue-next-template-explorer/deploys/626bda94c00a94000955aeae
😎 Deploy Preview https://deploy-preview-4955--vue-next-template-explorer.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@edison1105 edison1105 added scope: types 🍰 p2-nice-to-have Priority 2: this is not breaking anything but nice to have it addressed. labels Oct 29, 2024
@edison1105
Copy link
Member

Could you please resolve the conflicts?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@posva posva Awaiting requested review from posva

Assignees
No one assigned
Labels
🍰 p2-nice-to-have Priority 2: this is not breaking anything but nice to have it addressed. scope: types wait changes
Projects
Issue / PR Management
Status: Stale
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Register built-in components Teleport and Suspense has type infer error
3 participants
@yuwu9145 @posva @edison1105