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

Unexpected "Attempting to define property on object that is not extensible." on 2.9 beta #14264

Closed
kellyselden opened this issue Sep 11, 2016 · 10 comments

Comments

@kellyselden
Copy link
Member

I have a very simple addon. It's latest master commit is from March. I deleted Travis cache and reran the master build. Here are the results:
https://travis-ci.org/kellyselden/ember-array-helper/builds/115857118
release working:
https://travis-ci.org/kellyselden/ember-array-helper/jobs/115857121
beta failing:
https://travis-ci.org/kellyselden/ember-array-helper/jobs/115857123

Here it is reproduced on twiddle:
working on release:
https://ember-twiddle.com/eb15180109e6467f27f88743245cf788/ab9ce64a8625d645b144795f930e10432e0c1816
broken on beta:
https://ember-twiddle.com/eb15180109e6467f27f88743245cf788/5d79be64c7da893b3fbce85e229fbed200b04963

Here is the relevant stack trace when testing locally:
capture

It fails on this line

Object.defineProperty(obj, META_FIELD, META_DESC);

Here is how I got it working:

import Ember from 'ember';

export function array(params/*, hash*/) {
  // this is the original failure
  // return params;

  // this also fails for the same reason (unexpectedly according to docs)
  // return Ember.A(params);

  // this makes it work
  let array = Ember.A();
  array.pushObjects(params);
  return array;
}

export default Ember.Helper.helper(array);

But there may be an unexpected change in beta that is causing the prototype extensions to act differently. This also only seems to be an issue with helpers.

@rwjblue
Copy link
Member

rwjblue commented Sep 11, 2016

This is a consequence of #14244. When the params / hash are frozen, we cannot put a meta on the object (which is done if it is used in the template).

@krisselden
Copy link
Contributor

You can't mutate the params array and we seal it in the debug build now. You can just use slice.

We made it fail fast when we realized people were doing this, it prevents us from having to allocate tons of empty arrays.

@rwjblue
Copy link
Member

rwjblue commented Sep 11, 2016

@krisselden - The specific thing being done here is Ember mutating the array (by trying to put meta on it).

@krisselden
Copy link
Contributor

Robert we should not try to observe a frozen object it isn't going to change

@rwjblue
Copy link
Member

rwjblue commented Sep 11, 2016

kk, good point, I'll look there to see how much work that will be for us to avoid observing frozen objects

@krisselden
Copy link
Contributor

That is the appropriate fix rather than revert the other change

@krisselden
Copy link
Contributor

Object.isFrozen on the add observer methods

@krisselden
Copy link
Contributor

we should also use freeze instead of seal seal is shape changes not writes but we want to be readonly

@rwjblue
Copy link
Member

rwjblue commented Sep 11, 2016

Ya, we are freezing (done in #14244)

@mixonic
Copy link
Sponsor Member

mixonic commented Nov 29, 2016

@kellyselden I'm going to close this in favor of a PR with a reproduction: #14649 Thanks for keeping the oil burning on this.

@mixonic mixonic closed this as completed Nov 29, 2016
mixonic added a commit to mixonic/glimmer that referenced this issue Dec 17, 2016
When arrays and objects are frozen in JavaScript, it is impossible to
attach meta-data (like Ember's own `meta`) to them without using a
WeakMap. Ember does adopt the WeakMap strategy in browsers that support
it, however there are still supported environments (IE9, IE10) where
`Object.freeze` is supported but WeakMap is not. But not freezing these
empty arrays and object if WeakMap is missing, legacy meta-data
strategies are permitted on those instances.

See:

* emberjs/ember.js#14264
* emberjs/ember.js#14244
chancancode pushed a commit to glimmerjs/glimmer-vm that referenced this issue Jan 10, 2017
When arrays and objects are frozen in JavaScript, it is impossible to
attach meta-data (like Ember's own `meta`) to them without using a
WeakMap. Ember does adopt the WeakMap strategy in browsers that support
it, however there are still supported environments (IE9, IE10) where
`Object.freeze` is supported but WeakMap is not. But not freezing these
empty arrays and object if WeakMap is missing, legacy meta-data
strategies are permitted on those instances.

See:

* emberjs/ember.js#14264
* emberjs/ember.js#14244

(cherry picked from commit 6954ade)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants