Fix local and exported type alias merging #36237
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sandersn
approved these changes
Jan 22, 2020
|
@typescript-bot test this |
|
Heya @andrewbranch, I've started to run the extended test suite on this PR at 36e7c34. You can monitor the build here. It should now contribute to this PR's status checks. |
|
Heya @andrewbranch, I've started to run the parallelized community code test suite on this PR at 36e7c34. You can monitor the build here. It should now contribute to this PR's status checks. |
|
The user suite test run you requested has finished and failed. I've opened a PR with the baseline diff from master. |
|
User test changes look all unrelated |
|
There is one new error in RWC (Skype), but it looks expected and easily fixable. |
DanielRosenwasser
added
the
Breaking Change
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #36196
The error is weird, but it’s consistent with what you get for every other kind of declaration, including other non-merging declarations like constants. The way the binder works is a little strange; an exported declaration does have a local symbol but it doesn’t get any symbol flags, so anything can merge into it. On the other hand, it runs through
declareSymbolwith a normal, full set ofexcludesflags, so it will not merge into something else—a strange asymmetry that means if you reverse the order of declarations such that the local-only comes first:you get the more expected “Duplicate identifier” error. It seems like there’s a potential to do some general work around making these collisions symmetrical in the binder, but it would be a much bigger and more dangerous change that can’t happen for 3.8 at this point.