Feedback from porting d3-time

[gilmoreorless]

I've been experimenting with porting the d3-time package to use Temporal.DateTime instead of legacy Date. Since a lot of data visualisations are built using d3, I thought it would be interesting to see how much work would be involved in supporting the new API. (Note: I'm not a d3 maintainer, so this is very much just my own non-official experiment.)

The result is at gilmoreorless/d3-time-temporal. These are some notes I took along the way:

Good

• The original d3-time has two methods for each type of interval (year, month, etc) — one for local time and one for UTC (thanks to relying on legacy Date). For the port I used only Temporal.DateTime and made the local and UTC variations the same. If the-proposal-currently-known-as-LocalDateTime (DO NOT MERGE: Proof-of-concept for ZonedDateTime (Instant + TimeZone + Calendar) #700) gets merged in, I'll update the port to use that.
• The consistency of the Temporal object fields and methods made the code a lot simpler. Once the conversion of d3.timeYear() was done, most of the other methods (e.g. d3.timeMonth()) were pretty much copy-paste-tweak. So much so that I ended up making a factory method to deduplicate them.
• Temporal's strong typing and not-implemented .valueOf() actually helped me find a bug in the d3-time test suite: Fix typo in offset tests d3/d3-time#42

Not so good

• I couldn't import the polyfill using standard NodeJS require() as listed in the polyfill's README. Fixed in Fix file paths to be relative in polyfill package.json #785.
• d3-time has a .round() method for its intervals (e.g. d3.timeHour.round(date)). It does this by getting the previous and next floored interval values for the provided datetime, and returning the closest one. My first instinct was to get the two respective Durations and then call Temporal.Duration.compare()... and then I realised it doesn't exist: Why no Duration.prototype.compare ? #608. Instead I had to use Temporal.DateTime.compare() based on adding the two Durations to the previous interval value, which felt like a workaround rather than the proper way to do it. See also How to round/ceil/floor? #560.
• The lack of negative Durations definitely made some code more clunky.
  • interval.offset(): d3-time allows positive and negative steps (e.g. d3.timeMinutes.offset(date, -15) for "15 minutes earlier"). This required switching on the sign of the step parameter and delegating to .plus() or .minus() appropriately. Not so bad when it was abstracted into a single helper, but annoyingly repetitive when it was a separate function per interval unit.
  • interval.count(): this counts how many instances of interval are between two datetimes. d3-time was able to do this in a single line for each interval unit (e.g. return end.getFullYear() - start.getFullYear();). But using Temporal, I had to do a complicated dance to store which DateTime was earlier in time, grab the Duration difference while always taking the earlier DateTime from the later one, get the time unit I needed, then possibly negate the result.
  • From what I can see, the plan for negative Duration values (Proposal: Negative Durations #782) would clean up both of these cases. 👍

---

Depending on time availability, I might also look at porting some other d3 packages that build upon d3-time, such as d3-scale. I'll add comments on this issue if I find anything else noteworthy.

[ptomato]

Thanks for doing this!

I think all of the negative points have been covered now! We have negative durations and duration rounding, so I'll close this issue.