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

[DEPRECATION] (WIP) calling this._super when there is no method on parent class #15082

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 10 additions & 47 deletions packages/ember-glimmer/lib/component.js
Original file line number Diff line number Diff line change
Expand Up @@ -637,7 +637,7 @@ const Component = CoreView.extend(
readDOMAttr(name) {
let element = getViewElement(this);
return readDOMAttr(element, name);
}
},

/**
The WAI-ARIA role of the control represented by this view. For example, a
Expand Down Expand Up @@ -717,24 +717,7 @@ const Component = CoreView.extend(
@public
@since 1.13.0
*/

/**
Called when the attributes passed into the component have been updated.
Called both during the initial render of a container and during a rerender.
Can be used in place of an observer; code placed here will be executed
every time any attribute updates.
@event didReceiveAttrs
@public
@since 1.13.0
*/

/**
Called after a component has been rendered, both on initial render and
in subsequent rerenders.
@method didRender
@public
@since 1.13.0
*/
didReceiveAttrs() {},

/**
Called after a component has been rendered, both on initial render and
Expand All @@ -743,14 +726,7 @@ const Component = CoreView.extend(
@public
@since 1.13.0
*/

/**
Called before a component has been rendered, both on initial render and
in subsequent rerenders.
@method willRender
@public
@since 1.13.0
*/
didRender() {},

/**
Called before a component has been rendered, both on initial render and
Expand All @@ -759,6 +735,7 @@ const Component = CoreView.extend(
@public
@since 1.13.0
*/
willRender() {},

/**
Called when the attributes passed into the component have been changed.
Expand All @@ -767,14 +744,7 @@ const Component = CoreView.extend(
@public
@since 1.13.0
*/

/**
Called when the attributes passed into the component have been changed.
Called only during a rerender, not during an initial render.
@event didUpdateAttrs
@public
@since 1.13.0
*/
didUpdateAttrs() {},

/**
Called when the component is about to update and rerender itself.
Expand All @@ -783,14 +753,7 @@ const Component = CoreView.extend(
@public
@since 1.13.0
*/

/**
Called when the component is about to update and rerender itself.
Called only during a rerender, not during an initial render.
@event willUpdate
@public
@since 1.13.0
*/
willUpdate() {},

/**
Called when the component has updated and rerendered itself.
Expand All @@ -799,14 +762,14 @@ const Component = CoreView.extend(
@public
@since 1.13.0
*/
didUpdate() {},

/**
Called when the component has updated and rerendered itself.
Called only during a rerender, not during an initial render.
@event didUpdate
@event didDestroyElement
@public
@since 1.13.0
@since 2.13.0
*/
didDestroyElement() {}

/**
A component may contain a layout. A layout is a regular template but
Expand Down
18 changes: 16 additions & 2 deletions packages/ember-metal/lib/mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,15 @@ function giveDescriptorSuper(meta, key, property, values, descs, base) {
}

if (superProperty === undefined || !(superProperty instanceof ComputedProperty)) {
return property;
superProperty = {
_getter: function() {
deprecate(
`Calling \`_super\` when there is no computed \`${key}\` on the parent class is deprecated.`,
false,
{ id: 'ember-metal.missing-super-computed', until: '3.0.0' }
);
}
};
}

// Since multiple mixins may inherit from the same parent, we need
Expand Down Expand Up @@ -129,7 +137,13 @@ function giveMethodSuper(obj, key, method, values, descs) {

// Only wrap the new method if the original method was a function
if (superMethod === undefined || 'function' !== typeof superMethod) {
return method;
superMethod = function() {
deprecate(
`Calling \`_super\` when there is no method \`${key}\` on the parent class is deprecated.`,
false,
{ id: 'ember-metal.missing-super-method', until: '3.0.0' }
);
};
}

return wrap(method, superMethod);
Expand Down
16 changes: 16 additions & 0 deletions packages/ember-metal/tests/mixin/computed_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -141,3 +141,19 @@ QUnit.test('setter behavior works properly when overriding computed properties',
equal(get(obj, 'cpWithoutSetter'), 'test', 'The default setter was called, the value is correct');
ok(!cpWasCalled, 'The default setter was called, not the CP itself');
});

QUnit.test('calling _super when there is no computed on parent class is deprecated', function() {
expectDeprecation('Calling `_super` when there is no computed `foo` on the parent class is deprecated.');

let SuperMixin = Mixin.create({});

let SubMixin = Mixin.create(SuperMixin, {
foo: computed(function() {
return this._super(...arguments);
})
});

let obj = {};
SubMixin.apply(obj);
get(obj, 'foo');
});
24 changes: 21 additions & 3 deletions packages/ember-metal/tests/mixin/method_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,8 @@ QUnit.test('Including the same mixin more than once will only run once', functio
equal(cnt, 1, 'should invoke MixinA.foo one time');
});

QUnit.test('_super from a single mixin with no superclass does not error', function() {
QUnit.test('_super from a single mixin with no superclass is deprecated', function() {
expectDeprecation('Calling `_super` when there is no method `foo` on the parent class is deprecated.');
let MixinA = Mixin.create({
foo() {
this._super(...arguments);
Expand All @@ -127,10 +128,11 @@ QUnit.test('_super from a single mixin with no superclass does not error', funct
MixinA.apply(obj);

obj.foo();
ok(true);
});

QUnit.test('_super from a first-of-two mixins with no superclass function does not error', function() {
QUnit.test('_super from a first-of-two mixins with no superclass function is deprecated', function() {
expectDeprecation('Calling `_super` when there is no method `foo` on the parent class is deprecated.');

// _super was previously calling itself in the second assertion.
// Use remaining count of calls to ensure it doesn't loop indefinitely.
let remaining = 3;
Expand All @@ -154,6 +156,22 @@ QUnit.test('_super from a first-of-two mixins with no superclass function does n
ok(true);
});

QUnit.test('calling _super when there is no method on parent class is deprecated', function() {
expectDeprecation('Calling `_super` when there is no method `foo` on the parent class is deprecated.');
let MixinA = Mixin.create({});

let MixinB = Mixin.create(MixinA, {
foo() {
this._super(...arguments);
}
});

let obj = {};
MixinB.apply(obj);

obj.foo();
});

// ..........................................................
// CONFLICTS
//
Expand Down