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

Fixes #3609 - Update Calendar typings to be precise #4972

Merged
merged 1 commit into from Mar 25, 2024

Conversation

Magiczne
Copy link
Contributor

As the calendar only allows to select two values, which one of them can be null (first range item selected, second not) the types should be precise and declared as tuple, and not allow arrays of unspecified length.

Fixes #3609

Copy link

vercel bot commented Dec 16, 2023

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

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
primevue ⬜️ Ignored (Inspect) Visit Preview Jan 20, 2024 9:01am

@melloware
Copy link
Member

Not sure if this helps but in PrimeReact we made the TS even smarter by knowing whether you were in single mode, range mode, or multiple mode.

interface CalendarPropsSingle extends CalendarBaseProps {
    /**
     * Specifies the selection mode either "single", "range", or "multiple";
     * @defaultValue single
     */
    selectionMode?: 'single' | undefined;
    /**
     * Value of the component.
     * @defaultValue null
     */
    value?: Nullable<Date>;
    /**
     * Callback to invoke when value changes.
     * @param { FormEvent<Date>} event - Custom change event
     */
    onChange?(event: FormEvent<Date>): void;
}

interface CalendarPropsRange extends CalendarBaseProps {
    /**
     * Specifies the selection mode either "single", "range", or "multiple";
     * @defaultValue single
     */
    selectionMode: 'range';
    /**
     * Value of the component.
     * @defaultValue null
     */
    value?: Nullable<(Date | null)[]>;
    /**
     * Callback to invoke when value changes.
     * @param { FormEvent<(Date | null)[]>} event - Custom change event
     */
    onChange?(event: FormEvent<(Date | null)[]>): void;
}

interface CalendarPropsMultiple extends CalendarBaseProps {
    /**
     * Specifies the selection mode either "single", "range", or "multiple";
     * @defaultValue single
     */
    selectionMode: 'multiple';
    /**
     * Value of the component.
     * @defaultValue null
     */
    value?: Nullable<Date[]>;
    /**
     * Callback to invoke when value changes.
     * @param {FormEvent<Date[]>} event - Custom change event
     */
    onChange?(event: FormEvent<Date[]>): void;
}

export type CalendarProps = CalendarPropsSingle | CalendarPropsRange | CalendarPropsMultiple;

@mertsincan
Copy link
Member

Hi @Magiczne,

There are some conflicts in it. Could you please update it and give an update to Melloware's comment? WDYT?

@mertsincan mertsincan added Resolution: Help Wanted Issue or pull request requires extra help and feedback Resolution: Needs Revision The pull request can't be merged. Conflicts need to be corrected or documentation and code updated. labels Jan 16, 2024
@Magiczne
Copy link
Contributor Author

@mertsincan will try but probably at the end of the week

@Magiczne Magiczne force-pushed the calendar-types-update branch 2 times, most recently from c84f936 to 21e8c83 Compare January 20, 2024 09:01
@Magiczne
Copy link
Contributor Author

Magiczne commented Jan 20, 2024

@mertsincan updated - since in vue on type level the emits are not aware of props the emits will have to stay as normal interface (updated type value), and only props will be a discriminated union, wchich works perfectly fine when I tested this types locally.

Only keep in mind, that this could be a breaking change for users using TypeScript.

@tugcekucukoglu tugcekucukoglu merged commit b9774c4 into primefaces:master Mar 25, 2024
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Resolution: Help Wanted Issue or pull request requires extra help and feedback Resolution: Needs Revision The pull request can't be merged. Conflicts need to be corrected or documentation and code updated.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Calendar: Invalid typings for modelValue.
4 participants