Skip to content

Conversation

Genarito
Copy link

Hi! First of all, thanks for this amazing project!

This pull request introduces performance enhancements to the daysInMonth function as well as a few improvements in the package.json file. The changes are divided into two categories:

Updates in daysInMonth and leap year handling:

Updated the daysInMonth function to use a new MONTH_DAYS constant and the yearIsLeap utility (I've centralized a function to use here and in the isLeapYear plugin) for determining the number of days in February during leap years. This is simpler than calling the startOf method and doing complex stuff.

Also, I've improved the Typescript definition of the daysInMonth function to return the correct possible numbers 28 | 29 | 30 | 31.

Improvements in package.json:

  • Simplified the lint script by removing the relative path to eslint.
  • Updated the test script to use cross-env for better cross-platform compatibility in the package.json file.
  • Updated cross-env to version 5.2.1.

Hope it helps 😄

src/index.js Outdated
10: 30, // November
11: 31 // December
}

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Introducing the MONTH_DAYS constant might increase the bundle size, which may not be an ideal optimization?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @iamkun, if you prefer, I can roll back this change. But with minification, it compresses to roughly:

const MONTH_DAYS={0:31,1:28,2:31,3:30,4:31,5:30,6:31,7:31,8:30,9:31,10:30,11:31}

That’s 80 bytes. After gzip compression, it’s 70 bytes (source).

So in practice, this code would increase the bundle size by 0.07 KB (virtually negligible).

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've noticed this PR introduces some additional code into both utils and index.

A core principle of Day.js is its minimal bundle size, and we're highly sensitive to any changes that might increase it. For a change like this to be a worthwhile trade-off, we would need to see a significant performance improvement over the current endOf implementation.

Have you conducted any performance benchmarks to demonstrate the gains of this new approach? This data would be very helpful for us to evaluate if the change is a net positive for the project.

Looking forward to your thoughts!

@Genarito
Copy link
Author

Genarito commented Oct 4, 2025

Hi!

I have discarded the changes related to calculating the days of the month, as I don't have time to measure whether the library now weighs 1 byte more. I left the package.json and cross-env changes that simplify the execution of tests on all operating systems. If they are useful, I will be delighted to have contributed to the project; otherwise, feel free to close the PR. I have also adjusted the title of the PR.

Best regards and thank you!

@Genarito Genarito changed the title Improve days in month performance + Little improvements Little improvements on npm commands + Upgrade of cross-env Oct 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants