Skip to content

Commit

Permalink
[FEATURE] Implement injection parameter normalization RFC. (#17858)
Browse files Browse the repository at this point in the history
[FEATURE] Implement injection parameter normalization RFC.

Co-authored-by: Robert Jackson <rjackson@linkedin.com>
  • Loading branch information
rwjblue and rwjblue committed May 9, 2019
2 parents d93c663 + 0b47cd2 commit 1731b14
Show file tree
Hide file tree
Showing 16 changed files with 212 additions and 28 deletions.
7 changes: 6 additions & 1 deletion packages/@ember/-internals/glimmer/lib/component.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { get, PROPERTY_DID_CHANGE } from '@ember/-internals/metal';
import { getOwner } from '@ember/-internals/owner';
import { TargetActionSupport } from '@ember/-internals/runtime';
import { setFrameworkClass, TargetActionSupport } from '@ember/-internals/runtime';
import { symbol } from '@ember/-internals/utils';
import {
ActionSupport,
Expand All @@ -16,6 +16,7 @@ import { DEBUG } from '@glimmer/env';
import { DirtyableTag } from '@glimmer/reference';
import { normalizeProperty, SVG_NAMESPACE } from '@glimmer/runtime';

import { EMBER_FRAMEWORK_OBJECT_OWNER_ARGUMENT } from '@ember/canary-features';
import { RootReference, UPDATE } from './utils/references';

export const DIRTY_TAG = symbol('DIRTY_TAG');
Expand Down Expand Up @@ -1086,4 +1087,8 @@ Component.reopenClass({
positionalParams: [],
});

if (EMBER_FRAMEWORK_OBJECT_OWNER_ARGUMENT) {
setFrameworkClass(Component);
}

export default Component;
7 changes: 6 additions & 1 deletion packages/@ember/-internals/glimmer/lib/helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,9 @@
*/

import { Factory } from '@ember/-internals/owner';
import { FrameworkObject } from '@ember/-internals/runtime';
import { FrameworkObject, setFrameworkClass } from '@ember/-internals/runtime';
import { symbol } from '@ember/-internals/utils';
import { EMBER_FRAMEWORK_OBJECT_OWNER_ARGUMENT } from '@ember/canary-features';
import { Dict, Opaque } from '@glimmer/interfaces';
import { DirtyableTag } from '@glimmer/reference';

Expand Down Expand Up @@ -131,6 +132,10 @@ let Helper = FrameworkObject.extend({

Helper.isHelperFactory = true;

if (EMBER_FRAMEWORK_OBJECT_OWNER_ARGUMENT) {
setFrameworkClass(Helper);
}

class Wrapper implements HelperFactory<SimpleHelper> {
isHelperFactory: true = true;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3594,6 +3594,30 @@ moduleFor(

this.assertText('[third][]');
}

['@feature(EMBER_FRAMEWORK_OBJECT_OWNER_ARGUMENT) it can render a basic component in native ES class syntax'](
assert
) {
let testContext = this;
this.registerComponent('foo-bar', {
ComponentClass: class extends Component {
constructor(owner) {
super(owner);

assert.equal(owner, testContext.owner, 'owner was passed as a constructor argument');
}
},
template: 'hello',
});

this.render('{{foo-bar}}');

this.assertComponentElement(this.firstChild, { content: 'hello' });

runTask(() => this.rerender());

this.assertComponentElement(this.firstChild, { content: 'hello' });
}
}
);

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/* globals EmberDev */

import { RenderingTestCase, moduleFor, runDestroy, runTask } from 'internal-test-helpers';

import { Helper } from '@ember/-internals/glimmer';
import { set } from '@ember/-internals/metal';

moduleFor(
Expand Down Expand Up @@ -599,6 +599,30 @@ moduleFor(
assert.equal(typeof instance.compute, 'function', 'expected instance.compute to be present');
assert.equal(instance.compute(), 'lolol', 'can invoke `.compute`');
}

['@feature(EMBER_FRAMEWORK_OBJECT_OWNER_ARGUMENT) class-based helper in native ES syntax receives owner'](
assert
) {
let testContext = this;
this.add(
'helper:hello-world',
class extends Helper {
constructor(owner) {
super(owner);

assert.equal(owner, testContext.owner, 'owner was passed as a constructor argument');
}

compute() {
return 'huzza!';
}
}
);

this.render('{{hello-world}}');

this.assertText('huzza!');
}
}
);

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { get } from '@ember/-internals/metal';
import { Owner } from '@ember/-internals/owner';
import { Factory, Owner } from '@ember/-internals/owner';
import { info } from '@ember/debug';
import { DEBUG } from '@glimmer/env';
/**
Expand All @@ -14,7 +14,7 @@ import { DEBUG } from '@glimmer/env';
@private
*/

export function generateControllerFactory(owner: Owner, controllerName: string) {
export function generateControllerFactory(owner: Owner, controllerName: string): Factory<{}> {
let Factory = owner.factoryFor<any, any>('controller:basic')!.class!;

Factory = Factory.extend({
Expand All @@ -27,7 +27,7 @@ export function generateControllerFactory(owner: Owner, controllerName: string)

owner.register(fullName, Factory);

return Factory;
return owner.factoryFor(fullName) as Factory<{}>;
}

/**
Expand Down
6 changes: 6 additions & 0 deletions packages/@ember/-internals/routing/lib/system/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,11 @@ import {
ActionHandler,
Evented,
Object as EmberObject,
setFrameworkClass,
typeOf,
} from '@ember/-internals/runtime';
import {
EMBER_FRAMEWORK_OBJECT_OWNER_ARGUMENT,
EMBER_METAL_TRACKED_PROPERTIES,
EMBER_ROUTING_BUILD_ROUTEINFO_METADATA,
} from '@ember/canary-features';
Expand Down Expand Up @@ -2627,4 +2629,8 @@ if (EMBER_ROUTING_BUILD_ROUTEINFO_METADATA) {
});
}

if (EMBER_FRAMEWORK_OBJECT_OWNER_ARGUMENT) {
setFrameworkClass(Route);
}

export default Route;
1 change: 1 addition & 0 deletions packages/@ember/-internals/runtime/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ export function deprecatingAlias(
): any;

export const FrameworkObject: any;
export function setFrameworkClass<T>(klass: new () => T): void;
export const Object: any;

export function _contentFor(proxy: any): any;
Expand Down
2 changes: 1 addition & 1 deletion packages/@ember/-internals/runtime/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ export { default as Comparable } from './lib/mixins/comparable';
export { default as Namespace } from './lib/system/namespace';
export { default as ArrayProxy } from './lib/system/array_proxy';
export { default as ObjectProxy } from './lib/system/object_proxy';
export { default as CoreObject } from './lib/system/core_object';
export { default as CoreObject, setFrameworkClass } from './lib/system/core_object';
export { default as ActionHandler } from './lib/mixins/action_handler';
export { default as Copyable } from './lib/mixins/copyable';
export { default as Enumerable } from './lib/mixins/enumerable';
Expand Down
66 changes: 56 additions & 10 deletions packages/@ember/-internals/runtime/lib/system/core_object.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,21 @@
*/

import { FACTORY_FOR } from '@ember/-internals/container';
import { getOwner } from '@ember/-internals/owner';
import { assign, _WeakSet as WeakSet } from '@ember/polyfills';
import {
guidFor,
getName,
setName,
symbol,
makeArray,
HAS_NATIVE_PROXY,
isInternalSymbol,
} from '@ember/-internals/utils';
import { EMBER_METAL_TRACKED_PROPERTIES } from '@ember/canary-features';
import {
EMBER_METAL_TRACKED_PROPERTIES,
EMBER_FRAMEWORK_OBJECT_OWNER_ARGUMENT,
} from '@ember/canary-features';
import { schedule } from '@ember/runloop';
import { meta, peekMeta, deleteMeta } from '@ember/-internals/meta';
import {
Expand Down Expand Up @@ -40,13 +45,15 @@ const factoryMap = new WeakMap();

const prototypeMixinMap = new WeakMap();

let PASSED_FROM_CREATE;
let initCalled; // only used in debug builds to enable the proxy trap
const initCalled = DEBUG ? new WeakSet() : undefined; // only used in debug builds to enable the proxy trap
const PASSED_FROM_CREATE = DEBUG ? symbol('PASSED_FROM_CREATE') : undefined;

// using DEBUG here to avoid the extraneous variable when not needed
if (DEBUG) {
PASSED_FROM_CREATE = Symbol();
initCalled = new WeakSet();
const FRAMEWORK_CLASSES = EMBER_FRAMEWORK_OBJECT_OWNER_ARGUMENT
? symbol('FRAMEWORK_CLASS')
: undefined;

export function setFrameworkClass(klass) {
klass[FRAMEWORK_CLASSES] = true;
}

function initialize(obj, properties) {
Expand Down Expand Up @@ -215,7 +222,7 @@ class CoreObject {
factoryMap.set(this, factory);
}

constructor(properties) {
constructor(passedFromCreate) {
// pluck off factory
let initFactory = factoryMap.get(this.constructor);
if (initFactory !== undefined) {
Expand Down Expand Up @@ -288,7 +295,25 @@ class CoreObject {
`An EmberObject based class, ${
this.constructor
}, was not instantiated correctly. You may have either used \`new\` instead of \`.create()\`, or not passed arguments to your call to super in the constructor: \`super(...arguments)\`. If you are trying to use \`new\`, consider using native classes without extending from EmberObject.`,
properties[PASSED_FROM_CREATE]
(() => {
if (passedFromCreate === PASSED_FROM_CREATE) {
return true;
}

if (!EMBER_FRAMEWORK_OBJECT_OWNER_ARGUMENT) {
return false;
}

if (initFactory === undefined) {
return false;
}

if (passedFromCreate === initFactory.owner) {
return true;
}

return false;
})()
);

// only return when in debug builds and `self` is the proxy created above
Expand Down Expand Up @@ -764,7 +789,28 @@ class CoreObject {
*/
static create(props, extra) {
let C = this;
let instance = DEBUG ? new C(Object.freeze({ [PASSED_FROM_CREATE]: true })) : new C();
let instance;

if (EMBER_FRAMEWORK_OBJECT_OWNER_ARGUMENT && this[FRAMEWORK_CLASSES]) {
let initFactory = factoryMap.get(this);
let owner;
if (initFactory !== undefined) {
owner = initFactory.owner;
} else if (props !== undefined) {
owner = getOwner(props);
}

if (owner === undefined) {
// fallback to passing the special PASSED_FROM_CREATE symbol
// to avoid an error when folks call things like Controller.extend().create()
// we should do a subsequent deprecation pass to ensure this isn't allowed
owner = PASSED_FROM_CREATE;
}

instance = new C(owner);
} else {
instance = DEBUG ? new C(PASSED_FROM_CREATE) : new C();
}

if (extra === undefined) {
initialize(instance, props);
Expand Down
25 changes: 22 additions & 3 deletions packages/@ember/-internals/runtime/lib/system/object.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,10 @@
*/

import { FACTORY_FOR } from '@ember/-internals/container';
import { OWNER } from '@ember/-internals/owner';
import { OWNER, setOwner } from '@ember/-internals/owner';
import { symbol, setName } from '@ember/-internals/utils';
import { addListener } from '@ember/-internals/metal';
import { EMBER_FRAMEWORK_OBJECT_OWNER_ARGUMENT } from '@ember/canary-features';
import CoreObject from './core_object';
import Observable from '../mixins/observable';
import { assert } from '@ember/debug';
Expand Down Expand Up @@ -49,13 +50,31 @@ setName(EmberObject, 'Ember.Object');

Observable.apply(EmberObject.prototype);

export let FrameworkObject = EmberObject;
export let FrameworkObject;

if (EMBER_FRAMEWORK_OBJECT_OWNER_ARGUMENT) {
FrameworkObject = class FrameworkObject extends CoreObject {
get _debugContainerKey() {
let factory = FACTORY_FOR.get(this);
return factory !== undefined && factory.fullName;
}

constructor(owner) {
super();

setOwner(this, owner);
}
};
Observable.apply(FrameworkObject.prototype);
} else {
FrameworkObject = class FrameworkObject extends EmberObject {};
}

if (DEBUG) {
let INIT_WAS_CALLED = symbol('INIT_WAS_CALLED');
let ASSERT_INIT_WAS_CALLED = symbol('ASSERT_INIT_WAS_CALLED');

FrameworkObject = class FrameworkObject extends EmberObject {
FrameworkObject = class DebugFrameworkObject extends EmberObject {
init() {
super.init(...arguments);
this[INIT_WAS_CALLED] = true;
Expand Down
6 changes: 3 additions & 3 deletions packages/@ember/-internals/runtime/lib/type-of.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import EmberObject from './system/object';
import CoreObject from './system/core_object';

// ........................................
// TYPING & ARRAY MESSAGING
Expand Down Expand Up @@ -90,13 +90,13 @@ export function typeOf(item) {
let ret = TYPE_MAP[toString.call(item)] || 'object';

if (ret === 'function') {
if (EmberObject.detect(item)) {
if (CoreObject.detect(item)) {
ret = 'class';
}
} else if (ret === 'object') {
if (item instanceof Error) {
ret = 'error';
} else if (item instanceof EmberObject) {
} else if (item instanceof CoreObject) {
ret = 'instance';
} else if (item instanceof Date) {
ret = 'date';
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import { EMBER_FRAMEWORK_OBJECT_OWNER_ARGUMENT } from '@ember/canary-features';
import { getOwner } from '@ember/-internals/owner';
import { FrameworkObject } from '../../../index';
import { moduleFor, AbstractRenderingTestCase } from 'internal-test-helpers';
import { setFrameworkClass } from '../../../lib/system/core_object';

if (EMBER_FRAMEWORK_OBJECT_OWNER_ARGUMENT) {
moduleFor(
'FrameworkObject',
class extends AbstractRenderingTestCase {
['@test tunnels the owner through to the base constructor for framework classes'](assert) {
assert.expect(2);

let testContext = this;
class Model extends FrameworkObject {
constructor(owner) {
super(owner);

assert.equal(
getOwner(this),
testContext.owner,
'owner was assigned properly in the root constructor'
);

assert.equal(owner, testContext.owner, 'owner was passed properly to the constructor');
}
}
setFrameworkClass(Model);
this.owner.register('model:blah', Model);

this.owner.factoryFor('model:blah').create();
}
}
);
}
Loading

0 comments on commit 1731b14

Please sign in to comment.