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

Component props in kebab-case #797

Closed
pooledge opened this issue Dec 16, 2020 · 15 comments
Closed

Component props in kebab-case #797

pooledge opened this issue Dec 16, 2020 · 15 comments

Comments

@pooledge
Copy link

I know, I am nose-picking, but wouldn't it be just wonderful if component props are in kebab-case like both usual html attributes and vue-convention-like?

Otherwise great framework, by far better than v-tify, thanks!

@nestorrente
Copy link
Contributor

As far as I know, nothing prevents you to use kebab-case in your project, even when using PrimeVue's components. Vue does the transformation for you.

In fact, in my projects, I register PrimeVue's components using kebab-case and the "p-" prefix for its names (in order to difference them from my own components), and then I use kebab-case again when passing props to them.

For example, I register the DataTable component this way:

import DataTable from 'primevue/datatable';

Vue.component('p-data-table', DataTable);

And I use it like this (note the use of data-key property in kebab-case):

<p-data-table
        :value="items"
        data-key="id"
/>

So I think there's no need to change anything inside PrimeVue's code. You can already use kebab-case if you want to :)

@pooledge
Copy link
Author

Hm yeah I know, right!

So, I've specifically tried now the selection-mode and header-style on <Column /> which stopped it from behaiving correctly. Do I miss something?

@cagataycivici
Copy link
Member

This is handled by Vue core not PrimeVue, Vue does the transformation. It is a matter of preference, for all Prime UI libraries we use camelCase on demos for consistency although Vue suggest kebap-case on their style guide.

@Anubarak
Copy link

Anubarak commented Apr 7, 2021

Hm yeah I know, right!

So, I've specifically tried now the selection-mode and header-style on <Column /> which stopped it from behaiving correctly. Do I miss something?

I have the same issue, were you able to fix it somehow?
I set my Linter to convert every prop to kebab-case but it doesn't work with many props in PrimeVue. I know they said it should all be done by Vue but I never had any issues with kebab-case props in any other framework - only with PrimeVue.

@dheimoz
Copy link

dheimoz commented Apr 7, 2021

hello @Anubarak , have you tried registering the components in kebab style? I usually register globally the components in main.js in a "lazy" way:

app.component(
  "progress-spinner",
  defineAsyncComponent(() =>
    import(/* webpackChunkName: "progressspinner" */ "primevue/progressspinner")
  )
);

That way I can use the component in any SFC.

@Anubarak
Copy link

Anubarak commented Apr 7, 2021

@dheimoz Not sure if we are talking about the same thing? It does not matter if I register my components kebab or camel case the props are still ignored.
A working example can be found here https://codesandbox.io/s/upbeat-easley-j8v4b?file=/src/components/DataTableDemo.vue:1323-1355

the first Column has :showFilterMatchModes and it works, the second one has :show-filter-match-modes and it does not hide the filter match mode select. No difference between column and Column

@dheimoz
Copy link

dheimoz commented Apr 7, 2021

@Anubarak ohh I see now what you meant. Sorry, I misread that props are the issue here.

I second what @cagataycivici stated, please take look at this:

vuejs/core#3495

@Anubarak
Copy link

Anubarak commented Apr 8, 2021

@dheimoz Thank you for your answer.
Since both issues are closed it should work shouldn't it?
I made sure I have Version 3.0.11 that includes the fix but the props are still ignored.

I updated my Sandbox to Vue 3.0.11 with one Column :show-filter-menu="false" and the other one :showFilterMenu="false"

Could you tell me what my mistake is? Unfortunately I cannot reproduce the bug without PrimeVue, when I create custom components, everything works.

@Anubarak
Copy link

Anubarak commented Apr 8, 2021

I had a conversation with a member from the Vue team and found out it's a PrimeVue issue because they don't use props as intended and rather just use the data-attributes via vNodes raw props via this.$slots.default()
Guess this issue won't be fixed anytime soon.... what a pity

@dheimoz
Copy link

dheimoz commented Apr 9, 2021

@Anubarak sorry for the delay. I had a busy day. Could you elaborate a little bit more? I am a user like you that loves Prime Vue. If the Vue team member can point out specific documentation I am positive PrimeTek Team will implement it soon enough.

Thanks for taking the time to investigate.

cc @cagataycivici , @tugcekucukoglu

@Anubarak
Copy link

Anubarak commented Apr 9, 2021

