Skip to content

[Bugfix Beta] Freeze attrs in debug#14265

Closed
chadhietala wants to merge 1 commit intoemberjs:masterfrom
chadhietala:freeze-attrs
Closed

[Bugfix Beta] Freeze attrs in debug#14265
chadhietala wants to merge 1 commit intoemberjs:masterfrom
chadhietala:freeze-attrs

Conversation

@chadhietala
Copy link
Contributor

@rwjblue @krisselden I believe there were talks about freezing these as well. I think we want to make attrs pay as you go as well.

@chadhietala
Copy link
Contributor Author

I actually don't know if we can do this. If you use attrs in the backing class for something like dependent keys on a CP we're going to install meta and thats going to result in

Cannot define property:__ember_meta__, object is not extensible

Specifically I ran into this with Link-to's use of attrs :trollface:.

@rwjblue
Copy link
Member

rwjblue commented Sep 11, 2016

Yep, that is exactly what #14264 is about.

@rwjblue
Copy link
Member

rwjblue commented Sep 11, 2016

Discussed in Slack, I think changing these lines:

To use this as the guard:

if (isFeatureEnabled('mandatory-setter') && !Object.isFrozen(parentValue)) {
}

Should fix?

@krisselden
Copy link
Contributor

It should fix it, to be honest it is annoying we haven't done this for frozen objects already

@rwjblue
Copy link
Member

rwjblue commented Sep 11, 2016

@krisselden - Agreed. I think we also need to do something in the CP system (since {{link-to}} is using attrs.foo as a dependent key).

@krisselden
Copy link
Contributor

That still goes through Ember.watch

@chadhietala
Copy link
Contributor Author

So guarding here causes a bunch of other things to break, about 1000 tests.

@homu
Copy link
Contributor

homu commented Sep 12, 2016

☔ The latest upstream changes (presumably #14267) made this pull request unmergeable. Please resolve the merge conflicts.

@krisselden
Copy link
Contributor

@chadhietala that is because there are assumptions about state that it breaks that would need to be updated.

addObserver is addListener + watch. cps use watch directly. there maybe some other cases with isWatching.

Also maybe assumptions about corresponding state in mandatory setter.

I didn't say it would be simple, I just think it is worth it.

You should be able to render a frozen object in a template.

@mmun
Copy link
Member

mmun commented Feb 23, 2018

Watching frozen objects works now.

@mmun mmun closed this Feb 23, 2018
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.

5 participants