js-temporal / temporal-polyfill

Polyfill for Temporal (under construction)
ISC License
529 stars 28 forks source link

PlainYearMonth.from(string) should not read 'YYYYMM' string #273

Closed nabe1653 closed 10 months ago

nabe1653 commented 10 months ago

In my understanding, PlainYearMonth.from(string) will read string arg as ISO 8601 and ISO 8601 will not allow without separator format about YearMonth. YearMonthDate has "YYYYMMDD" (without separator) as extended format but YearMonth has no extended format, because it has to differentiate to "YYMMDD"

But now polyfill does not throw error about Temporal.PlainYearMonth.from('202401') code, it can resolve as 2024-01 value.

I don't know this issue is of polyfill or temporal itself but I found this in this polyfill so I report here.

justingrant commented 10 months ago

Temporal does not support 2-digit years in any format, so we haven't been worried about ambiguity with any string format that uses 2-digit years.

2-digit years were supported in an older version of ISO 8601, but (for good reason that they were ambiguous) they were deprecated four years later. From wikipedia:

ISO 8601:2000 allowed truncation (by agreement), where leading components of a date or time are omitted. Notably, this allowed two-digit years to be used as well as the ambiguous formats YY-MM-DD and YYMMDD. This provision was removed in ISO 8601:2004.

Note that ISO 8601 (especially (https://en.wikipedia.org/wiki/ISO_8601#Standardised_extensions) like ISO 8601:2) has a huge surface area, only a fraction of which is commonly used in modern computing. Temporal has chosen to focus on that commonly-used subset, and to ignore obscure, infrequently-used, and deprecated extensions.

Given that we don't plan to support 2-digit years, I'm going to close this issue. Thanks for reporting this issue, it will be helpful to others in the future who may run into the same question!

nabe1653 commented 10 months ago

Hi @justingrant , Thank you for quick response.

What I'm concerned about is not YYMMDD format ref itself. My concerne is I can use this in future too because wiki said ISO does not support YYYYMM as YearMonth.

https://en.wikipedia.org/wiki/ISO_8601#Calendar_dates

YYYY-MM (but not YYYYMM)

I'm not an expert so I may not be understanding it correctly, but I feel that ISO 8601 clearly states that it does not support the YYYYMM format yet even if YYMMDD is not used. So my questions are below points:

  1. ISO 8601 supports YYYYMM format now? (If this is yes, so sorry about taking your time)
  2. Temporal will suport YYYYMM format even if ISO 8601 does not include this?

If this answers are No and Yes, I wish the document mentions too about omitted format is allowed or sample code include YYYYMM. Now document has only expected to be in ISO 8601 format text. https://tc39.es/proposal-temporal/docs/plainyearmonth.html#from

If PlainYearMonth.from supports YYYYMM format too, I would be glad because it's match to other class's omitted input and it is usefull.

justingrant commented 10 months ago

This is an intentional deviation that Temporal is making from the ISO 8601 specification. You can see this described in the Temporal specification here: https://tc39.es/proposal-temporal/#sec-temporal-iso8601grammar

  • When parsing a date representation without a day for a Temporal.PlainYearMonth, the expression is allowed to be in basic format (with no separator symbols).

Because this exception is in the Temporal spec, it's not going to change in the future.

If you'd like to submit a PR to improve the Temporal docs, PRs are always welcome! https://github.com/tc39/proposal-temporal

nabe1653 commented 10 months ago

Thank you so much! I wanted to read this grammar part.

And I will try to send PR for the document.