Skip to content

Commit

Permalink
experiment(CoreObject): Fix constructor by branching during extends
Browse files Browse the repository at this point in the history
  • Loading branch information
pzuraq authored and rwjblue committed Mar 26, 2018
1 parent d7f9675 commit 40c2eb2
Show file tree
Hide file tree
Showing 2 changed files with 209 additions and 31 deletions.
57 changes: 45 additions & 12 deletions packages/ember-runtime/lib/system/core_object.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,10 @@ function makeCtor() {
class Class {
constructor(properties) {
let self = this;
let m = meta(self);
let m = meta(this);

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

let beforeInitCalled; // only used in debug builds to enable the proxy trap
Expand Down Expand Up @@ -103,6 +103,10 @@ 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 proto = m.proto;
Expand Down Expand Up @@ -209,22 +213,33 @@ function makeCtor() {
static willReopen() {
let m = meta(this.prototype);
if (m.wasApplied) {
Class.PrototypeMixin = Mixin.create(Class.PrototypeMixin);
this.PrototypeMixin = Mixin.create(this.PrototypeMixin);
}

m.asApplied = 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);

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

return this.prototype;
Expand Down Expand Up @@ -683,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 @@ -790,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 @@ -857,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 @@ -1041,7 +1075,6 @@ CoreObject.ClassMixin = ClassMixin;

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

ClassMixin.apply(CoreObject);
Expand Down
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');
});

0 comments on commit 40c2eb2

Please sign in to comment.