search

kodie / moment-holiday

A Moment.js plugin for handling holidays. NO LONGER MAINTAINED (DEPRECATED)
MIT License
83 stars 86 forks source link

Calculates Day After Thanksgiving incorrectly for 2019 #21

Open AndrewLane opened 6 years ago

AndrewLane commented 6 years ago

Looks like the library assumes that the Day After Thanksgiving is the 4th Friday of November based on this code:

https://github.com/kodie/moment-holiday/blob/447e7ff467ec18d7af89ed2de6b4c98924cc3319/locale/united_states.js#L73

However, on years like 2019 when the first day of the month of November is Friday, this means that the code will actually calculate the day after Thanksgiving as being before Thanksgiving itself. To repro:

var momentHoliday = require("moment-holiday")
var moment = require('moment');

var newYears2019 = moment('2019 01 01', 'YYYY MM DD');

console.log(momentHoliday(newYears2019).holiday('Thanksgiving').format('YYYY MM DD'));
console.log(momentHoliday(newYears2019).holiday('Day After Thanksgiving').format('YYYY MM DD'));

Expected:

2019 11 28
2019 11 29

Actual:

2019 11 28
2019 11 22
huangxuewu commented 6 years ago

same issue

AndrewLane commented 6 years ago

FYI I added a test that demonstrates the bug here: https://github.com/kodie/moment-holiday/pull/34

I hope it's helpful.

jorgedrr commented 6 years ago

I have the same issue

smarth55 commented 5 years ago

date: '11/(5,[23])' does the job. I added a custom day after thanksgiving with that date, but I can make a PR with the change.

AndrewLane commented 5 years ago

@smarth55 actually at this point i'm wondering if we should consider this a dead project since I haven't seen commits since 2017.

joeycozza commented 5 years ago

The day of reckoning is upon us!!!

Hopefully no one has important client facing code relying on this! Our little slack bot tool didn't give us our notifications this morning because of this bug, but that doesn't REALLY matter. Good luck all moment-holiday users!