moment / luxon

⏱ A library for working with dates and times in JS
https://moment.github.io/luxon
MIT License
15.12k stars 728 forks source link

Breaking regression from 3.3.0 to 3.4.0 in milliseonds #1488

Closed bnb closed 11 months ago

bnb commented 11 months ago

Describe the bug I use Luxon for nodevu, which is a module to get data about Node.js versions. I run nightly builds of static data for the module, and they started failing a bit ago. I recently had a user reach out saying they'd realized there was an issue with Luxon. Upon pretty basic investigation, it seems there's an issue between Luxon 3.3.0 and 3.4.0 where behavior around milliseconds seemingly changed.

To Reproduce Please share a minimal code example that triggers the problem: I wasn't able to narrow down what in Luxon is triggering this, but I did create a minimal reproduction to trigger the issue with nodevu. I apologize for not being able to dig in more and make something more minimal, but I'm currently OOO and am limited on time to work on code.

git clone https://github.com/bnb/luxon-breaking-minor.git
npm install
npm start

That will output nodevu output that can safely be ignored and the error + stack trace.

Actual vs Expected behavior Milliseconds management should not change in minors.

Desktop (please complete the following information):

diesieben07 commented 11 months ago

Thank you for the report and apologies for the breaking bug in a minor release. I will work on a pull request with a fix.

diesieben07 commented 11 months ago

FYI, this was caused by https://github.com/moment/luxon/pull/1467

It only affects invalid Durations. So the resulting value was going to be NaN anyways, which isn't useful.

SudhirkumarRamani commented 11 months ago

Hello @diesieben07,

Please suggest ETA on this issue if you could as it is breaking my app as well.

diesieben07 commented 11 months ago

Hello @SudhirkumarRamani

I cannot provide you an ETA unfortunately, as I am not in charge of the release schedule. However I want to stress again that this only affects invalid Durations. If you are running into this issue, your app is not handling invalid Durations properly.

SudhirkumarRamani commented 11 months ago

Hello @diesieben07 ,

Could please suggest right person who can guide on ETA of the release. Also, app is failing in build stage itself with error as below:

ERROR in ./node_modules/luxon/src/duration.js 579:40 -- 214 | Module parse failed: Unexpected token (579:40) 215 | You may need an appropriate loader to handle this file type, currently no loaders are configured to process this file. See https://webpack.js.org/concepts#loaders 216 | \| */ 217 | \| toMillis() { 218 | > let sum = this.values.milliseconds ?? 0; 219 | \| for (let unit of reverseUnits.slice(1)) { 220 | \| if (this.values?.[unit]) { 221 | @ ./node_modules/luxon/src/luxon.js 2:0-37 14:0-26:2 222 | @ ./node_modules/cron-parser/lib/date.js 223 | @ ./node_modules/cron-parser/lib/expression.js 224 | @ ./node_modules/cron-parser/lib/parser.js
diesieben07 commented 11 months ago

@SudhirkumarRamani Your issue is different. Your issue is caused by an outdated version of Webpack, which does not support "nullish coalescing" or "optional chaning". See this Webpack issue: https://github.com/webpack/webpack/issues/10227

The primary maintainer for this package is @icambron, who I have already tagged in the pull request which will fix this issue. However please note again that your issue is unrelated.

icambron commented 11 months ago

The fix has been released in 3.4.1