Skip to content

Intl.NumberFormat: Add latest options, fix previous library discrepancies - #56902

Merged
Nathan Shively-Sanders (sandersn) merged 23 commits into
microsoft:mainfrom
Renegade334:intl-numberformat
Mar 1, 2024
Merged

Intl.NumberFormat: Add latest options, fix previous library discrepancies#56902
Nathan Shively-Sanders (sandersn) merged 23 commits into
microsoft:mainfrom
Renegade334:intl-numberformat

Conversation

@Renegade334

@Renegade334René (Renegade334) commented Dec 30, 2023

Copy link
Copy Markdown
Contributor

Fixes#56269
Fixes#52072
Fixes#43336

  • A few historic deviations from the relevant versions of the Intl specification have been addressed.
  • The latest changes are based on ECMA-402 10.0, which accompanied the ECMAScript 2023 language specification. Accordingly, I've created a new es2023.intl to house them. The declarations currently residing in esnext.intl were also from the same version of the spec, so I've moved these across as well.
  • Properties originally typed as string, but defined in the spec as a union of string literals, have been narrowed accordingly. Where the range of acceptable values for a string literal type have been expanded in subsequent versions of the spec, the types are now defined as keyof <RegistryInterface> in order to make them extensible, as per precedent. This is technically breaking, and will likely need to be tested against high-exposure libraries for side-effects.
  • useGrouping is a bit of a mess, as the property's primitive type was changed in version 10. The inclusion of "true" and "false" as acceptable values in the es2023 implementation is intentional, as per the specification.

@typescript-botTypeScript Bot (typescript-bot) added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Dec 30, 2023
@Renegade334René (Renegade334) changed the title Intl numberformatIntl.NumberFormat: Add latest options, fix previous library discrepanciesDec 30, 2023
Comment threadsrc/lib/es5.d.ts Outdated
@Renegade334

Copy link
Copy Markdown
ContributorAuthor

This should be ready for review.

Context for some less-obvious changes:

  • lib.es5
    • NumberFormatOptionsUseGrouping and ResolvedNumberFormatOptionsUseGrouping are typed conditionally. This is to satisfy the following change in the v10 spec:
      typeNumberFormatOptionsUseGrouping=/* es5 */boolean;/* es2023 */keyofNumberFormatOptionsUseGroupingRegistry|"true"|"false"|boolean;typeResolvedNumberFormatOptionsUseGrouping=/* es5 */boolean;/* es2023 */keyofNumberFormatOptionsUseGroupingRegistry|false;
      Tests under conformance/es2023 confirm these to be working appropriately.
  • lib.es2018.intl
    • The values "code", "symbol" and "name" were previously erroneously included in NumberFormatPartTypes; this was likely a misreading of the specification.
  • lib.es2020.intl
    • Changing the notation and signDisplay properties of ResolvedNumberFormatOptions from optional to required is an intentional change.
  • tests
    • The tests included in formatToPartsBigInt were already included under bigIntWithLib, hence its removal.
    • DateTimeFormatAndNumberFormatES2021 tested for NumberFormat methods that didn't exist in ES2021, and the baseline for this test contained errors accordingly. The test scope has been narrowed to DateTimeFormat and moved to conformance/es2021, while testing NumberFormat#formatRange has been moved to conformance/es2023.

I'd be grateful if someone could run top100/dt when convenient.

@Renegade334
René (Renegade334) marked this pull request as ready for review January 2, 2024 11:04
@typescript-bot

Copy link
Copy Markdown
Contributor

This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise.

@typescript-botTypeScript Bot (typescript-bot) added For Backlog Bug PRs that fix a backlog bug and removed For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Jan 2, 2024
@typescript-bot

TypeScript Bot (typescript-bot) commented Jan 4, 2024

Copy link
Copy Markdown
Contributor

Heya Jake Bailey (@jakebailey), I've started to run the diff-based user code test suite on this PR at 90b7d94. You can monitor the build here.

Update: The results are in!

@typescript-bot

TypeScript Bot (typescript-bot) commented Jan 4, 2024

Copy link
Copy Markdown
Contributor

