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

Kebap-case is not supported in some components like DataTable #1263

Closed
tk1 opened this issue May 17, 2021 · 21 comments
Closed

Kebap-case is not supported in some components like DataTable #1263

tk1 opened this issue May 17, 2021 · 21 comments
Assignees
Labels
Type: Enhancement Issue contains an enhancement related to a specific component. Additional functionality has been add
Milestone

Comments

@tk1
Copy link

tk1 commented May 17, 2021

I'm submitting a ... (check one with "x")

[x] bug report => Search github for a similar issue or PR before submitting
[ ] feature request => Please check if request is not on the roadmap already https://github.com/primefaces/primevue/wiki/Roadmap
[ ] support request => Please do not submit support request here, instead see http://forum.primefaces.org/viewforum.php?f=110

CodeSandbox Case (Bug Reports)
Take the Checkbox example from https://primefaces.org/primevue/showcase/#/datatable/selection and replace "selectionMode" by "selection-mode".

Current behavior
The checkboxes are missing

Expected behavior
The checkboxes should be visible.

  • Vue version: 3.0.5
    yes

  • PrimeVue version: 3.4.0
    yes

@Anubarak
Copy link

This issue was created several times in the past and is due to a missuse of vue but they still claim it's a vue bug. I had a conversation with the vue team and they also confirmed that vue should not be used the way it is used here... For more info #797 (comment)

@tk1
Copy link
Author

tk1 commented May 18, 2021

This problem will pop up for many users new to PrimeVue: If you use Vetur and the eslint plugin "vue/vue3-recommended", "selectionMode" will be converted by eslint autofix to "selection-mode" and the checkboxes are missing. Many a new user will then leave PrimeVue. So the team should really consider to fix this.

@Anubarak
Copy link

Anubarak commented May 18, 2021

I know, I'll do the same once I finish my current project with it because of those reasons. Unfortunately there is nothing we can do except writing many pull requests for every single component.

But I guess not so many will leave - the mayority will just disable their linter.

@cagataycivici
Copy link
Member

cagataycivici commented May 18, 2021

I believe vue only does the transformation when used with in-dom templates not string templates;

https://v3.vuejs.org/guide/component-props.html#prop-casing-camelcase-vs-kebab-case

kebab-case props are ignored for sure and does not function however not sure if there is anything we can do on our side. By the way reason of camel-case is used on components as a convention as PrimeFaces, PrimeReact and PrimeNG also do it. Also I do not think Vue enforces a convention, docs state there are two options;

https://vuejs.org/v2/guide/components-registration.html

For events, they suggest kebab so we used that for all the events;

https://vuejs.org/v2/guide/components-custom-events.html#Event-Names

If Vue can transform them somehow in string templates or allow an alias option, that'd be great. You know we can't change prop names casing as it'll break all the apps. Open to suggestions.

@cagataycivici cagataycivici added Status: Discussion Issue or pull request needs to be discussed by Core Team Resolution: Help Wanted Issue or pull request requires extra help and feedback labels May 18, 2021
@Anubarak
Copy link

Anubarak commented May 18, 2021

@cagataycivici Thank you very much for your attention. You (PrimeVue) use raw html attributes to check the props. Vue does not yet proceed them at this point.
It was never intended by Vue to check the raw attributes from vNodes. Vue only converts and validates those when a component is created.

The current behavior is

  1. you grab all vNodes via cols = this.$slots.default()
  2. you grab those vNode attributes via cols{index].props[someCamelAttribute]
  3. you will never ever access some-kebab-attribute with your someCamelAttribute here because Vue does not proceed those yet.

Vue creates a Component with a vNode by passing all these attributes into it. The component has then a props config (as we all know) and transforms / validates those vNode props into component props.
By the time when you access those via this.$slots.default()[index].prop[name] there could be anything there...
Solutions:

  1. One method would be at least as a "hotfix" to check at least kebab-case attributes as well.
  2. Another way would be to properly use all those "Placeholder" components instead of just grabbing props from raw vNodes
  3. v-bind all props from your placeholder component to your real component.

