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.

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

[BUGFIX 3.28] Improve implicit injections deprecation for routes #19854

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 7 additions & 1 deletion packages/@ember/-internals/container/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,10 @@ The public API, specified on the application namespace should be considered the
*/

export { default as Registry, privatize } from './lib/registry';
export { default as Container, getFactoryFor, setFactoryFor, INIT_FACTORY } from './lib/container';
export {
default as Container,
getFactoryFor,
setFactoryFor,
INIT_FACTORY,
DeprecatedStoreInjection,
} from './lib/container';
14 changes: 13 additions & 1 deletion packages/@ember/-internals/container/lib/container.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,13 @@ if (DEBUG) {
}
}

export class DeprecatedStoreInjection {
store: unknown;
constructor(store: unknown) {
this.store = store;
}
}

export interface ContainerOptions {
owner?: Owner;
cache?: { [key: string]: CacheMember };
Expand Down Expand Up @@ -470,7 +477,12 @@ function injectionsFor(container: Container, fullName: string) {
let typeInjections = registry.getTypeInjections(type);
let injections = registry.getInjections(fullName);

return buildInjections(container, typeInjections, injections);
let result = buildInjections(container, typeInjections, injections);

if (DEBUG && type === 'route' && result.injections.store) {
result.injections.store = new DeprecatedStoreInjection(result.injections.store);
}
return result;
}

function destroyDestroyables(container: Container): void {
Expand Down
27 changes: 25 additions & 2 deletions packages/@ember/-internals/routing/lib/system/route.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { privatize as P } from '@ember/-internals/container';
import { DeprecatedStoreInjection, privatize as P } from '@ember/-internals/container';
import {
addObserver,
computed,
Expand Down Expand Up @@ -2364,7 +2364,30 @@ Route.reopen(ActionHandler, Evented, {
},

set(key, value) {
defineProperty(this, key, null, value);
if (DEBUG && value instanceof DeprecatedStoreInjection) {
Object.defineProperty(this, key, {
mixonic marked this conversation as resolved.
Show resolved Hide resolved
configurable: true,
enumerable: false,
get(): any {
deprecate(
`A value for the \`store\` property was injected onto a route via the owner API. Implicit injection via the owner API is now deprecated, please add an explicit injection for this value. If the injected value is a service, consider using the @service decorator.`,
false,
{
id: 'implicit-injections',
until: '4.0.0',
url: 'https://deprecations.emberjs.com/v3.x/#toc_implicit-injections',
for: 'ember-source',
since: {
enabled: '3.28.7',
},
}
);
return value.store;
},
});
} else {
defineProperty(this, key, null, value);
}
},
}),

Expand Down
111 changes: 108 additions & 3 deletions packages/@ember/-internals/routing/tests/system/route_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@ moduleFor(
}

['@test default store utilizes the container to acquire the model factory'](assert) {
assert.expect(4);
assert.expect(5);

expectNoDeprecation();

let Post = EmberObject.extend();
let post = {};
Expand Down Expand Up @@ -64,8 +66,9 @@ moduleFor(
runDestroy(owner);
}

["@test 'store' can be injected by data persistence frameworks"](assert) {
assert.expect(8);
["@test 'store' can be injected by data persistence frameworks [DEPRECATED]"](assert) {
assert.expect(9);
expectDeprecation();
runDestroy(route);

let owner = buildOwner();
Expand Down Expand Up @@ -96,8 +99,110 @@ moduleFor(
runDestroy(owner);
}

["@test 'store' can be set via assignment by data persistence frameworks"](assert) {
assert.expect(9);
expectNoDeprecation();
runDestroy(route);

let owner = buildOwner();

let post = {
id: 1,
};

let store = {
find(type, value) {
assert.ok(true, 'injected model was called');
assert.equal(type, 'post', 'correct type was called');
assert.equal(value, 1, 'correct value was called');
return post;
},
};

owner.register('route:index', EmberRoute);

route = owner.lookup('route:index');
route.store = store;

assert.equal(route.model({ post_id: 1 }), post, '#model returns the correct post');
assert.equal(route.findModel('post', 1), post, '#findModel returns the correct post');

runDestroy(owner);
}

["@test 'store' can be set on subclass by data persistence frameworks"](assert) {
assert.expect(9);
expectNoDeprecation();
runDestroy(route);

let owner = buildOwner();

let post = {
id: 1,
};

let store = {
find(type, value) {
assert.ok(true, 'injected model was called');
assert.equal(type, 'post', 'correct type was called');
assert.equal(value, 1, 'correct value was called');
return post;
},
};

owner.register(
'route:index',
EmberRoute.extend({
store,
})
);

route = owner.lookup('route:index');

assert.equal(route.model({ post_id: 1 }), post, '#model returns the correct post');
assert.equal(route.findModel('post', 1), post, '#findModel returns the correct post');

runDestroy(owner);
}

["@test 'store' can be set via reopen by data persistence frameworks"](assert) {
assert.expect(9);
expectNoDeprecation();
runDestroy(route);

let owner = buildOwner();

let post = {
id: 1,
};

let store = {
find(type, value) {
assert.ok(true, 'injected model was called');
assert.equal(type, 'post', 'correct type was called');
assert.equal(value, 1, 'correct value was called');
return post;
},
};

let EmberRouteSubclassForReopen = EmberRoute.extend();
EmberRouteSubclassForReopen.reopen({
store,
});

owner.register('route:index', EmberRouteSubclassForReopen);

route = owner.lookup('route:index');

assert.equal(route.model({ post_id: 1 }), post, '#model returns the correct post');
assert.equal(route.findModel('post', 1), post, '#findModel returns the correct post');

runDestroy(owner);
}

["@test assert if 'store.find' method is not found"]() {
runDestroy(route);
expectNoDeprecation();

let owner = buildOwner();
let Post = EmberObject.extend();
Expand Down