Skip to content

Commit

Permalink
Merge pull request #16795 from pzuraq/feat/delay-init-until-after-con…
Browse files Browse the repository at this point in the history 
…struction

[core-object] Delay init until after construction
  • Loading branch information
@rwjblue
rwjblue authored Sep 26, 2018
2 parents 8685a1c + 9a817d4 commit 0554471
Show file tree
Hide file tree
Showing 14 changed files with 179 additions and 201 deletions.
206 changes: 115 additions & 91 deletions packages/@ember/-internals/runtime/lib/system/core_object.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import {
classToString,
} from '@ember/-internals/metal';
import ActionHandler from '../mixins/action_handler';
import { assert } from '@ember/debug';
import { assert, deprecate } from '@ember/debug';
import { DEBUG } from '@glimmer/env';

const reopen = Mixin.prototype.reopen;
Expand All @@ -37,6 +37,103 @@ const factoryMap = new WeakMap();

const prototypeMixinMap = new WeakMap();

const DELAY_INIT = Object.freeze({});

let initCalled; // only used in debug builds to enable the proxy trap

// using DEBUG here to avoid the extraneous variable when not needed
if (DEBUG) {
initCalled = new WeakSet();
}

function initialize(obj, properties) {
let m = meta(obj);

if (properties !== undefined) {
assert(
'EmberObject.create only accepts objects.',
typeof properties === 'object' && properties !== null
);

assert(
'EmberObject.create no longer supports mixing in other ' +
'definitions, use .extend & .create separately instead.',
!(properties instanceof Mixin)
);

let concatenatedProperties = obj.concatenatedProperties;
let mergedProperties = obj.mergedProperties;
let hasConcatenatedProps =
concatenatedProperties !== undefined && concatenatedProperties.length > 0;
let hasMergedProps = mergedProperties !== undefined && mergedProperties.length > 0;

let keyNames = Object.keys(properties);

for (let i = 0; i < keyNames.length; i++) {
let keyName = keyNames[i];
let value = properties[keyName];

assert(
'EmberObject.create no longer supports defining computed ' +
'properties. Define computed properties using extend() or reopen() ' +
'before calling create().',
!(value instanceof ComputedProperty)
);
assert(
'EmberObject.create no longer supports defining methods that call _super.',
!(typeof value === 'function' && value.toString().indexOf('._super') !== -1)
);
assert(
'`actions` must be provided at extend time, not at create time, ' +
'when Ember.ActionHandler is used (i.e. views, controllers & routes).',
!(keyName === 'actions' && ActionHandler.detect(obj))
);

let possibleDesc = descriptorFor(obj, keyName, m);
let isDescriptor = possibleDesc !== undefined;

if (!isDescriptor) {
let baseValue = obj[keyName];

if (hasConcatenatedProps && concatenatedProperties.indexOf(keyName) > -1) {
if (baseValue) {
value = makeArray(baseValue).concat(value);
} else {
value = makeArray(value);
}
}

if (hasMergedProps && mergedProperties.indexOf(keyName) > -1) {
value = assign({}, baseValue, value);
}
}

if (isDescriptor) {
possibleDesc.set(obj, keyName, value);
} else if (typeof obj.setUnknownProperty === 'function' && !(keyName in obj)) {
obj.setUnknownProperty(keyName, value);
} else {
if (DEBUG) {
defineProperty(obj, keyName, null, value, m); // setup mandatory setter
} else {
obj[keyName] = value;
}
}
}
}

// using DEBUG here to avoid the extraneous variable when not needed
if (DEBUG) {
initCalled.add(obj);
}
obj.init(properties);

// re-enable chains
m.proto = obj.constructor.prototype;
finishChains(m);
sendEvent(obj, 'init', undefined, undefined, undefined, m);
}

/**
@class CoreObject
@public
Expand All @@ -59,13 +156,6 @@ class CoreObject {

let self = this;

let beforeInitCalled; // only used in debug builds to enable the proxy trap

// using DEBUG here to avoid the extraneous variable when not needed
if (DEBUG) {
beforeInitCalled = true;
}

if (DEBUG && HAS_NATIVE_PROXY && typeof self.unknownProperty === 'function') {
let messageFor = (obj, property) => {
return (
Expand All @@ -87,7 +177,8 @@ class CoreObject {
if (property === PROXY_CONTENT) {
return target;
} else if (
beforeInitCalled ||
// init called will be set on the proxy, not the target, so get with the receiver
!initCalled.has(receiver) ||
typeof property === 'symbol' ||
isInternalSymbol(property) ||
property === 'toJSON' ||
Expand Down Expand Up @@ -116,92 +207,22 @@ class CoreObject {
FACTORY_FOR.set(self, initFactory);
}

// disable chains
let m = meta(self);
let proto = m.proto;
m.proto = self;

if (properties !== undefined) {
assert(
'EmberObject.create only accepts objects.',
typeof properties === 'object' && properties !== null
);

assert(
'EmberObject.create no longer supports mixing in other ' +
'definitions, use .extend & .create separately instead.',
!(properties instanceof Mixin)
);

let concatenatedProperties = self.concatenatedProperties;
let mergedProperties = self.mergedProperties;
let hasConcatenatedProps =
concatenatedProperties !== undefined && concatenatedProperties.length > 0;
let hasMergedProps = mergedProperties !== undefined && mergedProperties.length > 0;

let keyNames = Object.keys(properties);

for (let i = 0; i < keyNames.length; i++) {
let keyName = keyNames[i];
let value = properties[keyName];

assert(
'EmberObject.create no longer supports defining computed ' +
'properties. Define computed properties using extend() or reopen() ' +
'before calling create().',
!(value instanceof ComputedProperty)
);
assert(
'EmberObject.create no longer supports defining methods that call _super.',
!(typeof value === 'function' && value.toString().indexOf('._super') !== -1)
);
assert(
'`actions` must be provided at extend time, not at create time, ' +
'when Ember.ActionHandler is used (i.e. views, controllers & routes).',
!(keyName === 'actions' && ActionHandler.detect(this))
);

let possibleDesc = descriptorFor(self, keyName, m);
let isDescriptor = possibleDesc !== undefined;

if (!isDescriptor) {
let baseValue = self[keyName];

if (hasConcatenatedProps && concatenatedProperties.indexOf(keyName) > -1) {
if (baseValue) {
value = makeArray(baseValue).concat(value);
} else {
value = makeArray(value);
}
}

if (hasMergedProps && mergedProperties.indexOf(keyName) > -1) {
value = assign({}, baseValue, value);
}
}

if (isDescriptor) {
possibleDesc.set(self, keyName, value);
} else if (typeof self.setUnknownProperty === 'function' && !(keyName in self)) {
self.setUnknownProperty(keyName, value);
} else {
if (DEBUG) {
defineProperty(self, keyName, null, value, m); // setup mandatory setter
} else {
self[keyName] = value;
}
if (properties !== DELAY_INIT) {
deprecate(
'using `new` with EmberObject has been deprecated. Please use `create` instead, or consider using native classes without extending from EmberObject.',
false,
{
id: 'object.new-constructor',
until: '3.9.0',
}
}
}
);

// using DEBUG here to avoid the extraneous variable when not needed
if (DEBUG) {
beforeInitCalled = false;
initialize(self, properties);
}
self.init(...arguments);

m.proto = proto;
finishChains(m);
sendEvent(self, 'init', undefined, undefined, undefined, m);

// only return when in debug builds and `self` is the proxy created above
if (DEBUG && self !== this) {
Expand Down Expand Up @@ -676,12 +697,15 @@ class CoreObject {
*/
static create(props, extra) {
let C = this;
let instance = new C(DELAY_INIT);

if (extra === undefined) {
return new C(props);
initialize(instance, props);
} else {
return new C(flattenProps.apply(this, arguments));
initialize(instance, flattenProps.apply(this, arguments));
}

return instance;
}

/**
Expand Down
8 changes: 4 additions & 4 deletions packages/@ember/-internals/runtime/tests/helpers/array.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ class CopyableNativeArray extends AbstractArrayHelper {

class CopyableArray extends AbstractArrayHelper {
newObject() {
return new CopyableObject();
return CopyableObject.create();
}

isEqual(a, b) {
Expand Down Expand Up @@ -280,15 +280,15 @@ const CopyableObject = EmberObject.extend(Copyable, {
},

copy() {
let ret = new CopyableObject();
let ret = CopyableObject.create();
set(ret, 'id', get(this, 'id'));
return ret;
},
});

class MutableArrayHelpers extends NativeArrayHelpers {
newObject(ary) {
return new TestMutableArray(super.newObject(ary));
return TestMutableArray.create(super.newObject(ary));
}

// allows for testing of the basic enumerable after an internal mutation
Expand All @@ -299,7 +299,7 @@ class MutableArrayHelpers extends NativeArrayHelpers {

class EmberArrayHelpers extends MutableArrayHelpers {
newObject(ary) {
return new TestArray(super.newObject(ary));
return TestArray.create(super.newObject(ary));
}
}

Expand Down
4 changes: 2 additions & 2 deletions packages/@ember/-internals/runtime/tests/mixins/array_test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ moduleFor(
}

['@test slice supports negative index arguments'](assert) {
let testArray = new TestArray({ _content: [1, 2, 3, 4] });
let testArray = TestArray.create({ _content: [1, 2, 3, 4] });

assert.deepEqual(testArray.slice(-2), [3, 4], 'slice(-2)');
assert.deepEqual(testArray.slice(-2, -1), [3], 'slice(-2, -1');
Expand Down Expand Up @@ -248,7 +248,7 @@ moduleFor(
'EmberArray.@each support',
class extends AbstractTestCase {
beforeEach() {
ary = new TestArray({
ary = TestArray.create({
_content: [
{ isDone: true, desc: 'Todo 1' },
{ isDone: false, desc: 'Todo 2' },
Expand Down
44 changes: 20 additions & 24 deletions packages/@ember/-internals/runtime/tests/system/core_object_test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,35 +6,31 @@ import { moduleFor, AbstractTestCase, buildOwner } from 'internal-test-helpers';
moduleFor(
'Ember.CoreObject',
class extends AbstractTestCase {
['@test works with new (one arg)'](assert) {
let obj = new CoreObject({
firstName: 'Stef',
lastName: 'Penner',
});

assert.equal(obj.firstName, 'Stef');
assert.equal(obj.lastName, 'Penner');
}

['@test works with new (> 1 arg)'](assert) {
let obj = new CoreObject(
{
['@test throws deprecation with new (one arg)']() {
expectDeprecation(() => {
new CoreObject({
firstName: 'Stef',
lastName: 'Penner',
},
{
other: 'name',
}
);

assert.equal(obj.firstName, 'Stef');
assert.equal(obj.lastName, 'Penner');
});
}, /using `new` with EmberObject has been deprecated/);
}

assert.equal(obj.other, undefined); // doesn't support multiple pojo' to the constructor
['@test throws deprecation with new (> 1 arg)']() {
expectDeprecation(() => {
new CoreObject(
{
firstName: 'Stef',
lastName: 'Penner',
},
{
other: 'name',
}
);
}, /using `new` with EmberObject has been deprecated/);
}

['@test toString should be not be added as a property when calling toString()'](assert) {
let obj = new CoreObject({
let obj = CoreObject.create({
firstName: 'Foo',
lastName: 'Bar',
});
Expand All @@ -54,7 +50,7 @@ moduleFor(
},
}).create();

let obj = new CoreObject({
let obj = CoreObject.create({
someProxyishThing,
});

Expand Down
Loading

0 comments on commit 0554471

Please sign in to comment.