Update date-fns to v3
Summary
This PR updates date-fns to v3 and adds support for it. Some other minor changes include:
- Adding
@babel/preset-envand removing the individual babel plugin transforms since they are included. - Running
xiton the tests that expected aRangeError. This error does not seem to occur anymore indate-fnsv3 but have chosen not to remove it in case these tests want to be rewritten.
We do not miss a loader, but we need to support https://babeljs.io/docs/babel-plugin-transform-logical-assignment-operators.
I understand that there are two options
- explicitly use the plugin
- use preset-env
Plugin
{
"plugins": ["@babel/plugin-transform-logical-assignment-operators"]
}
@babel/preset-env
"presets": [
["@babel/env", {
"targets": ["2021"], // at least 2021
}]
Here is a checklist for other changes we might need:
- [ ] update
peerDependenciesto>=3.0.0fordate-fns(related to https://github.com/marnusw/date-fns-tz/pull/262) - [ ] Should
.yarn/releasesbe committed? https://yarnpkg.com/getting-started/qa#which-files-should-be-gitignored
Thanks everyone, I hope to find time for this soon.
Thanks for the reviews @antoinepairet @neilhem @JeromeFitz ! 🙌🏾
I got the tests to run now. Looks like 6 tests are failing at the moment, most are missing RangeErrors and some incorrect timezones.
$ yarn test
START:
ℹ 「wdm」: Built at: 12/29/2023 9:46:54 AM
ℹ 「wdm」: Compiled successfully.
ℹ 「wdm」: Compiling...
⚠ 「wdm」: Built at: 12/29/2023 9:46:55 AM
Entrypoint ../test = ../test.js
[./node_modules/date-fns/format.js] 25.7 KiB {../test} [built]
[./node_modules/date-fns/locale/en-GB.js] 772 bytes {../test} [built]
[./node_modules/power-assert/index.js] 2.2 KiB {../test} [built]
[./src sync recursive \/test\.js$] ./src sync \/test\.js$ 460 bytes {../test} [built]
[./src/_lib/newDateUTC/test.js] 36.4 KiB {../test} [optional] [built]
[./src/_lib/tzIntlTimeZoneName/test.js] 22.4 KiB {../test} [optional] [built]
[./src/_lib/tzParseTimezone/test.js] 127 KiB {../test} [optional] [built]
[./src/_lib/tzTokenizeDate/test.js] 31.8 KiB {../test} [optional] [built]
[./src/format/test.js] 171 KiB {../test} [optional] [built]
[./src/formatInTimeZone/test.js] 16.4 KiB {../test} [optional] [built]
[./src/getTimezoneOffset/test.js] 100 KiB {../test} [optional] [built]
[./src/toDate/test.js] 255 KiB {../test} [optional] [built]
[./src/utcToZonedTime/test.js] 53.5 KiB {../test} [optional] [built]
[./src/zonedTimeToUtc/test.js] 89.8 KiB {../test} [optional] [built]
[./test.js] 124 bytes {../test} [built]
+ 241 hidden modules
WARNING in ./node_modules/power-assert-formatter/lib/create.js 30:28-49
Critical dependency: the request of a dependency is an expression
@ ./node_modules/power-assert-formatter/index.js
@ ./node_modules/power-assert/index.js
@ ./src/format/test.js
@ ./src sync \/test\.js$
@ ./test.js
ℹ 「wdm」: Compiled with warnings.
29 12 2023 09:46:55.911:INFO [karma-server]: Karma v6.4.2 server started at http://localhost:9876/
29 12 2023 09:46:55.912:INFO [launcher]: Launching browsers LocalChrome with concurrency unlimited
29 12 2023 09:46:55.925:INFO [launcher]: Starting browser Chrome
29 12 2023 09:46:56.960:INFO [Chrome 120.0.0.0 (Mac OS 10.15.7)]: Connected on socket f5AV3NtniDRTrLKXAAAB with id 79769157
Finished in 0.004 secs / 0.023 secs @ 09:46:57 GMT+0100 (GMT+01:00)
SUMMARY:
✔ 259 tests completed
ℹ 1 test skipped
✖ 6 tests failed
FAILED TESTS:
format
✖ throws `RangeError` if `options.weekStartsOn` is not convertable to 0, 1, ..., 6 or undefined
Chrome 120.0.0.0 (Mac OS 10.15.7)
AssertionError: Missing expected exception (RangeError)..
at _throws (webpack:///node_modules/assert/assert.js:454:1 <- /Users/christopherklint/coding/date-fns-tz/test.js:1163:5)
at ./node_modules/assert/assert.js.assert.throws (webpack:///node_modules/assert/assert.js:478:1 <- /Users/christopherklint/coding/date-fns-tz/test.js:1187:3)
at Context.<anonymous> (webpack:///src/format/test.js:865:12 <- /Users/christopherklint/coding/date-fns-tz/test.js:29176:26)
✖ throws `RangeError` if `options.firstWeekContainsDate` is not convertable to 1, 2, ..., 7 or undefined
Chrome 120.0.0.0 (Mac OS 10.15.7)
AssertionError: Missing expected exception (RangeError)..
at _throws (webpack:///node_modules/assert/assert.js:454:1 <- /Users/christopherklint/coding/date-fns-tz/test.js:1163:5)
at ./node_modules/assert/assert.js.assert.throws (webpack:///node_modules/assert/assert.js:478:1 <- /Users/christopherklint/coding/date-fns-tz/test.js:1187:3)
at Context.<anonymous> (webpack:///src/format/test.js:873:12 <- /Users/christopherklint/coding/date-fns-tz/test.js:29183:26)
time zone
✖ Local date with specific non-location time zone
Chrome 120.0.0.0 (Mac OS 10.15.7)
AssertionError: # src/format/test.js:669
assert(result === dateAndTimeZoneAmericaNY)
| | |
| | "1986-04-04 10:32:55 EST"
| false
"1986-04-04 10:32:55 GMT-5"
--- [string] dateAndTimeZoneAmericaNY
+++ [string] result
@@ -17,7 +17,9 @@
:55
-EST
+GMT-5
at ./node_modules/empower-core/lib/decorator.js.Decorator._callFunc (webpack:///node_modules/empower-core/lib/decorator.js:110:1 <- /Users/christopherklint/coding/date-fns-tz/test.js:12223:20)
at ./node_modules/empower-core/lib/decorator.js.Decorator.concreteAssert (webpack:///node_modules/empower-core/lib/decorator.js:103:1 <- /Users/christopherklint/coding/date-fns-tz/test.js:12216:17)
at decoratedAssert (webpack:///node_modules/empower-core/lib/decorate.js:49:1 <- /Users/christopherklint/coding/date-fns-tz/test.js:12084:30)
at powerAssert (webpack:///node_modules/empower-core/index.js:63:1 <- /Users/christopherklint/coding/date-fns-tz/test.js:12007:32)
at Context.<anonymous> (webpack:///src/format/test.js:669:13 <- /Users/christopherklint/coding/date-fns-tz/test.js:28804:32)
✖ https://github.com/marnusw/date-fns-tz/issues/138
Chrome 120.0.0.0 (Mac OS 10.15.7)
AssertionError: # src/format/test.js:685
assert.equal(result, '2020-10-31 9:37pm -05:00 -05:00 GMT-5 CDT')
|
"2020-10-31 9:37pm -05:00 -05:00 GMT-5 GMT-5"
at ./node_modules/empower-core/lib/decorator.js.Decorator._callFunc (webpack:///node_modules/empower-core/lib/decorator.js:110:1 <- /Users/christopherklint/coding/date-fns-tz/test.js:12223:20)
at ./node_modules/empower-core/lib/decorator.js.Decorator.concreteAssert (webpack:///node_modules/empower-core/lib/decorator.js:103:1 <- /Users/christopherklint/coding/date-fns-tz/test.js:12216:17)
at Function.decoratedAssert [as equal] (webpack:///node_modules/empower-core/lib/decorate.js:49:1 <- /Users/christopherklint/coding/date-fns-tz/test.js:12084:30)
at Context.<anonymous> (webpack:///src/format/test.js:685:14 <- /Users/christopherklint/coding/date-fns-tz/test.js:28837:28)
custom locale
✖ throws `RangeError` if `options.locale` doesn't have `localize` property
Chrome 120.0.0.0 (Mac OS 10.15.7)
AssertionError: Missing expected exception (RangeError)..
at _throws (webpack:///node_modules/assert/assert.js:454:1 <- /Users/christopherklint/coding/date-fns-tz/test.js:1163:5)
at ./node_modules/assert/assert.js.assert.throws (webpack:///node_modules/assert/assert.js:478:1 <- /Users/christopherklint/coding/date-fns-tz/test.js:1187:3)
at Context.<anonymous> (webpack:///src/format/test.js:844:14 <- /Users/christopherklint/coding/date-fns-tz/test.js:29157:28)
✖ throws `RangeError` if `options.locale` doesn't have `formatLong` property
Chrome 120.0.0.0 (Mac OS 10.15.7)
AssertionError: Missing expected exception (RangeError)..
at _throws (webpack:///node_modules/assert/assert.js:454:1 <- /Users/christopherklint/coding/date-fns-tz/test.js:1163:5)
at ./node_modules/assert/assert.js.assert.throws (webpack:///node_modules/assert/assert.js:478:1 <- /Users/christopherklint/coding/date-fns-tz/test.js:1187:3)
at Context.<anonymous> (webpack:///src/format/test.js:856:14 <- /Users/christopherklint/coding/date-fns-tz/test.js:29168:28)
@marnusw you probably know best here if we should keep the RangError checks and implement them in date-fns-tz or just remove them?
Thanks for making the time on the latest and greatest. Awesome, work @christopherklint97 !
For the RangeError ones date-fns@3 introduced types to inform the developer that they shouldn't do these things. So that may mean that intentionally breaking in a test will no longer work, since the errors themselves have changed. (I ran into these exact ones in an out-of-scope move to types.)
But would defer to someone who knows more on this package and karma for sure.
For this one though:
throws
RangeErrorifoptions.firstWeekContainsDateis not convertable to 1, 2, ..., 7 or undefined
This changed to only allow 1 | 4 in date-fns@3 (in 2021):
- Reference: https://github.com/date-fns/date-fns/blob/main/src/types.ts#L138
I believe this should also happen in date-fns-tz given the comments in date-fns code (reference in #2427). If so this would need to be changed in a few references:
- https://github.com/marnusw/date-fns-tz/blob/master/typings.d.ts#L32
- https://github.com/marnusw/date-fns-tz/blob/master/docs/OptionsWithTZ.js#L13
Every index.js.flow would get their update in the next build per the current structure.
Though now that date-fns@3 exports types this could be changed further in typings.d.ts:
- Some other slight changes with
weekStartsOn|unit|roundingMethodshould be confirmed ifdate-fns-tzhas different values or should be kept in-sync.
import type { Day, FirstWeekContainsDate, LocaleUnit, RoundingMethod } from "date-fns";
import type { Locale } from 'date-fns'
type OptionsWithTZ = {
weekStartsOn?: Day,
firstWeekContainsDate?: FirstWeekContainsDate,
additionalDigits?: 0 | 1 | 2,
timeZone?: string,
locale?: Locale,
includeSeconds?: boolean,
addSuffix?: boolean,
unit?: LocaleUnit,
roundingMethod?: RoundingMethod,
awareOfUnicodeTokens?: boolean,
}
declare module.exports: (
date: Date | string | number,
timeZone: string,
formatStr: string,
options?: OptionsWithTZ
) => string
This is looking real good! Props again! 🎉
Additional Edit:
The Local date with specific non-location time zone error looks to be local on your machine. None of the Ci/CD have it, nor do I. (Who happens to be dateAndTimeZoneAmericaNY.)
Wonder if running:
env TZ=utc yarn run test
Would work locally?
But it is looking just like the RangeErrors:
SUMMARY:
✔ 261 tests completed
ℹ 1 test skipped
✖ 4 tests failed
A potential option if the tests should be kept in until date-fns-tz moves to types or something, their functions could be changed to xit which will tell karma to ignore the test.
Aye ... digging into this a bit more if the move to date-fns@3 types is to accompany.
The three places of change are:
1️⃣
./scripts/build/_lib/typings/formatBlock.js
L114:
"import type { Day, FirstWeekContainsDate, Locale, LocaleUnit, RoundingMethod } from 'date-fns'\n" +
2️⃣
./scripts/build/_lib/typings/typeScript.js
L296:
import type { Day, FirstWeekContainsDate, Locale, LocaleUnit, RoundingMethod } from "date-fns"
3️⃣
./docs/OptionsWithTZ.js
Then editing the way they are currently formatted with flow
L9:
* @property {Day} [weekStartsOn=0] - the index of the first day of the week (0 - Sunday).
L13:
* @property {FirstWeekContainsDate} [firstWeekContainsDate=1] - the day of January,
L29:
* @property {LocaleUnit} [unit] - used by `formatDistanceStrict`.
L31:
* @property {RoundingMethod} [roundingMethod='floor'] - used by `formatDistanceStrict`.
Then after a yarn run build that will update the rest of the files accordingly. 😅
@marnusw all tests are passing now since I am xiting the RangeError tests. All you have to do is merge and release a new version if you are happy 😉
@JeromeFitz I really appreciate the comments here and making it so easy for me to copy and paste the changes 👍🏾
Looking forward to this 😃
Hello, what is the status of this PR? Thanks
Hello, what is the status of this PR? Thanks
@tomalaforge I would say status here is awaiting a review and possible merge from @marnusw if he is happy with the changes.
@christopherklint97, thanks for this PR! I hope @marnusw merges soon.
please merge 👯
When merge? 👁️ 👄 👁️
Hi all, thank you very much for all this work! I will do my best to get to this and merge it within a week or so.
Hi all, thank you very much for all this work! I will do my best to get to this and merge it within a week or so.
Any news @marnusw ?
For people looking at a solution now: https://www.npmjs.com/package/date-fns-tz-v3
Hi all, thank you very much for all this work! I will do my best to get to this and merge it within a week or so.
Any news @marnusw ?
For people looking at a solution now: https://www.npmjs.com/package/date-fns-tz-v3
@marnusw I don't understand why this linked package exists. Should we use that or wait for this PR?
Hi all, thank you very much for all this work! I will do my best to get to this and merge it within a week or so.
Any news @marnusw ? For people looking at a solution now: https://www.npmjs.com/package/date-fns-tz-v3
@marnusw I don't understand why this linked package exists. Should we use that or wait for this PR?
No, sir. This library should not be used. The following repo will tell you all you need to know
https://github.com/davidramosweb/date-fns-tz
No, sir. This library should not be used. The following repo will tell you all you need to know
https://github.com/davidramosweb/date-fns-tz
Thanks for pointing it out. The npm page for it is super misleading and makes you think @marnusw released it.
No, sir. This library should not be used. The following repo will tell you all you need to know https://github.com/davidramosweb/date-fns-tz
Thanks for pointing it out. The npm page for it is super misleading and makes you think @marnusw released it.
Misleading indeed! had me fooled :(
Does anyone have a working solution other than waiting for this PR to be merged? I don't want to wait any longer.
@keesvanbemmel if you really can't wait you can npm install from any github repo or branch, including the branch for this PR (christopherklint97:update-to-v3).
@keesvanbemmel if you really can't wait you can
npm installfrom any github repo or branch, including the branch for this PR (christopherklint97:update-to-v3).
Thanks will do!
Can we have a release please? Installing from branch may be okay for development but launching a product to production with a dependency like that is not safe. I cannot start using this library because our project was started with date-fns v3 and downgrading is more effort than what it's worth. I get that maintainers may be busy people, but someone already put time and effort into making this PR while community keeps begging for this to be released.
I know that maintaining a OSS is a lot of work. I'm maintaining one myself. But here, everything has been done. I don't understand what is stopping you from merging and publishing a new version. It should take 30min max. 🙏
In defense of @marnusw this is a new major version with breaking changes and maybe he needs time to consider the implications and how to document. Besides the imports, several tests were turned off because they no longer throw expected errors. If you think installing from a branch is "not safe," publishing a major version without fully understanding the breaking changes is worse. There's literally a comment right above @BlindDespair with a bug.
You can always clone the PR branch to your own repo and lock your install to the exact commit SHA. That's really as safe as you can you can ask for, especially if you're so personally sure this code is 100% ready and @marnusw has no reason not to merge it.
@stephenhmarsh that is so, but this package is linked by date-fns' documentation as go-to solution for working with timezones when using date-fns. Normally other libraries or frameworks update together around the same time, meaning date-fns-tz would take care of these changes before v3 was even released on the main package. Otherwise the date-fns package shouldn't link to this package and not claim that there's a way to work with timezones and maybe explicitly state that their package is not recommended if timezones are necessary.
"Patience is a virtue". Does v2 have any security implications, not yet. Is this being worked on for free, yes.
Please merge @marnusw 🙏
Please merge @marnusw 🙇♂️
@marnusw anything pending for this PR to be merged? If possible release a beta
@marnusw anything pending for this PR to be merged? If possible release a beta
A beta can be official or, use this branch directly. See https://martinwolf.org/2018/04/github-branch-as-dependency-package-json/ Then you can help troubleshoot but, I would not use it for production.