@dheimoz Usually you create a component in Vue and pass the props directly like you are used to.
Vue creates an internal vNode of your HTML and fills that vNode with the attributes.
For example

<my-custom-tag :foo-bar="true" ;barFoo="false">

would create a vNode with a props attribute

props: {
    'foo-bar':  true,
    'barFoo': false
}

Those are not yet proceeded by Vue. When Vue creates the component it will convert the names, validate them and injects those fitting into the component. But at this point (vNode state) they are basically just Raw values. Nethertheless some components of PrimeVue are no "real" components. Instead they are "placeholders" to hold the props for their internal logic. For example take a look at their Column component. You won't see any HTML there will you? Thats because they don't actually use this one. They just grab their raw vNodes via this.$slots.default() and check if they have an attribute showFilterMenu but since those are not yet converted by Vue they won't check kebab case props. You can see that here (Fun fact: all those placeholders could actually be empty, at this point prop names are not even validated)

columnProp(col, prop) {
    return col.props ? ((col.type.props[prop].type === Boolean && col.props[prop] === '') ? true : col.props[prop]) : null;
}

while col is the vNode. They grab those via

getChildren() {
    return this.$slots.default ? this.$slots.default() : null;
},

I think they should include a big red alert in their docs for those components

Edit: this would make it working

columnProp(col, prop) {
    let snake = prop.replace(/\B(?:([A-Z])(?=[a-z]))|(?:(?<=[a-z0-9])([A-Z]))/g, '-$1$2').toLowerCase();
    if(col.props && typeof col.props[snake] !== 'undefined'){
        return ((col.type.props[prop].type && col.props[snake] === '') ? true : col.props[snake])
    }

    return col.props ? ((col.type.props[prop].type === Boolean && col.props[prop] === '') ? true : col.props[prop]) : null;
},

@dheimoz
Copy link

dheimoz commented Apr 11, 2021

@Anubarak thank you very much for taking the time for this very detailed explanation. Honestly, until this I could not grasp the problem. Definitely I hope the team can take look and make an informed decision, either updating the docs or improving the way the pros are handled.

Thanks again, sorry for not being as useful. Based on this information, I truly believe this issue is not closed.

@szilardd
Copy link

szilardd commented Aug 27, 2021

Based on the hint from @Anubarak I think this could work, see below. It overrides the columnProp method in all PrimeVue components and enables support for kebab-case properties

/**
 * Checks if requested property exists with kebab-case and provides it as camelCase
 * because PrimeVue properties are camelCase
 * See https://github.com/primefaces/primevue/issues/797
 */
function columnPropWithKebabCase(col, prop, original) {

    var toKebabCase = function(value)  {
        return value.replace(/\B(?:([A-Z])(?=[a-z]))|(?:(?<=[a-z0-9])([A-Z]))/g, '-$1$2').toLowerCase();
    };

    var propWithKebabCase;

    // handle Column components
    if (col && !prop && this.column) {
        var colWithKebabCase = toKebabCase(col);
        propWithKebabCase = this.column.props[colWithKebabCase];

        if (propWithKebabCase) {
            this.column.props[col] = propWithKebabCase;
        }

        return original.call(this, col, prop);
    } else {
        // handle other components
        // see: https://github.com/primefaces/primevue/issues/797#issuecomment-816438158
        propWithKebabCase = toKebabCase(prop);

        if (col.props && typeof col.props[propWithKebabCase] !== 'undefined'){
            return ((col.type.props[prop].type && col.props[propWithKebabCase] === '') ? true : col.props[propWithKebabCase])
        }

        return original.call(this, col, prop);
    }
}

/**
 * Overrides `columnProp` method to enable support for kebab-case properties
 * See https://github.com/primefaces/primevue/issues/797
 */
function overrideColumnPropMethod(obj) {
    for (let key in obj) {
        if (typeof obj[key] === "object") {
            overrideColumnPropMethod(obj[key])
        } else {
            // base case, stop recurring
            if (key === 'columnProp') {
                var original = obj[key];
                
                obj[key] = function (col, prop) {
                    return columnPropWithKebabCase.call(this, col, prop, original);
                };
            }
        }
    }
}

overrideColumnPropMethod(this.primevue);

@JohnCampionJr
Copy link
Contributor

This needs to be fixed; I ran into this using some properties on PrimeVue today.

@cagataycivici
Copy link
Member

Resolved at #1263

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

No branches or pull requests

7 participants