Skip to content

Commit

Permalink
RFC: 680 - Deprecate Implicit Injection on arbitrary Ember Framework …
Browse files Browse the repository at this point in the history
…objects
  • Loading branch information
snewcomer committed Jan 26, 2021
1 parent f2be195 commit 206379c
Show file tree
Hide file tree
Showing 14 changed files with 352 additions and 75 deletions.
20 changes: 10 additions & 10 deletions packages/@ember/-internals/extension-support/lib/data_adapter.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,18 +53,18 @@ export default EmberObject.extend({
init() {
this._super(...arguments);
this.releaseMethods = emberA();
},

/**
The container-debug-adapter which is used
to list all models.
/**
The container-debug-adapter which is used
to list all models.
@property containerDebugAdapter
@default undefined
@since 1.5.0
@public
**/
containerDebugAdapter: undefined,
@property containerDebugAdapter
@default undefined
@since 1.5.0
@public
**/
this.containerDebugAdapter = getOwner(this).lookup('container-debug-adapter:main');
},

/**
The number of attributes to send
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,17 @@ const DataAdapter = EmberDataAdapter.extend({
},
init() {
this._super(...arguments);
this.set('containerDebugAdapter', {
canCatalogEntriesByType() {
return true;
},
catalogEntriesByType() {
return emberA(['post']);
},
Object.defineProperty(this, 'containerDebugAdapter', {
get() {
return {
canCatalogEntriesByType() {
return true;
},
catalogEntriesByType() {
return emberA(['post']);
},
}
}
});
},
});
Expand Down
10 changes: 10 additions & 0 deletions packages/@ember/-internals/routing/lib/ext/controller.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { get } from '@ember/-internals/metal';
import { getOwner } from '@ember/-internals/owner';
import ControllerMixin from '@ember/controller/lib/controller_mixin';
import { deprecateTransitionMethods, prefixRouteNameArg } from '../utils';

Expand All @@ -9,6 +10,15 @@ import { deprecateTransitionMethods, prefixRouteNameArg } from '../utils';
ControllerMixin.reopen({
concatenatedProperties: ['queryParams'],

init() {
this._super(...arguments);
let owner = getOwner(this);
if (owner) {
this.namespace = owner.lookup('application:main');
this.target = owner.lookup('router:main');
}
}

/**
Defines which query parameters the controller accepts.
If you give the names `['category','page']` it will bind
Expand Down
17 changes: 15 additions & 2 deletions packages/@ember/-internals/routing/lib/system/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
setProperties,
} from '@ember/-internals/metal';
import { getOwner, Owner } from '@ember/-internals/owner';
import { BucketCache } from '@ember/-internals/routing';
import {
A as emberA,
ActionHandler,
Expand Down Expand Up @@ -98,8 +99,22 @@ class Route extends EmberObject implements IRoute {
controller!: Controller;
currentModel: unknown;

_bucketCache: BucketCache | undefined;
_internalName!: string;
_names: unknown;
_router: EmberRouter | undefined;

constructor() {
super(...arguments);

let owner = getOwner(this);
if (owner) {
this._router = owner.lookup('router:main');
this._bucketCache = owner.lookup('-bucket-cache:main');
this._topLevelViewTemplate = owner.lookup('template:-outlet');
this._environment = owner.lookup('-environment:main');
}
}

serialize!: (
model: {},
Expand All @@ -110,8 +125,6 @@ class Route extends EmberObject implements IRoute {
}
| undefined;

_router!: EmberRouter;

/**
The name of the route, dot-delimited.
Expand Down
7 changes: 7 additions & 0 deletions packages/@ember/-internals/routing/lib/system/router.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { OutletState as GlimmerOutletState, OutletView } from '@ember/-internals/glimmer';
import { computed, get, notifyPropertyChange, set } from '@ember/-internals/metal';
import { BucketCache } from '@ember/-internals/routing';
import { FactoryClass, getOwner, Owner } from '@ember/-internals/owner';
import { A as emberA, Evented, Object as EmberObject, typeOf } from '@ember/-internals/runtime';
import Controller from '@ember/controller';
Expand Down Expand Up @@ -138,6 +139,7 @@ class EmberRouter extends EmberObject {
_qpUpdates = new Set();
_queuedQPChanges: { [key: string]: unknown } = {};

_bucketCache: BucketCache | undefined;
_toplevelView: OutletView | null = null;
_handledErrors = new Set();
_engineInstances: { [name: string]: { [id: string]: EngineInstance } } = Object.create(null);
Expand All @@ -149,6 +151,11 @@ class EmberRouter extends EmberObject {
super(...arguments);

this._resetQueuedQueryParameterChanges();
let owner = getOwner(this);
if (owner) {
this.namespace = owner.lookup('application:main');
this._bucketCache = owner.lookup('-bucket-cache:main');
}
}

_initRouterJs() {
Expand Down
78 changes: 74 additions & 4 deletions packages/@ember/-internals/runtime/lib/system/core_object.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import {
guidFor,
getName,
setName,
lookupDescriptor,
inspect,
makeArray,
HAS_NATIVE_PROXY,
HAS_NATIVE_SYMBOL,
Expand All @@ -28,7 +30,7 @@ import {
DEBUG_INJECTION_FUNCTIONS,
} 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';
import { _WeakSet as WeakSet } from '@glimmer/util';
import { destroy, isDestroying, isDestroyed, registerDestructor } from '@glimmer/destroyable';
Expand Down Expand Up @@ -64,6 +66,21 @@ function initialize(obj, properties) {
!(properties instanceof Mixin)
);

let injectedProperties;
if (DEBUG) {
// these are all the implicit injectinos
injectedProperties = [];

let factory = getFactoryFor(obj);
if (factory) {
for (let injection in factory.injections) {
if (factory.injections[injection] === properties[injection]) {
injectedProperties.push(injection);
}
}
}
}

let concatenatedProperties = obj.concatenatedProperties;
let mergedProperties = obj.mergedProperties;
let hasConcatenatedProps =
Expand All @@ -73,8 +90,8 @@ function initialize(obj, properties) {
let keyNames = Object.keys(properties);

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

assert(
'EmberObject.create no longer supports defining computed ' +
Expand Down Expand Up @@ -112,12 +129,48 @@ function initialize(obj, properties) {
}

if (isDescriptor) {
if (DEBUG) {
// need to check if implicit injection owner.inject('component:my-component', 'foo', 'service:bar') does not match explicit injection @service foo
// implicit injection takes precedence so need to tell user to rename property on obj
let isInjectedProperty = DEBUG_INJECTION_FUNCTIONS.has(possibleDesc._getter);
if (isInjectedProperty && value !== possibleDesc.get(obj, keyName)) {
implicitInjectionDeprecation(keyName, `You have explicitly defined '${keyName}' for ${inspect(obj)} that does not match the implicit injection for '${keyName}'. Please ensure you are explicitly defining '${keyName}' on ${inspect(obj)}.`);
}
}

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
// If deprecation, either 1) an accessor descriptor or 2) class field declaration 3) only an implicit injection
let desc = lookupDescriptor(obj, keyName);
if (!injectedProperties.includes(keyName)) {
defineProperty(obj, keyName, null, value, m); // setup mandatory setter
} else if (desc && desc.get) {
if (value !== desc.get.call(obj)) {
implicitInjectionDeprecation(keyName, `You have defined '${keyName}' for ${inspect(obj)} as a getter which does not match the implicit injection for '${keyName}'. Please migrate to '@service ${keyName}'.`);
}
} else if (desc && desc.value) {
if (desc.value !== value) {
// implicit injection does not match value descriptor set on object
implicitInjectionDeprecation(keyName, `You have defined '${keyName}' for ${inspect(obj)} as a value which does not match the implicit injection for '${keyName}'. Please migrate to '@service ${keyName}'.`);
}
} else {
// wrapper getter for original adding one time deprecation
// TODO add setter
Object.defineProperty(obj, keyName, {
configurable: true,
enumerable: false,
get() {
// only want to deprecate on first access
Object.defineProperty(obj, keyName, { value });

implicitInjectionDeprecation(keyName, `Implicit injection for property '${keyName}' is now deprecated. Please add an explicit injection for '${keyName}' to ${inspect(obj)}`);
return value;
}
});
}
} else {
obj[keyName] = value;
}
Expand Down Expand Up @@ -1120,4 +1173,21 @@ if (!HAS_NATIVE_SYMBOL) {
});
}

// TODO: customize messages and add more debugging info like obj and mismatch information
function implicitInjectionDeprecation(keyName, msg = null) {
deprecate(
msg,
false,
{
id: 'ember-object.implicit-injection',
until: '4.0.0',
url: 'https://',
for: 'ember-source',
since: {
enabled: '3.24.0',
},
}
);
}

export default CoreObject;
Loading

0 comments on commit 206379c

Please sign in to comment.