Skip to content

Preserve separators in numeric literals with target=ES2021+ #57144

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

Merged
merged 3 commits into from
Feb 15, 2024

Conversation

danvk
Copy link
Contributor

@danvk danvk commented Jan 23, 2024

Please verify that:

  • There is an associated issue in the Backlog milestone (required)
  • Code is up-to-date with the main branch
  • You've successfully run hereby runtests locally
  • There are new or updated unit tests validating the change

Fixes #53182

The issue was that back when numeric separators were first implemented in TypeScript back in 2017 (#20324 (review)) there was no ES2021 emit target (or ES2018 for that matter!) and so ESNext was used as a stand-in. When target=ES2021 was added later, the check for numeric separators wasn't updated from ESNext → ES2021.

@typescript-bot typescript-bot added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Jan 23, 2024
@danvk
Copy link
Contributor Author

danvk commented Jan 23, 2024

@microsoft-github-policy-service agree

Copy link
Member

@sandersn sandersn left a comment

Choose a reason for hiding this comment

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

This looks OK to me. @DanielRosenwasser @rbuckton is this safe to put in 5.4? Do we want it there? It's technically a fix but @RyanCavanaugh treated it as a bonus feature in the bug.

@RyanCavanaugh
Copy link
Member

Not a feature
Not a bug
Some secret third thing?

@RyanCavanaugh RyanCavanaugh merged commit f41a5f5 into microsoft:main Feb 15, 2024
@RyanCavanaugh
Copy link
Member

TODO: Figure out how to avoid this kind of bug in the future? It's come up multiple times that people compare against ESNext because we don't know which ES20XX version the spec update will land in, then we forget to update the code. Maybe we should make ES2099 as an internal-only name and then rename it to e.g. ES2026 when the spec versions land?

@sandersn
Copy link
Member

I think the right answer is a manual sweep of esnext each year as the standard is finalised. From asking @rbuckton yesterday we learned that it's usually April (provisional) or June (final).

There's no way to predict with ES20xx a particular addition will end up in, so I don't think renaming a placeholder will help. Even if each proposal gets its own placeholder, they'd still need a manual update to the year that they actually ended up in.

@danvk
Copy link
Contributor Author

danvk commented Feb 16, 2024

Let's all hope we're around to see the final feature set of ES2099! 😀

@RyanCavanaugh
Copy link
Member

Sowing the grounds for the first Y2.1K bug 😭

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

NumericLiteral text property strips the number of separators
6 participants