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

Wrong types issue with aria-label and aria-labelledby Props #4511

Closed
kefniark opened this issue Sep 27, 2023 · 4 comments
Closed

Wrong types issue with aria-label and aria-labelledby Props #4511

kefniark opened this issue Sep 27, 2023 · 4 comments
Assignees
Labels
Type: Bug Issue contains a bug related to a specific component. Something about the component is not working
Milestone

Comments

@kefniark
Copy link

kefniark commented Sep 27, 2023

Describe the bug

I was updating an existing project to recent version vitejs, vue, typescript.
A big upgrade party, where I got almost everything working except one weird last typing issue.

I started to get a some errors caused by two properties aria-label & aria-labelledby in primevue components (most of them):

[vite] Internal server error: Transform failed with 1 error:
file.vue:65:8: ERROR: Expected "}" but found "-"
Expected "}" but found "-"
63 | emptyMessage: { type: null, required: false },
64 | tabindex: { type: null, required: false },
65 | aria-label: { type: null, required: false },
| ^
66 | aria-labelledby: { type: null, required: false },
67 | pt: { type: null, required: false },

I don't know were the issue comes from exactly yet, but there is definitely some mismatch in these types between kebab-case and camelCase. I guess typescript or vue got more strict over time and are now able to complain about some primevue issues which were ignored until now.

And the place the error is pointing at feels indeed wrong, these two properties should probably be camelcase like everything else in these files. They seem to have been copied from somewhere else and go against the code convention used by every other Props.

These properties seem to have been added in the past few month for accessibility purpose:
https://github.com/primefaces/primevue/pull/3354/files

Example

Screenshot

image

Reproducer

https://github.com/kefniark/primevue-casing-issue

PrimeVue version

3.35

Vue version

3.x

Language

TypeScript

Build / Runtime

Vite

Browser(s)

No response

Steps to reproduce the behavior

Follow the Readme

Expected behavior

These two properties aria-label & aria-labelledby
Should be camelCase (in both the .vue and .d.ts files) and don't need extra quotes

    ...
    /**
     * Establishes relationships between the component and label(s) where its value should be one or more element IDs.
     */
    ariaLabelledby?: string | undefined;
    /**
     * Used to define a string that labels the element.
     */
    ariaLabel?: string | undefined;
    ...
@kefniark kefniark added the Status: Needs Triage Issue will be reviewed by Core Team and a relevant label will be added as soon as possible label Sep 27, 2023
@kefniark kefniark changed the title Typing issue with aria-label and aria-labelledby Props Casing and Typing issue with aria-label and aria-labelledby Props Sep 27, 2023
@kefniark
Copy link
Author

kefniark commented Oct 10, 2023

Dirty workaround to get primevue to work with recent vue@3.3/typescript@5.2

Ignoring primevue problematic properties and reemit them with valid name

// See: https://github.com/primefaces/primevue/issues/4511
export interface Props extends Omit<DropdownProps, 'aria-label' | 'aria-labelledby'> {
  /**
   * Used to define a string that labels the element.
   */
  ariaLabel?: string | undefined
  /**
   * Establishes relationships between the component and label(s) where its value should be one or more element IDs.
   */
  ariaLabelledby?: string | undefined
}

const props = defineProps<Props>()

Sadly, vue-compiler needs props types to be statically typed to infer props, so we can't use a generic to automate that

@kefniark kefniark changed the title Casing and Typing issue with aria-label and aria-labelledby Props Wrong types issue with aria-label and aria-labelledby Props Oct 10, 2023
@stefandrazicstefan
Copy link

I'm getting the same issue.

const props = defineProps<Partial<ExposedNativeAttrs>>();

type ExposedNativeAttrs = {
  [K in "aria-label"]: InputHTMLAttributes[K];
};

const nativeAttributes = computed(
  (): ExposedNativeAttrs => ({
    "aria-label": props["aria-label"],
  }),
);

I found this answer online, but it fails in build time.

@dukesteen
Copy link

Also getting this issue here. This is only when using the InputXProps directly, if you add it into a property of the props type it will work.

export type NumberInputProps = {
    useCurrency?: boolean;
    inputProps?: InputNumberProps;
}

@cagataycivici cagataycivici added this to the 3.42.0 milestone Nov 24, 2023
@tugcekucukoglu tugcekucukoglu added Type: Bug Issue contains a bug related to a specific component. Something about the component is not working and removed Status: Needs Triage Issue will be reviewed by Core Team and a relevant label will be added as soon as possible labels Nov 24, 2023
tugcekucukoglu added a commit that referenced this issue Nov 24, 2023
@cagataycivici
Copy link
Member

Thanks Tuğçe

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Issue contains a bug related to a specific component. Something about the component is not working
Projects
None yet
Development

No branches or pull requests

5 participants