Duration.fromObject({ hours: 2 }).milliseconds is 0

[KlausNie]

Describe the bug Duration.fromObject({ hours: 2 }).milliseconds === 0

Why does it not behave like

To Reproduce

import {
  DateTime,
  Duration
} from "luxon";

describe(`test`, function () {
  it(`should work`, function () {
    // correct behaviour with Duration created by diff
    const d1 = DateTime.now().minus({ days: 1 });
    const d2 = DateTime.now().plus({ days: 2 });
    const dur1 = d2.diff(d1);
    const diffInMilliseconds = dur1.milliseconds;
    console.log(diffInMilliseconds);
    expect(diffInMilliseconds).not.toBe(0);

    // incorrect behaviour with Duration creatd by Duration factory
    const duration = Duration.fromObject({ hours: 2 });
    console.log(duration.milliseconds);
    expect(duration.milliseconds).not.toBe(0)
  });
});

Actual vs Expected behavior Durations created by factories should behave the same way as diff Durations OR this needs to be documented!!!

Desktop (please complete the following information):

• luxon 3.4.4

Additional context IMO, the convenience accessors like get milliseconds(): IfValid<number, typeof NaN, IsValid>; are not fit for the Duration interface. It makes sense, for DateTime to get:

• milliseconds: the milliseconds part of this date
• date as milliseconds: time from 1970 as milliseconds but not for the Duration interface.

[diesieben07]

What do you expect Duration.fromObject({ years: 2 }).days to return? Valid options are 730, 731, 732. Which one is correct depends on your use case, because some years are leap years and some are not. Conversions between raw duration units (not bound to a particular point in time) are going to be lossy hence Luxon does not do them by default.

This is documented extensively: https://moment.github.io/luxon/#/math?id=duration-math.

The reason diff behaves "like you expect" is because you omitted the optional parameter unit that tells Luxon which units you want in the result. The default is "milliseconds", so because you didn't specify this parameter you get a Duration with just milliseconds in it. If you were to look at e.g. hours of the resulting Duration it would be 0, because you did not ask for hours.

[KlausNie]

@diesieben07 Understandable and makes sense. It wouldn't hurt to add that information to

    /**
     * Get the years.
     */
    get years(): IfValid<number, typeof NaN, IsValid>;

    /**
     * Get the quarters.
     */
    get quarters(): IfValid<number, typeof NaN, IsValid>;

    /**
     * Get the months.
     */
    get months(): IfValid<number, typeof NaN, IsValid>;

    /**
     * Get the weeks
     */
    get weeks(): IfValid<number, typeof NaN, IsValid>;

    /**
     * Get the days.
     */
    get days(): IfValid<number, typeof NaN, IsValid>;

    /**
     * Get the hours.
     */
    get hours(): IfValid<number, typeof NaN, IsValid>;

    /**
     * Get the minutes.
     */
    get minutes(): IfValid<number, typeof NaN, IsValid>;

    /**
     * Get the seconds.
     */
    get seconds(): IfValid<number, typeof NaN, IsValid>;

    /**
     * Get the milliseconds.
     */
    get milliseconds(): IfValid<number, typeof NaN, IsValid>;

in the code, or just that documentation link you posted. Unless I'm the only one who stumbled upon that, then you can ignore my suggestion.

[diesieben07]

The Duration class itself does talk about this, although it could be a bit clearer maybe. Unfortunately I do not really have the time (and energy) to work on Luxon at the moment (as you can see from the backlog of issues...), but if you think you can improve the inline documentation, a PR would be welcome.

[KlausNie]

@diesieben07 let me know what you think: https://github.com/moment/luxon/pull/1627