Skip to content
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.

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

Corrected and improved docs on type definitions for Custom Elements. #3073

Merged
merged 2 commits into from
Nov 19, 2024

Conversation

trusktr
Copy link
Contributor

@trusktr trusktr commented Oct 22, 2024

The doc was incorrect for two reasons:

  • For Vue-based custom elements, the types passed into GlobalComponents need to be the Vue component types, not the custom element types, or the custom elements will not be type checked in Vue templates.
  • The name of the elements need to have at least two words and a hyphen, otherwise with single words the custom elements will not work at all.

A few questions:

Having examples/helpers/tests could prevent the docs from being incorrect.

There's a conversation in #typescript in Vue's Discord where we chatted about and worked on this.

The conversation includes the idea of updating vue-tsc so that Custom Element type definitions can instead be added to IntrinsicElementAttributes, which I tried at first but it currently does not work. Only built-in elements on IntrinsicElementAttributes work, so I resorted to using GlobalComponents with the $props and $emit hacks which results in element ref types exposing non-existent $props and $emit properties to the user.

So one more question is:

  • Do we merge this update now so it is correct with the current state of vue-tsc? Or do we wait until vue-tsc can be updated so that hyphenated element names can successully be added to IntrinsicElementAttributes, and update these docs to show the proper way of defining element types with IntrinsicElementAttributes? cc @KazariEX @johnsoncodehk

Copy link

netlify bot commented Oct 22, 2024

Deploy Preview for vuejs ready!

Name Link
🔨 Latest commit 9da839d
🔍 Latest deploy log https://app.netlify.com/sites/vuejs/deploys/67368327140d8b00086655bc
😎 Deploy Preview https://deploy-preview-3073--vuejs.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 configuration.

@trusktr trusktr marked this pull request as draft October 22, 2024 09:25
@trusktr

This comment was marked as outdated.

@trusktr trusktr marked this pull request as ready for review October 27, 2024 03:58
@trusktr
Copy link
Contributor Author

trusktr commented Oct 27, 2024

Alright, updated and ready for initial review.

@trusktr trusktr changed the title Correct and improve docs on type definitions for Custom Elements. Corrected and improved docs on type definitions for Custom Elements. Oct 27, 2024
@trusktr trusktr force-pushed the patch-1 branch 3 times, most recently from acac193 to ea64547 Compare October 27, 2024 05:56
…lements.

The doc was incorrect for two reasons:

- For Vue-based custom elements, the types passed into `GlobalComponents` need to be the Vue component types, not the custom element types, or the custom elements will not be type checked in Vue templates.
- The name of the elements need to have at least two words and a hyphen, otherwise with single words the custom elements will not work at all.
Copy link
Member

@NataliaTepluhina NataliaTepluhina left a comment

Choose a reason for hiding this comment

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

@trusktr thank you for working on this, it's an extremely valuable and detailed addition to the documentation!

@NataliaTepluhina NataliaTepluhina merged commit 1cea431 into vuejs:main Nov 19, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants