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.

Sign up for GitHub

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

Jump to bottom

[core-object] Delay init until after construction #16795

Merged
merged 2 commits into from
Sep 26, 2018
Merged

[core-object] Delay init until after construction #16795

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
428dfbf
[core-object] Delay init until after construction
pzuraq Jul 5, 2018
9a817d4
remove new usage from tests
pzuraq Jul 6, 2018
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
206 changes: 115 additions & 91 deletions packages/@ember/-internals/runtime/lib/system/core_object.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,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 @@ -38,6 +38,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 @@ -60,13 +157,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 @@ -88,7 +178,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 @@ -117,92 +208,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 @@ -677,12 +698,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
Open in desktop
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
Open in desktop
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
Open in desktop
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