temporal
temporal copied to clipboard
boa-dev
Audit return types on all built-in methods
So far routinely TemporalResult has been used as the default return type. But currently a TemporalResult is sometimes used even in cases where the operation is infallible (most cases tend to be holdovers from when the method was returning a not-implemented error).
This means that the return types should be audited to ensure that any method that is infallible is not returning a return type. This will also properly stabilize the API where it is needed for next release / 0.1
For recent examples of a similar improvement, see boa-dev/temporal#235.
NOTE: infallible operations are marked in the Temporal spec with a ! whereas fallible operations are marked with a ?
I've looked through the code a few times, and it seems like everything that's fallible is wrapped in a TemporalResult, everything infallible that's wrapped in a TemporalResult is unimplemented or fallible in rust (due to getting system time information and such).
Maybe i haven't looked thoroughly enough.. but it's been a bit hard to detect infallible operations wrapped in Result types
Great! That's good to know. Thanks for looking into this.
fallible in rust (due to getting system time information and such).
I had a feeling that this might be the case on a lot of them. I'm going to leave this issue open as a reminder, because before a 0.1, I'd like to do a once over just as a double check.
But we can consider this functionally closed.
FWIW I think that we should avoid unwraps and use temporal_unwrap for cases where the spec has a !, which means we should still use TemporalResult.
This issue is mostly because I believe there are some methods around that are returning a TemporalResult that should be infallible.
Is the intention for this to extend to ZonedDateTime? I'm mentioning this mostly in consideration of #328.