From c9e0abe1974839ea2222ccae08077dcbf8059766 Mon Sep 17 00:00:00 2001 From: Matthew Beale Date: Wed, 24 Nov 2021 15:47:32 -0500 Subject: [PATCH] Improve implicit injections deprecation for routes The store property on routes uses a setter, and so was not impacted by the implicit-injections deprecation issued elsewhere. This may lead app or addon authors to miss usage of a deprecated store injection on the road to 4.0. This improvement of the deprecation fidelity means libraries which use the type injection system to define a store, like Ember Data, should consider another approach to maintain any undeprecated API (for example reopening the route class). --- packages/@ember/-internals/container/index.ts | 8 +- .../-internals/container/lib/container.ts | 14 ++- .../-internals/routing/lib/system/route.ts | 27 ++++- .../routing/tests/system/route_test.js | 111 +++++++++++++++++- 4 files changed, 153 insertions(+), 7 deletions(-) diff --git a/packages/@ember/-internals/container/index.ts b/packages/@ember/-internals/container/index.ts index d3c5006ba40..24100fa997f 100644 --- a/packages/@ember/-internals/container/index.ts +++ b/packages/@ember/-internals/container/index.ts @@ -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'; diff --git a/packages/@ember/-internals/container/lib/container.ts b/packages/@ember/-internals/container/lib/container.ts index ffde24b9385..116057e542c 100644 --- a/packages/@ember/-internals/container/lib/container.ts +++ b/packages/@ember/-internals/container/lib/container.ts @@ -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 }; @@ -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 { diff --git a/packages/@ember/-internals/routing/lib/system/route.ts b/packages/@ember/-internals/routing/lib/system/route.ts index 69dcd37e48b..980d49bcbdc 100644 --- a/packages/@ember/-internals/routing/lib/system/route.ts +++ b/packages/@ember/-internals/routing/lib/system/route.ts @@ -1,4 +1,4 @@ -import { privatize as P } from '@ember/-internals/container'; +import { DeprecatedStoreInjection, privatize as P } from '@ember/-internals/container'; import { addObserver, computed, @@ -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, { + 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); + } }, }), diff --git a/packages/@ember/-internals/routing/tests/system/route_test.js b/packages/@ember/-internals/routing/tests/system/route_test.js index bcf4bfa32a5..8a78c873cda 100644 --- a/packages/@ember/-internals/routing/tests/system/route_test.js +++ b/packages/@ember/-internals/routing/tests/system/route_test.js @@ -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 = {}; @@ -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(); @@ -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();