Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Breaking regression from 3.3.0 to 3.4.0 in milliseonds #1488

Closed
bnb opened this issue Aug 16, 2023 · 7 comments · Fixed by #1489
Closed

Breaking regression from 3.3.0 to 3.4.0 in milliseonds #1488

bnb opened this issue Aug 16, 2023 · 7 comments · Fixed by #1489

Comments

@bnb
Copy link
Contributor

bnb commented Aug 16, 2023

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):

  • OS: [e.g. iOS] macOS
  • Browser [e.g. Chrome 84, safari 14.0] N/A
  • Luxon version [e.g. 1.25.0] 3.4.0
  • Your timezone [e.g. "America/New_York"] America/New_York
@diesieben07
Copy link
Collaborator

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
Copy link
Collaborator

diesieben07 commented Aug 16, 2023

FYI, this was caused by #1467

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

@SudhirkumarRamani
Copy link

Hello @diesieben07,

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

@diesieben07
Copy link
Collaborator

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
Copy link

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
Copy link
Collaborator

@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: webpack/webpack#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
Copy link
Member

The fix has been released in 3.4.1

brboi added a commit to odoo-dev/odoo that referenced this issue Aug 30, 2023
**Changelog**

**3.4.2 (2023-08-26)**

- Fixes regression from 3.4.1 (moment/luxon#1493)

**3.4.1 (2023-08-23)**

- Fixes for regressions from 3.4.0
  (moment/luxon#1482 and moment/luxon#1488)

**3.4.0 (2023-08-08)**

- Fix type checking on input zones
- Fix Islamic months listing
- Fix normalize() for negative inputs

**3.3.0 (2023-03-03)**

- Fix off-by-one in Interval#count (moment/luxon#1308)
- Support formatting for custom zones (moment/luxon#1377)
- Fix parsing for narrow spaces (moment/luxon#1369)
- Handle leap year issue with AD 100 (moment/luxon#1390)
- Allow parsing of just an offset

**3.2.1 (2023-01-04)**

- Fix for RFC-2822 regex vulnerability
- Better handling of BCP tags with -x- extensions

**3.2.0 (2022-12-29)**

- Allow timeZone to be specified as an intl option
- Fix for diff's handling of end-of-month when crossing leap years
  (moment/luxon#1340)
- Add Interval.toLocaleString() (moment/luxon#1320)

**3.1.1 (2022-11-28)**

- Add Settings.twoDigitCutoffYear

**3.1.0 (2022-10-31)**

- Add Duration.rescale

**3.0.4 (2022-09-24)**

- Fix quarters in diffs (moment/luxon#1279)
- Export package.json in package (moment/luxon#1239)

**3.0.2 (2022-08-28)**

- Lots of doc changes
- Added DateTime.expandFormat
- Added support for custom conversion matrices in Durations
robodoo pushed a commit to odoo/odoo that referenced this issue Sep 4, 2023
**Changelog**

**3.4.2 (2023-08-26)**

- Fixes regression from 3.4.1 (moment/luxon#1493)

**3.4.1 (2023-08-23)**

- Fixes for regressions from 3.4.0
  (moment/luxon#1482 and moment/luxon#1488)

**3.4.0 (2023-08-08)**

- Fix type checking on input zones
- Fix Islamic months listing
- Fix normalize() for negative inputs

**3.3.0 (2023-03-03)**

- Fix off-by-one in Interval#count (moment/luxon#1308)
- Support formatting for custom zones (moment/luxon#1377)
- Fix parsing for narrow spaces (moment/luxon#1369)
- Handle leap year issue with AD 100 (moment/luxon#1390)
- Allow parsing of just an offset

**3.2.1 (2023-01-04)**

- Fix for RFC-2822 regex vulnerability
- Better handling of BCP tags with -x- extensions

**3.2.0 (2022-12-29)**

- Allow timeZone to be specified as an intl option
- Fix for diff's handling of end-of-month when crossing leap years
  (moment/luxon#1340)
- Add Interval.toLocaleString() (moment/luxon#1320)

**3.1.1 (2022-11-28)**

- Add Settings.twoDigitCutoffYear

**3.1.0 (2022-10-31)**

- Add Duration.rescale

**3.0.4 (2022-09-24)**

- Fix quarters in diffs (moment/luxon#1279)
- Export package.json in package (moment/luxon#1239)

**3.0.2 (2022-08-28)**

- Lots of doc changes
- Added DateTime.expandFormat
- Added support for custom conversion matrices in Durations

closes #133599

Related: odoo/enterprise#46556
Signed-off-by: Luca Vitali (luvi) <luvi@odoo.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants