allow a union in Interval of

[xenoterracide]

this is needed for code like this to compile

    const end = offeredEnd
      ? Instant.ofEpochMilli(offeredEnd?.getUTCMilliseconds())
      : Duration.ofSeconds(Number.MAX_VALUE);

    const start = Instant.ofEpochMilli(offeredStart.getUTCMilliseconds());
    this._offeredDuring = Interval.of(start, end);

[perceptron8]

Seems to be caused by microsoft/TypeScript#14107.

Another, slightly simpler example of code that doesn't compile:

const start = Instant.EPOCH;
const endExclusiveOrDuration = Math.random() < 0.5 ? Instant.EPOCH : Duration.ZERO;
const interval = Interval.of(start, endExclusiveOrDuration);

[InExtremaRes]

Hi and thank you for contributing. I have a couple of comments:

1. The change you are proposing makes the overload definition redundant. I think you can remove the other two and keep just the one with the union type.
2. I would suggest adding some tests. Make sure to test a simplified version of the code you posted and see if the rest of the tests for this method make sense to you, so we don't end up having a breaking change.
3. The rest of the code tends to use names as instantOrDuration for parameters with union types. For consistency you can maybe change this one to endExclusiveOrDuration or something similar.

[xenoterracide]

The change you are proposing makes the overload definition redundant. I think you can remove the other two and keep just the one with the union type.

yeah I thought of this. wasn't sure how you'd want to deal with it. Basically I did as little as possible

I would suggest adding some tests. Make sure to test a simplified version of the code you posted and see if the rest of the tests for this method make sense to you, so we don't end up having a breaking change.

do you have typescript tests? I didn't see them

The rest of the code tends to use names as instantOrDuration for parameters with union types. For consistency you can maybe change this one to endExclusiveOrDuration or something similar.

isn't that what I called it? I think you typo-ed your suggestion. I thinking you want endInstantExclusiveOrDuration

[InExtremaRes]

The change you are proposing makes the overload definition redundant. I think you can remove the other two and keep just the one with the union type.

yeah I thought of this. wasn't sure how you'd want to deal with it. Basically I did as little as possible

I see. I think deleting the other two is very likely safe, but better ensure that with some tests.

do you have typescript tests? I didn't see them

Here. The file is just type-checked, it never runs.

isn't that what I called it? I think you typo-ed your suggestion. I thinking you want endInstantExclusiveOrDuration

I think you called it just end, but your suggestion is good too, whichever makes more sense to you and is consistent with core types.

[pithu]

Thanks @InExtremaRes for jumping in !

[xenoterracide]

https://github.com/packages/extra/test/typescript_definitions/js-joda-extra-tests.ts 404's ;)

[InExtremaRes]

@xenoterracide I must've misscopied the filepath, but the file is easy to find inside the project.

[xenoterracide]

sorry it took so long, busy little bee... this work?

[pithu]

Beside the one comment, LGTM.
@InExtremaRes do you have any further objections ?

[InExtremaRes]

Hi! I'm currently out of city and without any computer to really test it right now. I'll be back in a couple of days. However, if this PR is urgent, I feel very confident in the change it proposes and the tests can (if needed) be improved later.

[xenoterracide]

ok, I moved things around, better?

[xenoterracide]

p.s. it's not urgent. I think I worked around it, or realized something else needed doing anyways. Sorry that it took yet another 2 days. I've come down with covid, yay!

[pithu]

@xenoterracide thanks for contributing and sorry for the delay

[pithu]

And get well soon