Convenience comparison methods: lessThan(), greaterThan(), lessEquals(), greaterEquals()

[justingrant]

After a few days of using since and until, I'm now 100% converted to @gibson042's position that confusingly-ordered parameters are evil. While discussing difference last week before we decided to rename it to since and until, we also discussed how compare also has confusing parameter order. The other thing that's annoying about compare is that it's not "chainable", so you can't read it left to right. Instead you have to wrap the compare call around your code which makes it harder to read.

If it's not too late, I got inspired today and built (polyfill, tests, docs, spec) convenience comparison methods, tentatively named greaterThan(), lessThan(), greaterEquals(), and lessEquals(). I don't have a strong opinion about the names other than wanting to not make them any longer.

Here's an example. It's 13-24 fewer characters per call. Also IMHO it's easier to understand what the code is doing.

// proposed
function afterKidsBedtime(time) {
  return time.greaterThan('20:30');
}
// current
function afterKidsBedtime(time) {
  return Temporal.Time.compare(time, '20:30') > 0;
}

Here's another sample from the docs:

dt1 = Temporal.DateTime.from('1995-12-07T03:24:30.000003500');
dt2 = Temporal.DateTime.from('2019-01-31T15:30');
dt1.lessThan(dt2); // => true
dt1.lessThan('1999-12-31'); // => false
dt1.lessThan(dt1); // => false

I wasn't sure about how to encode a simple if/else statement in the spec syntax. Is this valid?

1. If _comparison_ ≥ *+0* then return *true* else return *false*.

If not, what's the right spec syntax to express this?

[codecov]

Codecov Report

Merging #1074 into main will increase coverage by 0.05%.
The diff coverage is 95.69%.

@@            Coverage Diff             @@
##             main    #1074      +/-   ##
==========================================
+ Coverage   94.88%   94.93%   +0.05%     
==========================================
  Files          18       18              
  Lines        7134     7286     +152     
  Branches     1137     1194      +57     
==========================================
+ Hits         6769     6917     +148     
- Misses        358      362       +4     
  Partials        7        7              

Flag	Coverage Δ
#test262	43.63% <11.61%> (-1.99%)	⬇️
#tests	91.08% <91.78%> (-0.11%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
polyfill/lib/yearmonth.mjs	91.93% <55.00%> (-4.14%)	⬇️
polyfill/lib/ecmascript.mjs	96.45% <98.38%> (+0.41%)	⬆️
polyfill/lib/date.mjs	93.93% <100.00%> (+0.87%)	⬆️
polyfill/lib/datetime.mjs	95.85% <100.00%> (+0.20%)	⬆️
polyfill/lib/duration.mjs	97.54% <100.00%> (+0.08%)	⬆️
polyfill/lib/instant.mjs	97.04% <100.00%> (+0.26%)	⬆️
polyfill/lib/time.mjs	97.68% <100.00%> (+0.17%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 643fd26...a42b528. Read the comment docs.

[ptomato]

Meeting, Oct 29: We won't include this in V1 of the proposal, but will consider it for a follow up, after doing more deliberate design work on the API.

[justingrant]

Meeting, Oct 29: We won't include this in V1 of the proposal, but will consider it for a follow up, after doing more deliberate design work on the API.

My understanding from the meeting was that the team was willing to consider this in V1 but after the release of the specs to Stage 3 reviewers, with the justification that this proposal is not a breaking change and has no upstream or downstream dependencies inside or outside Temporal. It's also possible that reviewers may not want this which seems reasonable. But IMHO this seems like a low-cost usability win that's worth considering if possible.

EDIT: here's the meeting notes where we agreed to continue considering this:

RGN: I see the value but I'm not sure about the priority. It's probably worth brainstorming on the names, continuing to talk about it, but not considering it a blocker.
SFC: Agreed, it wouldn't be a disruptive change if we were to add this during Stage 3 review.
We'll keep considering this but it won't be considered a blocker.

Regardless, sounds like there was interest in bikeshedding on the names and perhaps other parts of this proposal, so I'll open an issue for that purpose.

[littledan]

Should this PR be closed?

[ryzokuken]

Converted to draft in order to make it more explicit that this is not a finished PR. @justingrant @ptomato how should we proceed with this?

[ptomato]

I've moved it over to Temporal V2 at: js-temporal/proposal-temporal-v2#6