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(createReusableTemplate): camelize props #3253
fix(createReusableTemplate): camelize props #3253
Conversation
Weird, I wonder why Vue does not handle them. I'd like to see if there is a way to solve this better without needing to port code from Vue |
I was wondering too. Seems like Vue transforms slot attribute cases only at compile-time, see
Based on that, slot-props are never accessible in hyphenated format. So I think we could only camelize the attributes. |
Co-authored-by: Kasper Seweryn <github@wvffle.net>
In my opinion, the methods from Vue should be used instead of porting them. If anything changes in Vue, VueUse wouldn't need to be updated unless it was a breaking change. Also, if somebody wanted to use Also If |
Fixes #3261 |
In practice, it would be hard to push this to happen, as it would require alignment across all Vue versions (2.6, 2.7, 3+), while Vue has been cautious about adding new public APIs. If you want, you can try to push it in Vue core. But note that even if it gets shipped, it will only be available in the next version, meaning we still need to port to support the older version. In that sense, I'd think porting is the fast solution that is approachable. Maybe we could remove the cache, as they are probably not very perf-sensitive in VueUse. |
You're right, I've missed that in my reasoning. Vue 2 does not export |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about using Vue's camelize
if available and if not, then port?
@@ -0,0 +1,18 @@ | |||
// copied from vue: https://github.com/vuejs/core/blob/3be4e3cbe34b394096210897c1be8deeb6d748d8/packages/shared/src/general.ts#L90-L112 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// copied from vue: https://github.com/vuejs/core/blob/3be4e3cbe34b394096210897c1be8deeb6d748d8/packages/shared/src/general.ts#L90-L112 | |
import { camelize as camelize_ } from 'vue-demi' | |
// copied from vue: https://github.com/vuejs/core/blob/3be4e3cbe34b394096210897c1be8deeb6d748d8/packages/shared/src/general.ts#L90-L112 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't this result in an error if someone is on a Vue version that doesn't export it?
I don't think it's worth it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
vue-demi
returned an undefined
on vue 2 when I was testing it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately not. That would require vue-demi
to include such exports but it only re-exports everything from Vue, meaning that you basically try to import something non-existent from Vue.
I created an example for you https://stackblitz.com/edit/vitejs-vite-eikher?file=src%2FApp.vue
) | ||
|
||
const camelizeRE = /-(\w)/g | ||
export const camelize = cacheStringFunction((str: string): string => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
export const camelize = cacheStringFunction((str: string): string => { | |
export const camelize = camelize_ ?? cacheStringFunction((str: string): string => { |
Before submitting the PR, please make sure you do the following
fixes #123
).Description
This PR allows to mix attribute cases, similar to native Vue behavior.
Currently, attributes passed to reusableTemplates are passed to v-slot, as is. This leads to issues when trying to mix cases on define and reuse.
People might end up writing the following, which doesn't work
Additional context
Technically vue exports
camelize
which could be used here, but for some reasonhyphenate
is not.So I decided to copy these functions from vue for now.
🤖 Generated by Copilot at 453731b
Added prop name flexibility and improved code quality for
createReusableTemplate
. This function creates a Vue component from a template string and a set of props.🤖 Generated by Copilot at 453731b
hyphenate
andcamelize
functions from vue core package to convert attribute keys between cases (link)keysToCamelKebabCase
function to createattrs
object with both camelCase and kebab-case keys for reusable template component props (link, link)DefineTemplateComponent
,ReuseTemplateComponent
,ReusableTemplatePair
, andcreateReusableTemplate
to fit in one line for readability and consistency (link, link, link)