Repository navigation
Fix local and exported type alias merging - #36237
Conversation
Nathan Shively-Sanders (sandersn)
left a comment
Nathan Shively-Sanders (sandersn)
left a comment
There was a problem hiding this comment.
Can you run this on RWC and user tests before merging? DT seems unlikely to have this error.
|
TypeScript Bot (@typescript-bot) test this |
|
Heya Andrew Branch (@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 Andrew Branch (@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. |
Daniel Rosenwasser (DanielRosenwasser)
added
the
Breaking Change
4 participants
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.