Skip to content
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

Check object extensibility before adding default property. #113

Merged
merged 1 commit into from
Mar 5, 2017

Conversation

rwjblue
Copy link
Member

@rwjblue rwjblue commented Mar 5, 2017

The checks added previously were checking for Object.isFrozen, but that only covers one scenario (Object.freeze) and it is still possible to make an object
non-extensible without freezing it.

This adds tests for frozen objects (previously passing) and a new test for Object.seal'ed objects (which was failing prior to this change).

The checks added previously were checking for `Object.isFrozen`, but
that only covers one scenario (`Object.freeze`) and it is still possible
to make an object non-extensible without freezing it.

This adds tests for frozen objects (previously passing) and a new test
for `Object.seal`'ed objects (which was failing prior to this change).
@chadhietala
Copy link
Member

Seems good. I think I'm running into to this on Kris build rework branch.

@rwjblue
Copy link
Member Author

rwjblue commented Mar 5, 2017

Yes, it is an issue for anything (in babel@6) that exports something that is either frozen or sealed. The most common case is when you export default Ember.Mixin.create(/* whatever */) (since Ember.Mixin does Object.seal in its init during debug builds). I ran into this while working on babel@6 compat in ember-cli/ember-cli#6827.

@rwjblue rwjblue merged commit 9c34843 into master Mar 5, 2017
@rwjblue rwjblue deleted the check-extensibility branch March 5, 2017 22:35
@stefanpenner
Copy link
Contributor

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants