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

[BUGFIX beta] Fix various issues with descriptor trap. #16169

Merged
merged 4 commits into from
Jan 23, 2018

Conversation

rwjblue
Copy link
Member

@rwjblue rwjblue commented Jan 23, 2018

  • Avoid adding enumerable properties to Array.prototype. Ember.NativeArray is a normal Ember.Mixin that we mix into Array.prototype when prototype extensions are enabled. Mutating a native object prototype like this should not result in enumerable properties being added (or we have significant issues with things like deep equality checks from test frameworks, or things like jQuery.extend(true, [], [])). This is a hack, and we should stop mutating the array prototype by default. 😫

  • Cache the descriptor trap. Prior to this change, a new proxy was created for every property access. This resulted in some pretty bizarre semantics. Consider the following:

 bar: Ember.computed(function() { });
});

let obj = Foo.create(); console.log(obj.bar === obj.bar);
// => false

O_o

  • Avoid adding enumerable properties to String.prototype.
  • Avoid adding enumerable properties to Function.prototype.

Prior to this change, a new proxy was created for every property access.
This resulted in some pretty bizarre semantics. Consider the following:

```js
let Foo = Ember.Object.extend({
  bar: Ember.computed(function() { });
});

let obj = Foo.create();
console.log(obj.bar === obj.bar);
// => false
```

O_o
`Ember.NativeArray` is a normal Ember.Mixin that we mix into
`Array.prototype` when prototype extensions are enabled. Mutating a
native object prototype like this should _not_ result in enumerable
properties being added (or we have significant issues with things like
deep equality checks from test frameworks, or things like
`jQuery.extend(true, [], [])`).

this is a hack, and we should stop mutating the array prototype by
default 😫
@rwjblue rwjblue requested review from chancancode and mmun January 23, 2018 14:07
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.

1 participant