Conversation
ac865de to
affaf0e
Compare
|
It looks like you've sent a pull request to update our 'lib' files. These files aren't meant to be edited by hand, as they consist of last-known good states of the compiler and are generated from 'src'. Unless this is necessary, consider closing the pull request and sending a separate PR to update 'src'. |
|
cc @orta |
orta
left a comment
There was a problem hiding this comment.
Overall looks good, I think UnicodeBCP47LocaleIdentifier should probably live here and get removed from the 2020 version.
I'll also leave this a few days to let others give some feedback
|
ping @orta :) |
|
ping @DanielRosenwasser |
Hmm, is that a breaking change? |
|
That's what I thought but I'm cargo culting and tests seem upset if I just modified src |
Co-authored-by: Daniel Rosenwasser <DanielRosenwasser@users.noreply.github.com>
Co-authored-by: Daniel Rosenwasser <DanielRosenwasser@users.noreply.github.com>
Co-authored-by: Daniel Rosenwasser <DanielRosenwasser@users.noreply.github.com>
|
@DanielRosenwasser what do you think? Should we go ahead and merge this? Seems like the only outstanding question is whether to check in the |
|
can I get a verdict here 😄 ? cc @DanielRosenwasser @sandersn |
|
This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise. |
|
OK, I've rebased this and set it up locally - as it is a project targeting I think we'll need to remove the identifier from es2020 to have it be usable in es2016. The question of whether this is a breaking change is a good one. If you had a setup like: "target": "es2019",
"lib": ["es2020.intl"]and were referring to "target": "es2015",
"lib": ["es2020.intl"]Then moving the type back is a breaking change. Which given the breadth of the TS userbase, someone probably has this setup. Which gives three options:
Personally, I fall into the last bucket. |
|
I personally would pick the 1st option. ECMA402 doesn't often introduce a lot of new APIs per release and have them work independently without previous version, e.g you should not be able specify |
|
OK, I think this is good to go now:
This last one needs to get mentioned in the 4.5 release notes breaking changes section. Something like:
I agree it's pretty niche, but a single sentence should be enough to handle covering all cases during migration. |
|
@typescript-bot user test this |
|
The user suite test run you requested has finished and failed. I've opened a PR with the baseline diff from master. |
I don't see this code anywhere in #45647 or anywhere else in TS (as mentioned elsewhere). Is that intentional? |
part of #29129