Heya Jake Bailey (@jakebailey), I've started to run the tarball bundle task on this PR at 90b7d94. You can monitor the build here.

@typescript-bot

TypeScript Bot (typescript-bot) commented Jan 4, 2024

Copy link
Copy Markdown
Contributor

Hey Jake Bailey (@jakebailey), I've packed this into an installable tgz. You can install it for testing by referencing it in your package.json like so:

{
"devDependencies": {
"typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/159246/artifacts?artifactName=tgz&fileId=576D53D334DC37587E8C3B2DF7A4DA90B4DE69F90ABC032E783942C033EC773D02&fileName=/typescript-5.4.0-insiders.20240104.tgz"
}
}

and then running npm install.


There is also a playground for this build and an npm module you can use via "typescript": "npm:@typescript-deploys/pr-build@5.4.0-pr-56902-8".;

@typescript-bot

Copy link
Copy Markdown
Contributor

Jake Bailey (@jakebailey)
The results of the perf run you requested are in!

Here they are:

Compiler

Comparison Report - baseline..pr
MetricbaselineprDeltaBestWorstp-value
Angular - node (v18.15.0, x64)
Memory used295,465k (± 0.02%)295,506k (± 0.01%)~295,445k295,563kp=0.173 n=6
Parse Time2.65s (± 0.21%)2.65s (± 0.15%)~2.64s2.65sp=0.282 n=6
Bind Time0.82s (± 0.50%)0.82s (± 0.50%)~0.81s0.82sp=0.218 n=6
Check Time8.17s (± 0.34%)8.15s (± 0.51%)~8.11s8.22sp=0.171 n=6
Emit Time7.11s (± 0.41%)7.10s (± 0.33%)~7.07s7.14sp=0.517 n=6
Total Time18.75s (± 0.29%)18.71s (± 0.29%)~18.66s18.78sp=0.195 n=6
Compiler-Unions - node (v18.15.0, x64)
Memory used193,494k (± 1.57%)193,498k (± 1.56%)~191,490k197,407kp=0.471 n=6
Parse Time1.34s (± 0.87%)1.35s (± 0.94%)~1.33s1.37sp=0.245 n=6
Bind Time0.72s (± 0.00%)0.72s (± 0.00%)~0.72s0.72sp=1.000 n=6
Check Time9.26s (± 0.33%)9.26s (± 0.48%)~9.19s9.30sp=1.000 n=6
Emit Time2.61s (± 0.29%)2.61s (± 0.40%)~2.60s2.63sp=0.611 n=6
Total Time13.93s (± 0.20%)13.95s (± 0.27%)~13.90s13.98sp=0.517 n=6
Monaco - node (v18.15.0, x64)
Memory used347,406k (± 0.00%)347,400k (± 0.01%)~347,362k347,425kp=0.936 n=6
Parse Time2.46s (± 0.48%)2.46s (± 0.51%)~2.44s2.47sp=0.801 n=6
Bind Time0.92s (± 0.59%)0.93s (± 0.44%)~0.92s0.93sp=0.282 n=6
Check Time6.87s (± 0.36%)6.89s (± 0.38%)~6.87s6.94sp=0.286 n=6
Emit Time4.06s (± 0.39%)4.06s (± 0.30%)~4.04s4.07sp=0.803 n=6
Total Time14.31s (± 0.23%)14.33s (± 0.14%)~14.31s14.36sp=0.220 n=6
TFS - node (v18.15.0, x64)
Memory used302,730k (± 0.00%)302,733k (± 0.01%)~302,704k302,754kp=0.936 n=6
Parse Time1.99s (± 0.84%)2.01s (± 1.16%)~1.98s2.04sp=0.118 n=6
Bind Time1.00s (± 0.41%)1.00s (± 1.17%)~0.99s1.02sp=0.584 n=6
Check Time6.32s (± 0.29%)6.29s (± 0.50%)~6.26s6.33sp=0.140 n=6
Emit Time3.58s (± 0.42%)3.59s (± 0.45%)~3.57s3.61sp=0.934 n=6
Total Time12.89s (± 0.14%)12.89s (± 0.14%)~12.86s12.91sp=0.685 n=6
material-ui - node (v18.15.0, x64)
Memory used506,833k (± 0.01%)506,830k (± 0.01%)~506,800k506,872kp=0.936 n=6
Parse Time2.58s (± 0.45%)2.58s (± 0.32%)~2.58s2.60sp=0.547 n=6
Bind Time1.00s (± 0.84%)0.99s (± 0.76%)~0.98s1.00sp=0.209 n=6
Check Time16.97s (± 0.51%)16.95s (± 0.48%)~16.83s17.03sp=0.936 n=6
Emit Time0.00s (± 0.00%)0.00s (± 0.00%)~0.00s0.00sp=1.000 n=6
Total Time20.55s (± 0.39%)20.52s (± 0.39%)~20.42s20.60sp=0.872 n=6
xstate - node (v18.15.0, x64)
Memory used512,867k (± 0.01%)512,971k (± 0.01%)+104k (+ 0.02%)512,925k513,053kp=0.013 n=6
Parse Time3.27s (± 0.30%)3.28s (± 0.31%)~3.26s3.29sp=0.203 n=6
Bind Time1.54s (± 0.36%)1.54s (± 0.27%)~1.53s1.54sp=0.282 n=6
Check Time2.81s (± 1.20%)2.84s (± 0.35%)~2.83s2.85sp=0.102 n=6
Emit Time0.07s (± 0.00%)0.07s (± 5.69%)~0.07s0.08sp=0.405 n=6
Total Time7.70s (± 0.37%)7.73s (± 0.17%)~7.71s7.75sp=0.062 n=6
System info unknown
Hosts
  • node (v18.15.0, x64)
