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

[WIP es-classes] Fix extending from ES Classes #16184

Closed
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
17 changes: 17 additions & 0 deletions packages/ember-metal/lib/meta.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ export const UNDEFINED = symbol('undefined');
const SOURCE_DESTROYING = 1 << 1;
const SOURCE_DESTROYED = 1 << 2;
const META_DESTROYED = 1 << 3;
const WAS_APPLIED = 1 << 5;

const NODE_STACK = [];

Expand Down Expand Up @@ -157,6 +158,22 @@ export class Meta {
return (this._flags & flag) === flag;
}

get wasApplied() {
assert('cannot access wasApplied for an instances meta', this.proto === this.source);

return this._hasFlag(WAS_APPLIED);
}

set wasApplied(value) {
assert('cannot set wasApplied for an instances meta', this.proto === this.source);

if (value === true) {
this._flags |= WAS_APPLIED;
} else {
this._flags &= ~WAS_APPLIED;
}
}

_getOrCreateOwnMap(key) {
return this[key] || (this[key] = Object.create(null));
}
Expand Down
71 changes: 55 additions & 16 deletions packages/ember-runtime/lib/system/core_object.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,15 +45,15 @@ function makeCtor() {
// method a lot faster. This is glue code so we want it to be as fast as
// possible.

let wasApplied = false;
let initFactory;

class Class {
constructor(properties) {
let self = this;
let m = meta(this);

if (!wasApplied) {
Class.proto(); // prepare prototype...
if (!m.parent.wasApplied) {
this.constructor.proto(); // prepare prototype...
}

let beforeInitCalled; // only used in debug builds to enable the proxy trap
Expand Down Expand Up @@ -103,9 +103,12 @@ function makeCtor() {
assert(messageFor(receiver, property), value === undefined);
}
});

// We've changed "self" so meta will necessarily change as well, due to it being a different
// object. Reset meta in this case as well.
m = meta(self);
}

let m = meta(self);
let proto = m.proto;
m.proto = self;

Expand Down Expand Up @@ -208,22 +211,35 @@ function makeCtor() {
}

static willReopen() {
if (wasApplied) {
Class.PrototypeMixin = Mixin.create(Class.PrototypeMixin);
let m = meta(this.prototype);
if (m.wasApplied) {
this.PrototypeMixin = Mixin.create(this.PrototypeMixin);
}

wasApplied = false;
m.wasApplied = false;
}

static _initFactory(factory) { initFactory = factory; }

static proto() {
let superclass = Class.superclass;
if (superclass) { superclass.proto(); }
// Native classes set the prototype of the constructor to the superclass,
// so first we check to see if it exists and contains `proto`. If not, we
// look for a manually assigned superclass.
let superclass = Object.getPrototypeOf(this);

if (typeof superclass.proto === 'function') {
superclass.proto();
} else if (this.superclass) {
this.superclass.proto();
}

let m = meta(this.prototype);

if (!wasApplied) {
wasApplied = true;
Class.PrototypeMixin.applyPartial(Class.prototype);
// Setup proto incase it hasn't been setup, as in ES Class extend
m.proto = this.prototype;
if (!m.wasApplied) {
m.wasApplied = true;
this.PrototypeMixin.applyPartial(this.prototype);
}

return this.prototype;
Expand Down Expand Up @@ -682,10 +698,27 @@ let ClassMixinProps = {
@public
*/
extend() {
let Class = makeCtor();
let proto;
Class.ClassMixin = Mixin.create(this.ClassMixin);
Class.PrototypeMixin = Mixin.create(this.PrototypeMixin);
let Class, proto;

if (Object.getPrototypeOf(this) === Function.prototype) {
// The class we're extending is a base class, does not have anything
// else in the prototype chain, so continue as normal
Class = makeCtor();
} else {
// Create a simple wrapper class to defer to the rest of the prototype chain
// for native classes and classes that extend from native classes
Class = class extends this {};
}

if (this.hasOwnProperty('ClassMixin')) {
Class.ClassMixin = Mixin.create(this.ClassMixin);
Class.PrototypeMixin = Mixin.create(this.PrototypeMixin);
} else {
// Native classes do not have a ClassMixin or PrototypeMixin,
// create one using their constructor and prototype respectively
Class.ClassMixin = Mixin.create(this);
Class.PrototypeMixin = Mixin.create(this.prototype);
}

Class.ClassMixin.ownerConstructor = Class;
Class.PrototypeMixin.ownerConstructor = Class;
Expand Down Expand Up @@ -789,6 +822,7 @@ let ClassMixinProps = {
@public
*/
reopen() {
assert(`You cannot reopen ${this.name} because it was defined with native class syntax`, this.hasOwnProperty('PrototypeMixin'));
this.willReopen();
reopen.apply(this.PrototypeMixin, arguments);
return this;
Expand Down Expand Up @@ -856,6 +890,7 @@ let ClassMixinProps = {
@public
*/
reopenClass() {
assert(`You cannot reopen ${this.name} because it was defined with native class syntax`, this.hasOwnProperty('ClassMixin'));
reopen.apply(this.ClassMixin, arguments);
applyMixin(this, arguments, false);
return this;
Expand Down Expand Up @@ -1038,5 +1073,9 @@ ClassMixin.ownerConstructor = CoreObject;

CoreObject.ClassMixin = ClassMixin;

// ensure CoreObject itself has a proper class meta
let proto = CoreObject.prototype;
meta(proto).proto = proto;

ClassMixin.apply(CoreObject);
export default CoreObject;
183 changes: 164 additions & 19 deletions packages/ember-runtime/tests/system/object/es-compatibility-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,37 +36,173 @@ QUnit.test('extending an Ember.Object', function(assert) {
assert.equal(myObject.passedProperty, 'passed-property', 'passed property available on instance (new)');
});

QUnit.test('using super', function(assert) {
QUnit.test('normal method super', function(assert) {
let calls = [];

let SuperSuperObject = EmberObject.extend({
let Foo = EmberObject.extend({
method() {
calls.push('super-super-method');
calls.push('foo');
}
});

let SuperObject = SuperSuperObject.extend({
let Bar = Foo.extend({
method() {
this._super();
calls.push('super-method');
calls.push('bar');
}
});

class MyObject extends SuperObject {
class Baz extends Bar {
method() {
super.method();
calls.push('method');
calls.push('baz');
}
}

let myObject = new MyObject();
myObject.method();
let Qux = Baz.extend({
method() {
this._super();
calls.push('qux');
}
});

let Quux = Qux.extend({
method() {
this._super();
calls.push('quux');
}
});


class Corge extends Quux {
method() {
super.method();
calls.push('corge');
}
}

let callValues = ['foo', 'bar', 'baz', 'qux', 'quux', 'corge'];

[Foo, Bar, Baz, Qux, Quux, Corge].forEach((Class, index) => {
calls = [];
new Class().method();

assert.deepEqual(calls, callValues.slice(0, index + 1), 'ch,ain of static methods called with super');
});
});

QUnit.test('static method super', function(assert) {
let calls;

let Foo = EmberObject.extend();
Foo.reopenClass({
method() {
calls.push('foo');
}
});

let Bar = Foo.extend();
Bar.reopenClass({
method() {
this._super();
calls.push('bar');
}
});

assert.deepEqual(calls, [
'super-super-method',
'super-method',
'method'
], 'chain of prototype methods called with super');
class Baz extends Bar {
static method() {
super.method();
calls.push('baz');
}
}

let Qux = Baz.extend();
Qux.reopenClass({
method() {
this._super();
calls.push('qux');
}
});

let Quux = Qux.extend();
Quux.reopenClass({
method() {
this._super();
calls.push('quux');
}
});

class Corge extends Quux {
static method() {
super.method();
calls.push('corge');
}
}

let callValues = ['foo', 'bar', 'baz', 'qux', 'quux', 'corge'];

[Foo, Bar, Baz, Qux, Quux, Corge].forEach((Class, index) => {
calls = [];
Class.method();

assert.deepEqual(calls, callValues.slice(0, index + 1), 'chain of static methods called with super');
});
});

QUnit.test('reopen and reopenClass on native class do not work', function(assert) {
class Foo extends EmberObject {}

assert.throws(
() => {
Foo.reopen({
foo() {
// do nothing
}
});
},
/You cannot reopen Foo because it was defined with native class syntax/
);

assert.throws(
() => {
Foo.reopenClass({
foo() {
// do nothing
}
});
},
/You cannot reopen Foo because it was defined with native class syntax/
);
});

QUnit.test('reopen and reopenClass on native class do not work after .extend', function(assert) {
class Foo extends EmberObject {}

let Bar = Foo.extend();

class Baz extends Bar {}

assert.throws(
() => {
Baz.reopen({
foo() {
// do nothing
}
});
},
/You cannot reopen Baz because it was defined with native class syntax/
);

assert.throws(
() => {
Baz.reopenClass({
foo() {
// do nothing
}
});
},
/You cannot reopen Baz because it was defined with native class syntax/
);
});

QUnit.test('using mixins', function(assert) {
Expand Down Expand Up @@ -123,14 +259,15 @@ QUnit.test('extending an ES subclass of EmberObject', function(assert) {
assert.deepEqual(calls, ['constructor', 'init'], 'constructor then init called (new)');
});

// TODO: Needs to be fixed. Currently only `init` is called.
QUnit.skip('calling extend on an ES subclass of EmberObject', function(assert) {
QUnit.test('calling extend on an ES subclass of EmberObject', function(assert) {
let calls = [];

class SubEmberObject extends EmberObject {
constructor() {
calls.push('constructor');
calls.push('before constructor');
super(...arguments);
calls.push('after constructor');
this.foo = 123;
}

init() {
Expand All @@ -142,9 +279,17 @@ QUnit.skip('calling extend on an ES subclass of EmberObject', function(assert) {
let MyObject = SubEmberObject.extend({});

MyObject.create();
assert.deepEqual(calls, ['constructor', 'init'], 'constructor then init called (create)');
assert.deepEqual(calls, ['before constructor', 'init', 'after constructor'], 'constructor then init called (create)');

calls = [];
new MyObject();
assert.deepEqual(calls, ['constructor', 'init'], 'constructor then init called (new)');
assert.deepEqual(calls, ['before constructor', 'init', 'after constructor'], 'constructor then init called (new)');

let obj = MyObject.create({
foo: 456,
bar: 789
});

assert.equal(obj.foo, 123, 'sets class fields on instance correctly');
assert.equal(obj.bar, 789, 'sets passed in properties on instance correctly');
});