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

Fix(Calendar): Improve type declaration using generic types with default parameters (Fixes #7555) #7556

Merged

Conversation

iamkyrylo
Copy link
Contributor

@iamkyrylo iamkyrylo commented Jan 3, 2025

Related Issue

Closes #7555
Fix #6583

Summary

This PR implements the proposed improvement by replacing type overloading with generic types for CalendarProps. It addresses the problem with dynamically managing selectionMode as discussed in the issue.

Screenshots

I've checked the solution by creating a small wrapper inside the project. Here is a screenshot where you can see that TypeScript correctly defines the onChange event type based on the passed selectionMode.

Default (single):
Screenshot 2025-01-03 at 12 30 31

Multiple:
Screenshot 2025-01-03 at 12 30 36

Copy link

vercel bot commented Jan 3, 2025

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

2 Skipped Deployments
Name Status Preview Updated (UTC)
primereact ⬜️ Ignored (Inspect) Visit Preview Jan 3, 2025 10:47am
primereact-v9 ⬜️ Ignored (Inspect) Visit Preview Jan 3, 2025 10:47am

…ault params to dynamically manage the `selectionMode` (fixes primefaces#7555)
@iamkyrylo iamkyrylo force-pushed the fix/calendar-props-type-definition branch from ff10d95 to db97b5f Compare January 3, 2025 10:47
@iamkyrylo iamkyrylo changed the title Fix(Calendart): Improve type declaration using generic types with default parameters (Fixes #7555) Fix(Calendar): Improve type declaration using generic types with default parameters (Fixes #7555) Jan 3, 2025
@melloware
Copy link
Member

Interesting. Does this break backwards compatibility?

@melloware melloware added the Typescript Issue or pull request is *only* related to TypeScript definition label Jan 3, 2025
@iamkyrylo
Copy link
Contributor Author

Interesting. Does this break backwards compatibility?

No, it shouldn't break anything. Previously, only CalendarProps were exposed, so users won't need to update their code, I believe. But it depends on how they applied it 🤷‍♂️

TS can still infer the onChange event and value types automatically based on the selectionMode value. This means you don't need to explicitly pass TMode type, like Calendar<TMode>. The key difference is that TS now directly uses the provided value instead of analyzing multiple type overloads.

@melloware melloware merged commit 4fad1ca into primefaces:master Jan 3, 2025
5 checks passed
@melloware
Copy link
Member

Thanks!

@melloware
Copy link
Member

This also fixes #6583

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Typescript Issue or pull request is *only* related to TypeScript definition
Projects
None yet
2 participants