Scenarios
  • Angular - node (v18.15.0, x64)
  • Compiler-Unions - node (v18.15.0, x64)
  • Monaco - node (v18.15.0, x64)
  • TFS - node (v18.15.0, x64)
  • material-ui - node (v18.15.0, x64)
  • xstate - node (v18.15.0, x64)
BenchmarkNameIterations
Currentpr6
Baselinebaseline6

tsserver

Comparison Report - baseline..pr
MetricbaselineprDeltaBestWorstp-value
Compiler-UnionsTSServer - node (v18.15.0, x64)
Req 1 - updateOpen2,348ms (± 0.66%)2,344ms (± 0.74%)~2,319ms2,367msp=0.936 n=6
Req 2 - geterr5,393ms (± 0.57%)5,410ms (± 1.16%)~5,364ms5,531msp=1.000 n=6
Req 3 - references324ms (± 1.14%)324ms (± 1.15%)~320ms329msp=0.746 n=6
Req 4 - navto277ms (± 0.42%)275ms (± 1.01%)~270ms277msp=0.079 n=6
Req 5 - completionInfo count1,356 (± 0.00%)1,356 (± 0.00%)~1,3561,356p=1.000 n=6
Req 5 - completionInfo86ms (± 3.20%)86ms (± 6.81%)~79ms94msp=0.864 n=6
CompilerTSServer - node (v18.15.0, x64)
Req 1 - updateOpen2,471ms (± 0.98%)2,489ms (± 1.11%)~2,445ms2,523msp=0.230 n=6
Req 2 - geterr4,183ms (± 2.02%)4,166ms (± 2.15%)~4,079ms4,262msp=0.575 n=6
Req 3 - references335ms (± 1.13%)337ms (± 1.17%)~332ms343msp=0.293 n=6
Req 4 - navto286ms (± 1.14%)284ms (± 0.52%)~282ms286msp=0.807 n=6
Req 5 - completionInfo count1,518 (± 0.00%)1,518 (± 0.00%)~1,5181,518p=1.000 n=6
Req 5 - completionInfo81ms (± 7.36%)84ms (± 7.55%)~75ms90msp=0.520 n=6
xstateTSServer - node (v18.15.0, x64)
Req 1 - updateOpen2,602ms (± 0.93%)2,592ms (± 0.75%)~2,564ms2,610msp=0.261 n=6
Req 2 - geterr1,726ms (± 1.87%)1,707ms (± 2.15%)~1,672ms1,763msp=0.575 n=6
Req 3 - references106ms (± 7.58%)109ms (± 9.53%)~101ms123msp=0.868 n=6
Req 4 - navto364ms (± 1.02%)366ms (± 0.14%)~365ms366msp=0.070 n=6
Req 5 - completionInfo count2,073 (± 0.00%)2,073 (± 0.00%)~2,0732,073p=1.000 n=6
Req 5 - completionInfo310ms (± 1.72%)305ms (± 1.83%)~296ms312msp=0.170 n=6
System info unknown
Hosts
  • node (v18.15.0, x64)
