Skip to content

Conversation

gibson042
Copy link
Member

Prompted by discussion in Matrix.

@gibson042 gibson042 requested a review from ptomato October 10, 2025 14:39
@gibson042 gibson042 requested a review from a team as a code owner October 10, 2025 14:39
Copy link
Contributor

@ptomato ptomato left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

assert.throws(RangeError, () => Temporal.PlainMonthDay.from({ monthCode, day: 17 }),
`monthCode '${monthCode}' is not valid for ISO 8601 calendar`);
`monthCode '${monthCode}' is not valid for ISO 8601 calendar (without numeric month)`);
var plausibleMonth = Number(monthCode.slice(1, 3)) + (monthCode.length - 3);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I see what the intention of this line is, but 0, 19, 99, 13, and 14 are definitely not valid numeric months in the ISO calendar, so it wouldn't catch the case where the numeric month is valid but the month code isn't.

Maybe that's OK as is, or maybe a possible solution would be to clamp plausibleMonth between 1 and 12?

@gibson042 gibson042 force-pushed the 2025-10-temporal-plainmonthday-invalid-monthcode-with-month branch from d03a20e to 3f4763e Compare October 15, 2025 18:54
@gibson042 gibson042 force-pushed the 2025-10-temporal-plainmonthday-invalid-monthcode-with-month branch from 3f4763e to ab0c9f1 Compare October 15, 2025 18:55
@gibson042 gibson042 enabled auto-merge (squash) October 15, 2025 18:56
@gibson042 gibson042 merged commit 687cf88 into tc39:main Oct 15, 2025
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants