From d7f9675049a774341e2e7104ec09bbcebb3ed036 Mon Sep 17 00:00:00 2001 From: Christopher Garrett Date: Sat, 27 Jan 2018 15:07:24 -0800 Subject: [PATCH 1/2] move wasApplied to meta --- packages/ember-metal/lib/meta.js | 17 ++++++++++++++++ .../ember-runtime/lib/system/core_object.js | 20 ++++++++++++------- 2 files changed, 30 insertions(+), 7 deletions(-) diff --git a/packages/ember-metal/lib/meta.js b/packages/ember-metal/lib/meta.js index 7ee17596e92..106eab93311 100644 --- a/packages/ember-metal/lib/meta.js +++ b/packages/ember-metal/lib/meta.js @@ -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 = []; @@ -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)); } diff --git a/packages/ember-runtime/lib/system/core_object.js b/packages/ember-runtime/lib/system/core_object.js index 84229979476..d02036a6ac9 100644 --- a/packages/ember-runtime/lib/system/core_object.js +++ b/packages/ember-runtime/lib/system/core_object.js @@ -45,14 +45,14 @@ 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(self); - if (!wasApplied) { + if (!m.parent.wasApplied) { Class.proto(); // prepare prototype... } @@ -105,7 +105,6 @@ function makeCtor() { }); } - let m = meta(self); let proto = m.proto; m.proto = self; @@ -208,11 +207,12 @@ function makeCtor() { } static willReopen() { - if (wasApplied) { + let m = meta(this.prototype); + if (m.wasApplied) { Class.PrototypeMixin = Mixin.create(Class.PrototypeMixin); } - wasApplied = false; + m.asApplied = false; } static _initFactory(factory) { initFactory = factory; } @@ -221,8 +221,9 @@ function makeCtor() { let superclass = Class.superclass; if (superclass) { superclass.proto(); } - if (!wasApplied) { - wasApplied = true; + let m = meta(this.prototype); + if (!m.wasApplied) { + m.wasApplied = true; Class.PrototypeMixin.applyPartial(Class.prototype); } @@ -1038,5 +1039,10 @@ ClassMixin.ownerConstructor = CoreObject; CoreObject.ClassMixin = ClassMixin; +// ensure CoreObject itself has a proper class meta +let proto = CoreObject.prototype; +generateGuid(proto); +meta(proto).proto = proto; + ClassMixin.apply(CoreObject); export default CoreObject; From 40c2eb29323c0cc270cc933d21fcdb859971f1c4 Mon Sep 17 00:00:00 2001 From: Christopher Garrett Date: Sat, 27 Jan 2018 16:44:11 -0800 Subject: [PATCH 2/2] experiment(CoreObject): Fix constructor by branching during extends --- .../ember-runtime/lib/system/core_object.js | 57 ++++-- .../system/object/es-compatibility-test.js | 183 ++++++++++++++++-- 2 files changed, 209 insertions(+), 31 deletions(-) diff --git a/packages/ember-runtime/lib/system/core_object.js b/packages/ember-runtime/lib/system/core_object.js index d02036a6ac9..430629d30d8 100644 --- a/packages/ember-runtime/lib/system/core_object.js +++ b/packages/ember-runtime/lib/system/core_object.js @@ -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 @@ -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; @@ -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; @@ -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; @@ -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; @@ -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; @@ -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); diff --git a/packages/ember-runtime/tests/system/object/es-compatibility-test.js b/packages/ember-runtime/tests/system/object/es-compatibility-test.js index 6ad906a039c..df1fea07038 100644 --- a/packages/ember-runtime/tests/system/object/es-compatibility-test.js +++ b/packages/ember-runtime/tests/system/object/es-compatibility-test.js @@ -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) { @@ -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() { @@ -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'); });