Skip to content

Editorial: Fix assertion failures and streamline NonISOMonthDayToISOReferenceDate#124

Merged
ptomato merged 9 commits intotc39:mainfrom
ptomato:123-assertions-adjustment
Mar 5, 2026
Merged

Editorial: Fix assertion failures and streamline NonISOMonthDayToISOReferenceDate#124
ptomato merged 9 commits intotc39:mainfrom
ptomato:123-assertions-adjustment

Conversation

@ptomato
Copy link
Collaborator

@ptomato ptomato commented Mar 4, 2026

Thanks @fabon-f for pointing these out.

Fixing them made the chinese/dangi special cases in NonISOMonthDayToISOReferenceDate look a lot more like the non-chinese/dangi case, so it actually made sense to combine them, making the result much more readable.

Closes: #123

ptomato added 2 commits March 3, 2026 16:14
It's possible for _fields_.[[Month]] to still be UNSET at the end of
this operation, if the property bag contained `monthCode` and `day`.

h/t fabon-f

See: tc39#123
…Date

Similar to the previous commit, it's possible to get here without
_fields_.[[Month]] if the property bag contained `monthCode` and `day`.

h/t fabon-f
@ptomato
Copy link
Collaborator Author

ptomato commented Mar 4, 2026

See discussion in #123 for description of the individual assertion failures solved by each commit.

Once reviewed I'll merge this but also port it over to tc39/ecma402#1044.

ptomato added 7 commits March 4, 2026 10:29
Similar to tc39#122, the change there must also apply here. Editorial
because otherwise we fail an assertion.

h/t fabon-f
This needs to parallel the non-chinese/dangi case.
In this step we only need the month code, so there's no need to use the
given day (which may be 30 even in a 29-day month, in which case the
normal completion assertion would fail.)

Make the same change in the non-chinese/dangi case even though it's not
strictly necessary.

h/t fabon-f
This makes the chinese/dangi and non-chinese/dangi clauses mirror each
other a bit better.
Makes the behaviour of the chinese/dangi clause correspond to the
non-chinese/dangi clause. This fixes an unintended regression from tc39#118.

h/t fabon-f
After having fixed the assertion failures etc. in the previous commits,
these two clauses look a lot more alike. It's possible to combine them,
only special-casing two things for chinese/dangi: the maximum number 30
of days in month, and the extra lookup in the table to handle rare
month codes.

Closes: tc39#123
I find the "If X is not UNSET, Y is not UNSET" hard to reason about. We
already have an if-clause that covers these two code paths, so we can
move the assertions into the beginning of the if and else branches.
@ptomato ptomato force-pushed the 123-assertions-adjustment branch from 92aa562 to 3281a87 Compare March 4, 2026 18:40
@ptomato ptomato requested review from ben-allen, fabon-f, gibson042 and sffc and removed request for fabon-f March 4, 2026 18:46
@ptomato
Copy link
Collaborator Author

ptomato commented Mar 5, 2026

Based on fabon-f's check in the polyfill, going to merge this and move it into tc39/ecma402#1044.

@ptomato ptomato merged commit 10d078f into tc39:main Mar 5, 2026
@ptomato ptomato deleted the 123-assertions-adjustment branch March 5, 2026 23:58
ptomato added a commit to ptomato/ecma402 that referenced this pull request Mar 6, 2026
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.

spec: alleged issues about Temporal.PlainMonthDay construction from fields

2 participants