Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TODO: Define min/max values #24

Closed
mattjohnsonpint opened this issue Jul 8, 2017 · 16 comments · Fixed by #402
Closed

TODO: Define min/max values #24

mattjohnsonpint opened this issue Jul 8, 2017 · 16 comments · Fixed by #402
Assignees
Labels
behavior Relating to behavior defined in the proposal
Milestone

Comments

@mattjohnsonpint
Copy link
Collaborator

All of the objects (except perhaps ZonedInstant) should have a MIN_VALUE and MAX_VALUE constant.

@mattjohnsonpint
Copy link
Collaborator Author

For Instant, we will use the same range as the old Date type, which is ±100,000,000 days surrounding the epoch. Since we have nanosecond precision, that works out to be -271821-04-20T00:00:00.000000000Z through 275760-09-13T00:00:00.000000000Z. We should probably restrict LocalDate and LocalDateTime to values at or near here, but we should consider the edge cases also.

@maggiepint maggiepint self-assigned this Jul 24, 2018
@ryzokuken ryzokuken added this to the Stage 3 milestone Jul 12, 2019
@ryzokuken
Copy link
Member

@maggiepint is this behavior still undefined? I couldn’t see any maximum or minimum values on the polyfill.

@ryzokuken ryzokuken added the behavior Relating to behavior defined in the proposal label Jul 12, 2019
@littledan
Copy link
Member

Do we actually want this API? I'm not convinced we do. I don't think "Number has it" is a good reason, and I don't understand the motivation. It'd be good to hear from @pdunkel on this question.

@gibson042
Copy link
Collaborator

We definitely want to have minimum and maximum values and it makes sense to align them with Date, but that doesn't mean they must be exposed as properties.

@littledan
Copy link
Member

Why will we have minimum and maximum values?

@gibson042
Copy link
Collaborator

It matters in serialization and deserialization. Per ISO 8601, representing years outside of 0000 through 9999 requires "mutual agreement of the partners in information interchange" and "interchange parties shall agree the additional number of digits". The existing ECMAScript date-time string interchange format specifies six digits for expanded years, and sticking with that means we couldn't go beyond years -999999 through 999999.

And having established the necessity of limits, my opinion is that we might as well share them rather than triple the existing 500,000 year range (anyone needing more than that is likely to need much more, and should pursue a specialized library).

@littledan
Copy link
Member

OK, I could see that we don't want to make un-serializable Temporal objects. Thanks for explaining.

@ryzokuken
Copy link
Member

I guess this isn't relevant anymore since we are already adhering to a subset of the ISO format, Duration for example already implements the restrictions mentioned above. Closing, but feel free to reopen if necessary.

@ptomato
Copy link
Collaborator

ptomato commented Feb 13, 2020

Based on discussion in #198 I think we should reopen this. It was decided so far in the thread that Temporal.Absolute has the same range as old-style Date (epoch ± 1e8 days) but this behaviour needs to be reflected in the spec.

Also we need to decide on a range for Temporal.DateTime, Temporal.Date, and Temporal.YearMonth; should they be the same as Temporal.Absolute (in which case you have to throw a RangeError when doing Temporal.Absolute.from(maxAbsolute).inTimeZone('+01:00'), for example) or should they encompass an extra day around either end (in which case you have to throw a RangeError when doing Temporal.DateTime.from(maxDateTime).inTimeZone('UTC'))?

Temporal.Duration doesn't currently have any limits at all: new Temporal.Duration(Infinity).years is accepted.

@ptomato ptomato reopened this Feb 13, 2020
@gibson042
Copy link
Collaborator

If we view the Temporal.Absolute range as fundamental, then Temporal.Date and Temporal.DateTime should probably support a slightly larger range capable of representing any such value in any supported time zone (e.g., -271821-04-19T00:00 to +275760-09-14T00:00 [exclusive at both boundaries], assuming the Temporal.Absolute range matches the legacy Date range and the maximum UTC offset magnitude is less than ±24:00)—which does indeed imply RangeError from e.g. Temporal.DateTime.from("+275760-09-13T23:59").inTimeZone("+00:00"). And Temporal.YearMonth should then be similarly constrained (-271821-04 to +275760-09, inclusive).