(with placeholders I mean things like these https://github.com/primefaces/primevue/blob/master/src/components/column/Column.vue)

Edit: an easy way to see what I mean (for Vue 2 - this is unfortunately removed in Vue 3) is the following, select a component via Dev Tools and just print $vm.$slots
image
You'll see the vNodes
When you print $vm.$children instead you'll see Components
image

VueComponent prop values are parsed, vNode prop values are not parsed. This is no Vue Issue and has nothing to do with naming conventions at all.

@Anubarak
Copy link

Anubarak commented Jun 2, 2021

Just a s a side note: fixing this issue by removing this.$slots.default will also enable HMR. (#1106)
Since those VNodes are not reactive the way you use it. It would make our work easier because we don't need to reload the site again and again and could make use of.... well... Vue and Webpack

I really hope someone has an eye on this.

@Jogai
Copy link

Jogai commented Jul 26, 2021

@dheimoz any progress per #797 (comment)?

For now I added an exception in my lint config:

	"vue/attribute-hyphenation": [
		"error",
		"always",
		{
			ignore: ["selectionMode", "headerClass"]
		}
	]

@dheimoz
Copy link

dheimoz commented Aug 1, 2021

hello @Jogai hope you are doing great. I am a Primve Vue user, just like you. The PrimeTek team will work on the priorities they define.
Cheers.

@Jogai
Copy link

Jogai commented Aug 2, 2021

Oh, you're not a team member? In the other thread it sounded like you were.

Anyway, found another one to ignore: "headerClass" on the column in the datatable

@dheimoz
Copy link

dheimoz commented Aug 5, 2021

@Jogai great! thanks for sharing

@Jogai
Copy link

Jogai commented Aug 14, 2021

Found another: "rowEditor"

@PeterKnight
Copy link

I spent a long time trying to figure out if I had done something wrong after I couldn't get the checkboxes to appear because of this issue. At the very least it would be nice to have a warning in the docs, because it violates user expectations. Thank you @Jogai for posting the eslint config workaround.

@lobodol
Copy link

lobodol commented Sep 24, 2021

Hello there,
Found 3 others:

  • showFilterMatchModes
  • filterMatchModeOptions
  • showFilterOperator

Regards,

@matheuspaulo93
Copy link

Hello guys, i am using primevue version 3 and i need use the double click event from DataTable, but is not working. Can anybody help me? Sorry for my english, i'm using google translate.

@Jogai
Copy link

Jogai commented Sep 25, 2021

@matheuspaulo93 use row-dblclick. But that is not the discussion here.

@cagataycivici
Copy link
Member

We'll work on this again this week for potential improvements.

@matheuspaulo93
Copy link

@matheuspaulo93 use row-dblclick. But that is not the discussion here.

Thanks, it works.

@cagataycivici
Copy link
Member

cagataycivici commented Oct 1, 2021

@Anubarak, I see the issue now, the reason why I had to access vnodes was during migration from Vue 2 to Vue 3 where I could not find a way to access components. #1 would work but it is not ideal to check for kebap-case ourselves. Trying to figure out a way to access child components, hopefully newer versions of Vue 3.x had some ways to do it.

See: https://v3.vuejs.org/guide/migration/children.html

@cagataycivici cagataycivici changed the title Attribute in kebab-case not working Kebap-case is not supported in some components like DataTable Oct 2, 2021
@cagataycivici cagataycivici self-assigned this Oct 2, 2021
@cagataycivici cagataycivici added Type: Enhancement Issue contains an enhancement related to a specific component. Additional functionality has been add and removed Status: Discussion Issue or pull request needs to be discussed by Core Team Resolution: Help Wanted Issue or pull request requires extra help and feedback labels Oct 2, 2021
@cagataycivici
Copy link
Member

This is hopefully resolved now, components with issues were DataTable and TreeTable mainly who need to access their children. Since Vue 3 remove $children with no replacement, we had to use vnodes that caused this as we use camel by default. Just like Vue does, we now do kebab-camel conversion for these components and the issue is resolved. If one day $children is back, we'll migrate to that for sure. Thanks for everyone contributed to this issue. 4th of October is the release for 3.7.3

@cagataycivici cagataycivici added this to the 3.7.3 milestone Oct 2, 2021
@vukoviciv
Copy link

It seems this issue is present for TabPanel component.
I'm using v3.21.0

@Yves852
Copy link

Yves852 commented Jan 30, 2024

Happens for Dropdown on primevue 3.47.2:

<Dropdown showClear /> triggers the warning "Attribute ':showClear' must be hyphenated." and Volar just rewrite it to kebab-case and it's ignored.

Edit: after updating Nuxt to 3.10.0 it resolved my problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Enhancement Issue contains an enhancement related to a specific component. Additional functionality has been add
Projects
None yet
Development

No branches or pull requests

10 participants