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

fix: Typed component props #6477

Closed
wants to merge 2 commits into from
Closed

fix: Typed component props #6477

wants to merge 2 commits into from

Conversation

Eirmas
Copy link

@Eirmas Eirmas commented Sep 25, 2024

For issue #5903

The bug

PrimeVue implements their own DefineComponent function type instead of using the one provided by vue.
This is used to simplify the type arguments and allow for only providing the props, slots, emits and sometimes methods.

The issue is that this type function gives empty object to some type arguments related to props, thus overriding the value.
This makes the code intellisense not pick up on the components props correctly.

The fix

This PR replaces the empty objects with the same type functions used by vue. It's more or less a mirror of what they have done.
I also ran build, format and lint:fix before committing my changes, which is why there are changes to other files.

Testing

I tested my changes locally, and I now get the correct props, both the ones from the Vue component, and the public props that Vue normally displays
image

Copy link

vercel bot commented Sep 25, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

2 Skipped Deployments
Name Status Preview Comments Updated (UTC)
primevue ⬜️ Ignored (Inspect) Visit Preview Sep 25, 2024 6:41pm
primevue-v3 ⬜️ Ignored (Inspect) Visit Preview Sep 25, 2024 6:41pm

Comment on lines +325 to +330
// IftaLabel
export * from './iftalabel/IftaLabel.vue';
export { default as IftaLabel } from './iftalabel/IftaLabel.vue';
export * from './iftalabel/style/IftaLabelStyle.js';
export { default as IftaLabelStyle } from './iftalabel/style/IftaLabelStyle.js';

Copy link
Author

Choose a reason for hiding this comment

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

This came after running pnpm build

@@ -61,4 +61,4 @@
"engines": {
"node": ">=12.11.0"
}
}
}
Copy link
Author

Choose a reason for hiding this comment

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

Due to pnpm format or pnpm lint:fix

@Eirmas
Copy link
Author

Eirmas commented Sep 25, 2024

I now realize that all type arguments, Props, Slots and Emits are broken.

Props are overridden, slots are lost due to an intersection S & SlotsType, and emits are typed correctly due to the use of EmitFn, a function for Vue which should've been used with $emit: EmitFn<E> in PrimeVue's use-case.

I think the fix from PR #6088 is the best. I would suggest renaming the GlobalComponentConstructor to something different though.

I'm closing this PR in favor of #6088

@Eirmas Eirmas closed this Sep 25, 2024
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.

1 participant