As for Temporal.Duration, the range should be at least sufficient to cover the maximum difference of valid values (e.g., 200_000_002 days), but I don't see a problem with expanding it dramatically, potentially even past Number.{MIN,MAX}_SAFE_INTEGER into precision loss—but not so far that it accepts infinite values, which are an obvious problem.

@jasonwilliams
Copy link
Member

Chrono has max and min dates
https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=0fa2de3f66b7c7b1b27eff7f0faa5b6a

@quodlibetor were there reasons for this being added to Chrono?

@ptomato
Copy link
Collaborator

ptomato commented Feb 19, 2020

As for Temporal.Duration, the range should be at least sufficient to cover the maximum difference of valid values (e.g., 200_000_002 days), but I don't see a problem with expanding it dramatically, potentially even past Number.{MIN,MAX}_SAFE_INTEGER into precision loss—but not so far that it accepts infinite values, which are an obvious problem.

See also #324 about "unbalanced" Durations. Should every field be able to cover at least 2e8+2 days without precision loss, e.g. maximum nanoseconds must be at least 200_000_002n*86400n*BigInt(1e9) === 17280000172800000000000n? This would involve changing the type of the milliseconds, microseconds, and nanoseconds slots to BigInt.

@gibson042
Copy link
Collaborator

There will never be precision loss with the proposed default unit cutoff of "days", so I don't think it's necessary to support BigInt in Temporal.Duration fields—if an author wants to explicitly deal with large intervals using small units, they need to be prepared for floating-point errors (which at the extremes are within 5e-15 percent for milliseconds and 1e-14 percent for nanoseconds anyway).

@quodlibetor
Copy link

were there reasons for this being added to Chrono?

Implementation details, really, and making it convenient for folks to find out what the implementation limitations are. Ideally we'll support arbitrary underlying types in the future and the min/max values will become derived from the underlying timespan types. So a Date<u64> would have a different min/max from Date<ArbitraryPrecisionTimeSpan>.

@ptomato
Copy link
Collaborator

ptomato commented Feb 22, 2020

Paraphrasing #24 (comment), here's what I intend to implement:

  • Absolute's limits are exactly enough to represent any old-style JS Date with any in-range number of microseconds or nanoseconds.
  • DateTime's limits are exactly enough to represent any Absolute in any valid TimeZone (offset -24 to +24 hours, exclusive at both boundaries.)
  • Date's limits are exactly enough that a call of getDate() on any valid DateTime succeeds.
  • YearMonth's limits are exactly enough that a call of getYearMonth() on any valid Date succeeds.

I haven't figured out what to do yet for Duration. Infinite fields are obviously out, and arbitrarily large finite fields seem okay, but it's unclear how to roundtrip them via toString() if they are outside the range of Number.{MIN,MAX}_SAFE_INTEGER. Scientific notation is not a valid ISO 8601 duration. toFixed() uses scientific notation after 10^21. toLocaleString() seems to round. There's also BigInt(num).toString().

@gibson042
Copy link
Collaborator

I like that, and BigInt-like serialization of Duration fields makes sense (e.g., Duration.from({seconds: 1e30}).toString() == "PT1000000000000000019884624838656S").

@ptomato ptomato self-assigned this Feb 25, 2020
ptomato added a commit that referenced this issue Feb 27, 2020
- DateTime's limits are exactly enough to represent any Absolute in any
  valid TimeZone (offset -24 to +24 hours, exclusive at both
  boundaries.)
- Date's limits are exactly enough that a call of getDate() on any valid
  DateTime succeeds.
- YearMonth's limits are exactly enough that a call of getYearMonth() on
  any valid Date succeeds.

This also means that the inverse conversions of those mentioned above
can fail. If they are called in 'constrain' disambiguation mode, then
they'll be clamped to the nearest in-range value for the resulting type.
The 'balance' mode can't do anything in this case, so it'll behave like
'reject'.

Also ensures that plus() and minus() can't have results that are out of
the range.

See: #24.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
behavior Relating to behavior defined in the proposal
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants