[CLEANUP] removed Ember.required
#15892
Conversation
|
I'm not 100% certain that this can be implemented in the addon. I believe we may need to go with the conditional flagging on this one. @thoov - What do you think? |
|
@bekzod - ping (when you have a chance) |
|
@rwjblue so it should be runtime flag ? flags are not stripped in build time ? |
|
Confirm. It has to be a runtime flag because we cannot polyfill it from the ember-2-legacy addon. |
bekzod
force-pushed
the
remove-required
branch
3 times, most recently
from
03605e0
to
c0df986
Compare
|
does this look better ? |
| QUnit.module('Module.required', { | ||
| setup() { | ||
| originalEnvVal = ENV._ENABLE_PROPERTY_REQUIRED_SUPPORT |
| @@ -127,7 +127,6 @@ Ember.observersFor = metal.observersFor; | |||
| Ember.removeObserver = metal.removeObserver; | |||
| Ember._suspendObserver = metal._suspendObserver; | |||
| Ember._suspendObservers = metal._suspendObservers; | |||
| Ember.required = metal.required; | |||
There was a problem hiding this comment.
I believe we are going to have to guard this
| @@ -108,7 +108,6 @@ QUnit.module('ember reexports'); | |||
| ['removeObserver', 'ember-metal'], | |||
| ['_suspendObserver', 'ember-metal'], | |||
| ['_suspendObservers', 'ember-metal'], | |||
| ['required', 'ember-metal'], | |||
There was a problem hiding this comment.
I believe we are going to have to guard this
bekzod
force-pushed
the
remove-required
branch
from
c0df986
to
7a9723a
Compare
|
LGTM @rwjblue can you take a look |
|
In practice, this required doesn't do anything because of applyPartial and finishPartial doesn't handle it. It only affects mixin.apply() which is rare. For upgrading an addon could just make required() present and return null. |
No description provided.