Skip to content

Fix crash in expando assignment to alias#34566

Merged
sandersn merged 2 commits intomasterfrom
fix-expando-assignment-to-alias
Oct 18, 2019
Merged

Fix crash in expando assignment to alias#34566
sandersn merged 2 commits intomasterfrom
fix-expando-assignment-to-alias

Conversation

@sandersn
Copy link
Copy Markdown
Member

@sandersn sandersn commented Oct 18, 2019

This PR disallows expando assignments

Fixes #34493, but disallows the prototype assignment, which never worked.

This PR disallows expando assignments

Fixes #34493, but disallows the prototype assignment nonetheless.
@andrewbranch
Copy link
Copy Markdown
Member

Note: this is a superset of #34553

@sandersn
Copy link
Copy Markdown
Member Author

Sorry, that was a mistake because I was testing #34553 locally and forgot. I reverted those changes because it's much too confusing otherwise.

Copy link
Copy Markdown
Member

@andrewbranch andrewbranch left a comment

Choose a reason for hiding this comment

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

Is the fact that expandos don't work on alias symbols documented anywhere? I guess people don’t run into this very often.

@sandersn
Copy link
Copy Markdown
Member Author

sandersn commented Oct 18, 2019

Given that I didn't think of this before, I'd guess not. Our test coverage is heavily biased toward commonjs importing commonjs.

@sandersn sandersn merged commit fa1884e into master Oct 18, 2019
@sandersn sandersn deleted the fix-expando-assignment-to-alias branch October 18, 2019 20:31
@microsoft microsoft locked as resolved and limited conversation to collaborators Oct 21, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

--allowJs: Cannot update prototype of imported class (3.7.0 regression)

2 participants