Skip to content

fix: revamp es2020.intl.d.ts - #42945

Closed
Long Ho (longlho) wants to merge 1 commit into
microsoft:mainfrom
longlho:patch-1
Closed

fix: revamp es2020.intl.d.ts#42945
Long Ho (longlho) wants to merge 1 commit into
microsoft:mainfrom
longlho:patch-1

Conversation

@longlho

@longlho Long Ho (longlho) commented Feb 24, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor

fractionalSecondDigits is in ES2021 (unpublished), not ES2020

  • Remove fractionalSecondDigits, dateStyle, timeStyle, dayPeriod since they're in ES2021 draft, not ES2020.
  • Clarify NumberFormatOptions, add list of sanctioned unit.

Fixes #42944

@typescript-bot TypeScript Bot (typescript-bot) added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Feb 24, 2021
@longlho Long Ho (longlho) mentioned this pull request Feb 24, 2021
Closed
@DanielRosenwasser

Daniel Rosenwasser (DanielRosenwasser) commented Feb 24, 2021

Copy link
Copy Markdown
Member

Was this in TS4.1? If not, maybe we should revert for TS4.2 Ryan Cavanaugh (@RyanCavanaugh)

@longlho

Long Ho (longlho) commented Feb 24, 2021

Copy link
Copy Markdown
Contributor Author

I believe this was in 4.2 but not 4.1. Our workaround rn is to pin to 4.1

@longlho Long Ho (longlho) changed the title fix: remove fractionalSecondDigits from es2020.intl.d.ts fix: revamp es2020.intl.d.ts Feb 25, 2021
@longlho

Long Ho (longlho) commented Feb 25, 2021

Copy link
Copy Markdown
Contributor Author

Daniel Rosenwasser (@DanielRosenwasser) I apologize for increasing the scope of this PR but es2020 at its current state has some ES2021 stuff and also is missing some constraints in terms of unit.

@longlho
fix: revamp es2020.intl.d.ts
a4f4583 
- Remove `fractionalSecondDigits`, `dateStyle`, `timeStyle`, `dayPeriod`
since they're in ES2021 draft, not ES2020.
- Clarify NumberFormatOptions, add list of sanctioned unit
@longlho

Long Ho (longlho) commented Mar 3, 2021

Copy link
Copy Markdown
Contributor Author

gentle ping Ryan Cavanaugh (@RyanCavanaugh) :)

@sandersn

Nathan Shively-Sanders (sandersn) commented Mar 4, 2021

Copy link
Copy Markdown
Member

Maybe it was a related change, but I thought we had already made the types of something in intl into precise literal unions and then had to revert to the base primitive. Finding that change is the first step to reviewing this one, to make sure we don't end up waffling back and forth.

@longlho

Long Ho (longlho) commented Mar 4, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Nathan Shively-Sanders (@sandersn) so the context here is different and shouldn't be treated as blanket literal union vs none. The literal unions for those values are fine because the spec indicates that values outside of those choices, as of ES2020, will throw a RangeError. Specifically in this spec: https://402.ecma-international.org/7.0/#sec-initializenumberformat

Step 7 says numberingSystem is a string with no limited set of values, default to undefined (that's what GetOption does). This is why literal union change was reverted because the spec has no limit on sets of values. The previous union was pulled from LDML UTS#35 which is a separate spec.
However, step 19 says notation must be a string from a set of 4 values « "standard", "scientific", "engineering", "compact" », and default to standard. This is where literal union makes sense.

@longlho Long Ho (longlho) mentioned this pull request Apr 10, 2021
Closed
@DanielRosenwasser

Daniel Rosenwasser (DanielRosenwasser) commented May 10, 2021

Copy link
Copy Markdown
Member

TypeScript Bot (@typescript-bot) test this
TypeScript Bot (@typescript-bot) test dt
TypeScript Bot (@typescript-bot) user test this

@typescript-bot

TypeScript Bot (typescript-bot) commented May 10, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor

Heya Daniel Rosenwasser (@DanielRosenwasser), I've started to run the extended test suite on this PR at a4f4583. You can monitor the build here.

@typescript-bot

TypeScript Bot (typescript-bot) commented May 10, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor

Heya Daniel Rosenwasser (@DanielRosenwasser), I've started to run the parallelized community code test suite on this PR at a4f4583. You can monitor the build here.

@DanielRosenwasser

Daniel Rosenwasser (DanielRosenwasser) commented May 10, 2021

Copy link
Copy Markdown
Member

TypeScript Bot (@typescript-bot) run dt

@typescript-bot

TypeScript Bot (typescript-bot) commented May 10, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor

Heya Daniel Rosenwasser (@DanielRosenwasser), I've started to run the parallelized Definitely Typed test suite on this PR at a4f4583. You can monitor the build here.

Open
@typescript-bot

TypeScript Bot (typescript-bot) commented May 10, 2021

Copy link
Copy Markdown
Contributor

The user suite test run you requested has finished and failed. I've opened a PR with the baseline diff from master.

@tomsontom Tom Schindl (tomsontom) mentioned this pull request Jul 6, 2021
Closed
@IllusionMH Andrii Dieiev (IllusionMH) mentioned this pull request Aug 12, 2021
Closed
@orta

Orta Therox (orta) commented Sep 8, 2021

Copy link
Copy Markdown
Contributor

Deprecated by #45647 - thanks Long Ho (@longlho)

@microsoft Microsoft (microsoft) locked as resolved and limited conversation to collaborators Oct 21, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@sandersn Nathan Shively-Sanders (sandersn) Awaiting requested review from sandersn
@orta Orta Therox (orta) Awaiting requested review from orta

Assignees

@orta Orta Therox (orta)

Labels

For Uncommitted Bug PR for untriaged, rejected, closed or missing bug

Projects

PR Backlog
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

es2020.intl.d.ts is inaccurate

5 participants

@longlho @DanielRosenwasser @sandersn @typescript-bot @orta