globalizejs / globalize

A JavaScript library for internationalization and localization that leverages the official Unicode CLDR JSON data
https://globalizejs.com
MIT License
4.8k stars 602 forks source link

Wrong week of year calculation for the every first week day. #909

Closed nightskylark closed 3 years ago

nightskylark commented 3 years ago

Steps to reproduce in Node.js: (npm install globalize cldr-data)

Case 1.

var Globalize = require( "globalize" );
Globalize.load( require( "cldr-data" ).entireSupplemental() );
Globalize.load( require( "cldr-data" ).entireMainFor( "en" ) );
Globalize("en").formatDate(new Date(2021,0,3), {raw: 'w'});

It returns 1 but the weekData.firstDay is 0 in 'en' locale and weekData.minDays is 1, so the result should be 2. image

Case 2.

var Globalize = require( "globalize" );
Globalize.load( require( "cldr-data" ).entireSupplemental() );
Globalize.load( require( "cldr-data" ).entireMainFor( "de" ) );
Globalize("de").formatDate(new Date(2021,0,4), {raw: 'w'});

It returns 0 but the weekData.firstDay is 1 in 'de' locale and weekData.minDays is 4, so the result should be 1. image

Solution Here is a mistake in calculation: https://github.com/globalizejs/globalize/blob/master/src/date/format.js#L151 The previously calculated value of the value variable is the count of days in the first week in year that belongs to the previous year. In 'en' locale in 2021 it will be 5 (27-31 of December 2020). The dateDayOfYear( date ) returns a distance between the first day of year and the current day. For the 3th of January it will be 2. So our calculation is (5 (27-31 of December) + 2 (2-3 of January)) / 7. But there is no 1st of January here! What we need is to add +1 to the formula like this:

dateDayOfYear( date ) + 1 + value ) / 7

rxaviers commented 3 years ago

Thanks for filing the issue and your detailed debugging. If you (or someone) can (a) confirm your logic matches UTS#35 and (b) submit a PR, it will be accepted. Thanks

nightskylark commented 3 years ago

(a) confirm your logic matches UTS#35

I'm sure that "my logic" matches the standard because the "w" pattern means "Week of Year (numeric)" (See here). All days of the single week should have the same Week of Year index. I don't think it is described in the specification, because it's the basic logic that doesn't have to be explained.

rxaviers commented 3 years ago

Perfect! 👍 Awesome you've already created a draft PR.