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

Temporal: Add additional coverage based on issues reported by Anba #4248

Merged
merged 7 commits into from
Oct 7, 2024

Conversation

ptomato
Copy link
Contributor

@ptomato ptomato commented Oct 4, 2024

Best reviewed commit by commit.

cc @anba

const plainTime = new Temporal.PlainTime(12, 23, 37, 847);

const dateStyleFormatter = new Intl.DateTimeFormat(undefined, { dateStyle: "short", calendar: "iso8601", timeZone: "America/Vancouver" });
const dateStyleResult = JSON.stringify(dateStyleFormatter.formatRangeToParts(legacyDate, legacyDate));
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the serialization order guaranteed (or reliable enough?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's "ascending chronological order of property creation" and the order in which type, value, and source are created is specified, so I think we're good.

Copy link
Contributor

@Ms2ger Ms2ger left a comment

Choose a reason for hiding this comment

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

Thanks!

…ntifiers

This was an area missing test coverage.
Syntax is validated first. Only after the property bag is fully converted
into a Calendar Fields Record does the calendar validate whether the month
code actually exists in the year.

See tc39/proposal-temporal#2962
…lity

Syntax is validated first. Only after the property bag is fully converted
into a Calendar Fields Record does the time zone validate whether the UTC
offset is correct for that exact time in the time zone.

See tc39/proposal-temporal#2962
…one component

Adds a test for each Temporal object's toLocaleString() method, formatting
them with only one option e.g. { year: 'numeric' } and comparing it with
the corresponding output for legacy Date.

See tc39/proposal-temporal#2796.
For each Temporal object, add tests for what components are present by
default if no options for date or time components are passed.
These staging tests are incorrect. See tc39/proposal-temporal#2795. This
was an unintended behaviour. It differed from the behaviour for dateStyle
and timeStyle, which was the intended behaviour.
…s and Temporal object

See tc39/proposal-temporal#2795. When attempting to format a Temporal
object, if the DateTimeFormat has options that do not overlap with the
data model of the Temporal object, toLocaleString() and format() are
supposed to throw a TypeError.
@Ms2ger Ms2ger enabled auto-merge (rebase) October 7, 2024 10:00
@Ms2ger Ms2ger merged commit e2f2f92 into tc39:main Oct 7, 2024
8 checks passed
@ptomato ptomato deleted the temporal-coverage branch October 7, 2024 15:03
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