Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

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

Closed
longlho wants to merge 1 commit into
microsoft:mainmicrosoft/TypeScript:mainfrom
longlho:patch-1longlho/TypeScript:patch-1Copy head branch name to clipboard
Closed

fix: revamp es2020.intl.d.ts#42945
longlho wants to merge 1 commit into
microsoft:mainmicrosoft/TypeScript:mainfrom
longlho:patch-1longlho/TypeScript:patch-1Copy head branch name to clipboard

Conversation

@longlho

@longlho 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 added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Feb 24, 2021
@longlho longlho mentioned this pull request Feb 24, 2021
Closed
@DanielRosenwasser

DanielRosenwasser commented Feb 24, 2021

Copy link
Copy Markdown
Member

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

@longlho

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 longlho changed the title fix: remove fractionalSecondDigits from es2020.intl.d.ts fix: revamp es2020.intl.d.ts Feb 25, 2021
@longlho

longlho commented Feb 25, 2021

Copy link
Copy Markdown
Contributor Author

@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

longlho commented Mar 3, 2021

Copy link
Copy Markdown
Contributor Author

gentle ping @RyanCavanaugh :)

@sandersn sandersn requested review from orta and sandersn March 4, 2021 15:10
@sandersn

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

longlho commented Mar 4, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@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 longlho mentioned this pull request Apr 10, 2021
Closed
@DanielRosenwasser

DanielRosenwasser commented May 10, 2021

Copy link
Copy Markdown
Member

@typescript-bot test this
@typescript-bot test dt
@typescript-bot user test this

@typescript-bot

typescript-bot commented May 10, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor

Heya @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 commented May 10, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor

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

@DanielRosenwasser

DanielRosenwasser commented May 10, 2021

Copy link
Copy Markdown
Member

@typescript-bot run dt

@typescript-bot

typescript-bot commented May 10, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor

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

@typescript-bot typescript-bot mentioned this pull request May 10, 2021
Open
@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 tomsontom mentioned this pull request Jul 6, 2021
Closed
@IllusionMH IllusionMH mentioned this pull request Aug 12, 2021
Closed
@orta

orta commented Sep 8, 2021

Copy link
Copy Markdown
Contributor

Deprecated by #45647 - thanks @longlho

@orta orta closed this Sep 8, 2021
@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 sandersn Awaiting requested review from sandersn

@orta orta Awaiting requested review from orta

Assignees

@orta 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
Morty Proxy This is a proxified and sanitized view of the page, visit original site.