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

Re-expose Intl.NumberFormat.formatToParts #160

Merged
merged 2 commits into from
Sep 28, 2017

Conversation

stasm
Copy link
Contributor

@stasm stasm commented Aug 8, 2017

Intl.NumberFormat.formatToParts was first propsed in #30. The spec for it was created in #79 and merged in #100 (with follow-ups). Due to browser implementations not being ready at the time, it was moved back to Stage 3 in #101. The internal refactoring were kept in master and the user-facing method formatToParts was removed from the spec in #102.

As of August 1st, 2017, Intl.NumberFormat.prototype.formatToParts has shipped in two engines (behind a flag): SpiderMonkey and V8. This PR brings Intl.NumberFormat.formatToParts back as Stage 4 proposal.

> const usd = Intl.NumberFormat('en', { style: 'currency', currency: 'USD' });
> usd.format(123456.789)
'$123,456.79'
> usd.formatToParts(123456.789)
[ { type: 'currency', value: '$' },
  { type: 'integer', value: '123' },
  { type: 'group', value: ',' },
  { type: 'integer', value: '456' },
  { type: 'decimal', value: '.' },
  { type: 'fraction', value: '79' } ]

> const pc = Intl.NumberFormat('en', { style: 'percent', minimumFractionDigits: 2 })
> pc.format(-0.123456)
'-12.35%'
> pc.formatToParts(-0.123456)
[ { type: 'minusSign', value: '-' },
  { type: 'integer', value: '12' },
  { type: 'decimal', value: '.' },
  { type: 'fraction', value: '35' },
  { type: 'literal', value: '%' } ]

`Intl.NumberFormat.formatToParts` was first propsed in tc39#30. The spec for
it was created in tc39#79 and merged in tc39#100 (with follow-ups). Due to
browser implementations not being ready at the time, it was moved back
to Stage 3 in tc39#101.  The internal refactoring were kept in master and
the user-facing method `formatToParts` was removed from the spec in tc39#102.

As of August 1st, 2017, `Intl.NumberFormat.prototype.formatToParts` has
shipped in two engines (behind a flag): SpiderMonkey and V8.  This PR
brings `Intl.NumberFormat.formatToParts` back as Stage 4 proposal.

    > const usd = Intl.NumberFormat('en', { style: 'currency', currency: 'USD' });
    > usd.format(123456.789)
    '$123,456.79'
    > usd.formatToParts(123456.789)
    [ { type: 'currency', value: '$' },
      { type: 'integer', value: '123' },
      { type: 'group', value: ',' },
      { type: 'integer', value: '456' },
      { type: 'decimal', value: '.' },
      { type: 'fraction', value: '79' } ]

    > const pc = Intl.NumberFormat('en', { style: 'percent', minimumFractionDigits: 2 })
    > pc.format(-0.123456)
    '-12.35%'
    > pc.formatToParts(-0.123456)
    [ { type: 'minusSign', value: '-' },
      { type: 'integer', value: '12' },
      { type: 'decimal', value: '.' },
      { type: 'fraction', value: '35' },
      { type: 'literal', value: '%' } ]
@zbraniecki
Copy link
Member

@caridy - can you review this? This PR looks good to me :)

@@ -618,6 +618,23 @@
</emu-alg>
</emu-clause>

<emu-clause id="sec-intl.numberformat.prototype.formattoparts">
<h1>Intl.NumberFormat.prototype.formatToParts ( [ _value_ ] )</h1>
Copy link
Contributor

@caridy caridy Aug 8, 2017

Choose a reason for hiding this comment

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

in the DateTimeFormat.prototype.formatToParts we decided to make the argument non-optional. https://tc39.github.io/ecma402/#sec-Intl.DateTimeFormat.prototype.formatToParts, this should do the same. /cc @littledan

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. Should there be an explicit check for the existence of the argument, too?

@caridy
Copy link
Contributor

caridy commented Aug 9, 2017

@zbraniecki @stasm can you guys coordinate the tests needed for this? Let's aim for the next meeting to get this to stage 4, by that time, we need to get test262 to include this one.

@zbraniecki
Copy link
Member

@littledan
Copy link
Member

Yeah, I think this should be good to land as soon as it gets officially to Stage 4. LGTM at that point.

@domenic has suggested in the past allowing things to reach Stage 4 outside of actual meetings, based on meeting all the requirements and discussion online. However, we didn't reach agreement on doing this.

@zbraniecki
Copy link
Member

This has reached Stage 4 today. I believe we can merge it now :)

zbraniecki added a commit to zbraniecki/test262 that referenced this pull request Sep 28, 2017
<emu-alg>
1. Let _nf_ be *this* value.
1. If Type(_nf_) is not Object, throw a *TypeError* exception.
1. If _nf_ does not have an [[initializedNumberFormat]] internal slot, throw a *TypeError* exception.
Copy link
Contributor

Choose a reason for hiding this comment

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

one minor editorial detail, initializedNumberFormat => InitializedNumberFormat now that internal slots are camel case. I can probably take care of that directly.

Copy link
Contributor

Choose a reason for hiding this comment

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

done 675495b

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, @caridy!

@caridy caridy merged commit fe7b0a4 into tc39:master Sep 28, 2017
@stasm stasm deleted the NumberFormat.formatToParts-Stage4 branch September 29, 2017 08:40
leobalter pushed a commit to tc39/test262 that referenced this pull request Oct 2, 2017
@littledan
Copy link
Member

Yay!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants