-
Notifications
You must be signed in to change notification settings - Fork 13.1k
Fix auto import crashes caused by unrelocatable symbols #45055
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
Conversation
sandersn
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why can't you include the weird merge? Is it because it doesn't have a usable name?
Yes. I can include the weird merge, but it would either have bad performance characteristics or require a new/updated checker API to give me the keys along with the symbols when getting all exports and properties of a module, and would require some refactoring to store those in the cache and use them as lookups appropriately. That might be the way forward, but I wanted to get @weswigham’s opinion on whether this merge is valid/expected first. |
sandersn
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I vote to merge this to fix the crash if it's urgent; otherwise wait a few days to get @weswigham's read on whether the merge is expected and/or easy enough to fix.
|
I’ll wait until tomorrow. |
|
It's an odd merge, but intentional - if we didn't merge (and we used to not) the interface augmentation would override the base member, rather than merge with it (which is unintuitive and undesirable). |
|
@weswigham but why replace the export alias symbol altogether? Could the alias not merge with the module augmentation, rather than the alias’s target (an external module)? |
|
Going to merge and file a bug for the missing case. |
Fixes #45030
Fixes two separate crashes—one was a simple overlooked case for skipping symbol property names, but the other was caused by a weird symbol merge that I think might be wrong/undesirable—I was expecting to see an alias in the
exportsof a module symbol, but instead got a merged resolved module symbol—see the comment below the second fourslash test.