[typescript-migration] calendar.jsx and index.jsx

[yuki0410-dev]

name: Migrate calendar.jsx and index.jsx

Description

Linked issue: #4700

Changes calendar.jsx and index.jsx was migrated to ts

Contribution checklist

• [x] I have followed the contributing guidelines.
• [x] I have added sufficient test coverage for my changes.
• [x] I have formatted my code with Prettier and checked for linting issues with ESLint for code readability.

[martijnrusschen]

@yuki0410-dev anything needed to complete this?

[yuki0410-dev]

@martijnrusschen I am working with index.jsx. There is some rework to be done for consistency, etc. I will release the draft after index.js is complete.

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 98.36%. Comparing base (cf820dd) to head (6fe84be). Report is 4 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4766      +/-   ##
==========================================
+ Coverage   96.35%   98.36%   +2.01%     
==========================================
  Files           8        6       -2     
  Lines         959       61     -898     
  Branches      435       19     -416     
==========================================
- Hits          924       60     -864     
+ Misses         35        1      -34     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[mirus-ua]

I noticed a lot of changes that replaced randomProp?: type with randomProp?: type | null. IMO, it is better to replace null with undefined in place of usage instead of making all the types wider.

[yuki0410-dev]

@mirus-ua I am glad for your advice. I agree with unifying the internal types to undefined or null. However, since the original selected and other props were of type Date | null | undefined, we have modified the related props to Date | null | undefined to maintain compatibility. If you have any particular concerns, it would be appreciated if you could comment on them in the File changes tab.

[yuki0410-dev]

@mirus-ua I am struggling with the part of index.ts where the onChange type changes due to selectsRange,selectsMultiple Props. Any good advice would be appreciated. In my current code, I am using conditional type with reference to @types/react-datepicker, but the type inference does not work well at the point where onChage is called. Also, I defined some (conditional type) props using type = and tried to inherit them in the interface, but I get a ts(2312) error. Otherwise, if I declare Props with type instead of interface, the inference works correctly, but it is not consistent because other Props are declared with interface.

[mirus-ua]

@mirus-ua I am struggling with the part of index.ts where the onChange type changes due to selectsRange,selectsMultiple Props. Any good advice would be appreciated. In my current code, I am using conditional type with reference to @types/react-datepicker, but the type inference does not work well at the point where onChage is called. Also, I defined some (conditional type) props using type = and tried to inherit them in the interface, but I get a ts(2312) error. Otherwise, if I declare Props with type instead of interface, the inference works correctly, but it is not consistent because other Props are declared with interface.

Conditional types work well only with type notation, so if the rest is OK and the only concern is consistency, so I suggest migrating to types instead of interfaces, I don't think that someone will be against it (btw I pick type over interface almost always)

[yuki0410-dev]

@mirus-ua Thanks for the advice. I too use type when writing code in my daily life. However, when creating libraries, it is more convenient to be able to extend the type on the user side (as I have done in this PR by extending react-onclickoutside), so I was wondering if I could use interface. For now, I will try to implement it with type, and if there are requests for interface, I will consider it again at a later time.

[martijnrusschen]

@mirus-ua can you review this PR as well?

[yuki0410-dev]

@mirus-ua Thanks for the review.

I don't understand the reason why we replace HTMLDivElemnt with HTMLElement that quite generic and may cause problems for the extension in the future

Changed the HTMLElement type to a concrete type whenever possible.

VoidFunction as props looks like it should be replaced with proper typed function with arguments

We do not think it is necessary to add arguments at this time that are not being used at this time. (I don't think this is the scope of the TS conversion.)

[yuki0410-dev]

prop: Type | null | undefined can be simplified to prop?: Type | null

I use args: Type | undefined and args?: Type explicitly. The former always requires an argument at call time, whereas the latter does not raise an error if you forget to write the argument. Since Optional is not a mere shorthand for union type with undefined, we believe that it should be used with the best of intentions.

Also, as for the prop: Type | null | undefined type, @types/react-datepickr used Date | null | undefined for selected props, etc., so we do so to maintain compatibility. And the part where the value is passed is of a similar type.

[mirus-ua]

Also, as for the prop: Type | null | undefined type, @types/react-datepickr used Date | null | undefined for selected props, etc., so we do so to maintain compatibility. And the part where the value is passed is of a similar type.

But the optional operator implicitly adds undefined to the union, or I get your point wrong

We do not think it is necessary to add arguments at this time that are not being used at this time. (I don't think this is the scope of the TS conversion.)

If you say that all these functions don't receive any parameters then OK

[martijnrusschen]

@yuki0410-dev @mirus-ua is this PR ready to merge?

[mirus-ua]

@yuki0410-dev @mirus-ua is this PR ready to merge?

I don't see any major issues. Some of the unsolved problems are addressed here https://github.com/Hacker0x01/react-datepicker/pull/4757