Scenarios
  • CompilerTSServer - node (v18.15.0, x64)
  • Compiler-UnionsTSServer - node (v18.15.0, x64)
  • xstateTSServer - node (v18.15.0, x64)
BenchmarkNameIterations
Currentpr6
Baselinebaseline6

Startup

Comparison Report - baseline..pr
MetricbaselineprDeltaBestWorstp-value
tsc-startup - node (v18.15.0, x64)
Execution time153.69ms (± 0.21%)153.59ms (± 0.20%)-0.10ms (- 0.07%)152.45ms156.75msp=0.005 n=600
tsserver-startup - node (v18.15.0, x64)
Execution time228.69ms (± 0.16%)228.46ms (± 0.15%)-0.23ms (- 0.10%)227.05ms231.45msp=0.000 n=600
tsserverlibrary-startup - node (v18.15.0, x64)
Execution time230.23ms (± 0.18%)230.33ms (± 0.19%)+0.11ms (+ 0.05%)228.41ms233.50msp=0.024 n=600
typescript-startup - node (v18.15.0, x64)
Execution time230.60ms (± 0.19%)230.49ms (± 0.18%)-0.10ms (- 0.04%)228.91ms234.79msp=0.011 n=600
System info unknown
Hosts
  • node (v18.15.0, x64)
Scenarios
  • tsc-startup - node (v18.15.0, x64)
  • tsserver-startup - node (v18.15.0, x64)
  • tsserverlibrary-startup - node (v18.15.0, x64)
  • typescript-startup - node (v18.15.0, x64)
BenchmarkNameIterations
Currentpr6
Baselinebaseline6

Developer Information:

Download Benchmarks

@typescript-bot

Copy link
Copy Markdown
Contributor

Jake Bailey (@jakebailey) Here are the results of running the user test suite comparing main and refs/pull/56902/merge:

There were infrastructure failures potentially unrelated to your change:

  • 1 instance of "Package install failed"

Otherwise...

Something interesting changed - please have a look.

Details

puppeteer

packages/browsers/test/src/tsconfig.json

@typescript-bot

Copy link
Copy Markdown
Contributor

Hey Jake Bailey (@jakebailey), the results of running the DT tests are ready.
Everything looks the same!
You can check the log here.

@typescript-bot

Copy link
Copy Markdown
Contributor

Jake Bailey (@jakebailey) Here are the results of running the top-repos suite comparing main and refs/pull/56902/merge:

Everything looks good!

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think this is good to go, although I'd like to wait until after the beta.

The only question I have is about any as the property type for registries. Everything else is notes for myself about the combined members of registries.

Comment threadsrc/lib/es2018.intl.d.ts Outdated
const PluralRules: PluralRulesConstructor;

// We can only have one definition for 'type' in TypeScript, and so you can learn where the keys come from here:
type ES2018NumberFormatPartType = "literal" | "nan" | "infinity" | "percent" | "integer" | "group" | "decimal" | "fraction" | "plusSign" | "minusSign" | "percentSign" | "currency" | "code" | "symbol" | "name";

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

NumberFormatPartTypeRegistry

ES2018

  • literal
  • nan
  • infinity
  • percent
  • integer
  • group
  • decimal
  • fraction
  • plusSign
  • minusSign
  • percentSign
  • currency

ES2020

  • compact
  • exponentInteger
  • exponentMinusSign
  • exponentSeparator
  • unit
  • unknown

declare namespace Intl {
interface NumberFormat {
format(value: number | bigint): string;
resolvedOptions(): ResolvedNumberFormatOptions;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

duplicated with entry in es5, doesn't need to be here

exceptZero: any;
}

type NumberFormatOptionsSignDisplay = keyof NumberFormatOptionsSignDisplayRegistry;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ES2020

