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

Normative: Revert to previous behaviour by setting fallback value for fractionalDigits to *undefined* #150

Merged
merged 4 commits into from
Aug 3, 2023

Conversation

ben-allen
Copy link
Collaborator

Fix #149 Fix #144

@sffc
Copy link
Collaborator

sffc commented Jun 2, 2023

Two issues:

  1. Please note in "Properties of Intl.DurationFormat Instances" that the field can be undefined.
  2. We should pass 0/9 into Intl.NumberFormat instead of undefined/undefined in PartitionDurationFormatPattern

@anba
Copy link
Contributor

anba commented Jun 5, 2023

Intl.DurationFormat.prototype.resolvedOptions currently asserts that all internal slots have a non-undefined value. This is no longer true if [[FractionalDigits]] can be undefined.

I guess an undefined [[FractionalDigits]] should be omitted from the output for consistency with the other resolvedOptions methods, even though undefined for [[FractionalDigits]] has a different meaning when compared to undefined internal slots for the other Intl service constructors.

@anba
Copy link
Contributor

anba commented Jun 5, 2023

I also wonder if the entries in Resolved Options of DurationFormat Instances should be reordered if an undefined [[FractionalDigits]] is omitted, so that the optional [[FractionalDigits]] appears last. For example by moving [[NumberingSystem]] to the top, see also Resolved Options of DateTimeFormat Instances and Resolved Options of NumberFormat Instances.

Copy link
Member

@ryzokuken ryzokuken left a comment

Choose a reason for hiding this comment

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

There was also the suggestion of reordering the resolvedOptions, but I think it's something we can build consensus on this evening.

@FrankYFTang
Copy link
Collaborator

This PR reach TC39 consensus in July 12, 2023

@ryzokuken
Copy link
Member

Thanks for the update @FrankYFTang! I'll rebase and merge later this week.

…ged table in resolvedOptions to put optional FractionalDigits last and non-optional NumberingSystem first, change to pass 0/9 rather than undefined/undefined into Intl.NumberFormat for minimumFractionDigits/maximumFractionDigits
…ctionalDigits]] potentially being *undefined*
@ryzokuken ryzokuken merged commit f8828ab into tc39:main Aug 3, 2023
@FrankYFTang
Copy link
Collaborator

Could we add some test to test262 to validate the change of this PR?

@Josh-Cena Josh-Cena mentioned this pull request Aug 16, 2023
ben-allen added a commit to ben-allen/proposal-intl-duration-format that referenced this pull request Aug 21, 2023
ben-allen added a commit to ben-allen/proposal-intl-duration-format that referenced this pull request Aug 21, 2023
@FrankYFTang
Copy link
Collaborator

@sffc @ben-allen @ryzokuken
I do not belive the part of reordering the order of resolvedOptions to move "numberingSystem" to the top reach any consensus. The title of this PR has nothing to do with that and somehow that reordering to merged under this PR without TG2 discussion nor TG1 consensus. Please revert that change.

@ben-allen this PR is already merged to the trunk, you merge your restoration into your branch of this PR after the merging which will not fix the trunk.

The landing of this PR seems very very troublesome . it include part which we didn't discuss

The landing of this PR includes 4 commits according to
https://github.com/tc39/proposal-intl-duration-format/pull/150/commits

df63a06
4b9a46a
e4774b0
I believe these 3 are ok

32950ea
this is a troublesome change, it chagne two thing- it perform some necessary change for the above 3 commits but ALSO include the reordering change of numberingSystem in the resolvedOption
this later was never communicate and agree upon TG2 members and never mentioned to TG1 members.

I beleve our edtior should revert that part of the change (the order of numberingSystem in resolvedOption should not be on the top ASAP since that was a technical error for merging something we never agree upon. If we in deed want to reoder that. That need a Normative PR to be discuss in TG2 and also TG1.

ben-allen added a commit to ben-allen/proposal-intl-duration-format that referenced this pull request Aug 28, 2023
FrankYFTang added a commit that referenced this pull request Aug 28, 2023
restore Resolved Options table to ordering before PR #150 merged
ben-allen added a commit to ben-allen/proposal-intl-duration-format that referenced this pull request Sep 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Meeting Discussion Need to be discussed in one of the upcoming meetings normative
Projects
None yet
5 participants