  • auto
  • never
  • always
  • exceptZero

ES2023

  • negative

Comment threadsrc/lib/es5.d.ts
currency: any;
}

type NumberFormatOptionsStyle = keyof NumberFormatOptionsStyleRegistry;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ES5

  • decimal
  • percent
  • currency

ES2020

  • unit

Comment threadsrc/lib/es5.d.ts
name: any;
}

type NumberFormatOptionsCurrencyDisplay = keyof NumberFormatOptionsCurrencyDisplayRegistry;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ES5

  • code
  • symbol
  • name

ES2020

  • narrowSymbol

Comment threadsrc/lib/es5.d.ts

type NumberFormatOptionsCurrencyDisplay = keyof NumberFormatOptionsCurrencyDisplayRegistry;

interface NumberFormatOptionsUseGroupingRegistry {}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ES2023

  • min2
  • auto
  • always

@Renegade334

René (Renegade334) commented Feb 2, 2024

Copy link
Copy Markdown
ContributorAuthor

Updated the registry interfaces as discussed.

I've also included one other minor change. The defined behaviour of the resolved options {minimum,maximum}FractionDigits changed subtly in v10: whereas these properties were always included under ResolvedNumberFormatOptions in previous versions, they're now omitted under certain circumstances. For simplicity, these are now typed as optional throughout. Although unlikely to break anything, this probably warrants re-running the test suite.

@sandersn

Copy link
Copy Markdown
Member

@typescript-bot

TypeScript Bot (typescript-bot) commented Feb 16, 2024

Copy link
Copy Markdown
Contributor

Heya Nathan Shively-Sanders (@sandersn), I've started to run the parallelized Definitely Typed test suite on this PR at 254a8ce. You can monitor the build here.

Update: The results are in!

@typescript-bot

TypeScript Bot (typescript-bot) commented Feb 16, 2024

Copy link
Copy Markdown
Contributor

Heya Nathan Shively-Sanders (@sandersn), I've started to run the diff-based user code test suite on this PR at 254a8ce. You can monitor the build here.

Update: The results are in!

@typescript-bot

TypeScript Bot (typescript-bot) commented Feb 16, 2024

Copy link
Copy Markdown
Contributor

Heya Nathan Shively-Sanders (@sandersn), I've started to run the diff-based top-repos suite on this PR at 254a8ce. You can monitor the build here.

Update: The results are in!

@typescript-bot

Copy link
Copy Markdown
Contributor

Nathan Shively-Sanders (@sandersn) Here are the results of running the user test suite comparing main and refs/pull/56902/merge:

There were infrastructure failures potentially unrelated to your change:

  • 1 instance of "Package install failed"

Otherwise...

Something interesting changed - please have a look.

Details

puppeteer

packages/browsers/test/src/tsconfig.json

@typescript-bot

Copy link
Copy Markdown
Contributor

Hey Nathan Shively-Sanders (@sandersn), the results of running the DT tests are ready.
Everything looks the same!
You can check the log here.

@typescript-bot

Copy link
Copy Markdown
Contributor

Nathan Shively-Sanders (@sandersn) Here are the results of running the top-repos suite comparing main and refs/pull/56902/merge:

Everything looks good!

@felixfbecker

Copy link
Copy Markdown
Contributor

Can't wait for this to land!

@sandersn
Nathan Shively-Sanders (sandersn) merged commit 877d9d3 into microsoft:mainMar 1, 2024
Nathan Shively-Sanders (sandersn) added a commit to sandersn/TypeScript that referenced this pull request Mar 1, 2024
I forgot to check whether CI had run recently when I merged, and this PR
adds a new section to every resolution trace baseline.
@Renegade334
René (Renegade334) deleted the intl-numberformat branch March 2, 2024 18:31
@microsoftMicrosoft (microsoft) locked as resolved and limited conversation to collaborators Oct 16, 2025
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

For Backlog BugPRs that fix a backlog bug

Projects

Archived in project

5 participants

@Renegade334@typescript-bot@jakebailey@sandersn@felixfbecker