[BUGFIX release-3-28] Fix #19867

The way most Ember apps are setup in late 3.28 (an implicit injection of `store` but also an explicit injection) was not under test. This patch adds a test, and fixes a bug when applications use that pattern. The prior implementation of the `store` injection deprecation on routes (added in #19854) used a wrapper `DeprecatedStoreInjection` in some cases which was unwrapped by the store getter. A deprecation implemented on `CoreObject` for when a explicit and implicit deprecation mis-match conflicted with that logic. Here, refactor away from the `DeprecatedStoreInjection` wrapper to instead use a WeakSet to track injected instances.
emberjs · Dec 2, 2021 · 6a84367 · 6a84367
1 parent 147063f
commit 6a84367
Show file tree

Hide file tree

Showing 4 changed files with 51 additions and 13 deletions.
diff --git a/packages/@ember/-internals/container/index.ts b/packages/@ember/-internals/container/index.ts
@@ -11,5 +11,5 @@ export {
   getFactoryFor,
   setFactoryFor,
   INIT_FACTORY,
-  DeprecatedStoreInjection,
+  deprecatedStoreInjections,
 } from './lib/container';
diff --git a/packages/@ember/-internals/container/lib/container.ts b/packages/@ember/-internals/container/lib/container.ts
@@ -49,12 +49,7 @@ if (DEBUG) {
   }
 }
 
-export class DeprecatedStoreInjection {
-  store: unknown;
-  constructor(store: unknown) {
-    this.store = store;
-  }
-}
+export const deprecatedStoreInjections = DEBUG && WeakSet ? new WeakSet<object>() : undefined;
 
 export interface ContainerOptions {
   owner?: Owner;
@@ -479,8 +474,8 @@ function injectionsFor(container: Container, fullName: string) {
 
   let result = buildInjections(container, typeInjections, injections);
 
-  if (DEBUG && type === 'route' && result.injections.store) {
-    result.injections.store = new DeprecatedStoreInjection(result.injections.store);
+  if (DEBUG && deprecatedStoreInjections && type === 'route' && result.injections.store) {
+    deprecatedStoreInjections.add(result.injections.store as object);
   }
   return result;
 }

diff --git a/packages/@ember/-internals/routing/lib/system/route.ts b/packages/@ember/-internals/routing/lib/system/route.ts
@@ -1,4 +1,4 @@
-import { DeprecatedStoreInjection, privatize as P } from '@ember/-internals/container';
+import { deprecatedStoreInjections, privatize as P } from '@ember/-internals/container';
 import {
   addObserver,
   computed,
@@ -2377,7 +2377,7 @@ Route.reopen(ActionHandler, Evented, {
     },
 
     set(key, value) {
-      if (DEBUG && value instanceof DeprecatedStoreInjection) {
+      if (DEBUG && deprecatedStoreInjections?.has(value)) {
         Object.defineProperty(this, key, {
           configurable: true,
           enumerable: false,
@@ -2395,7 +2395,7 @@ Route.reopen(ActionHandler, Evented, {
                 },
               }
             );
-            return value.store;
+            return value;
           },
         });
       } else {

diff --git a/packages/@ember/-internals/routing/tests/system/route_test.js b/packages/@ember/-internals/routing/tests/system/route_test.js
@@ -68,7 +68,6 @@ moduleFor(
 
     ["@test 'store' can be injected by data persistence frameworks [DEPRECATED]"](assert) {
       assert.expect(9);
-      expectDeprecation();
       runDestroy(route);
 
       let owner = buildOwner();
@@ -93,6 +92,50 @@ moduleFor(
 
       route = owner.lookup('route:index');
 
+      expectDeprecation(
+        `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.`
+      );
+      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 injected by data persistence frameworks but explicitly injected"](
+      assert
+    ) {
+      assert.expect(9);
+      runDestroy(route);
+
+      let owner = buildOwner();
+
+      let post = {
+        id: 1,
+      };
+
+      let Store = EmberObject.extend({
+        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: injectService(),
+        })
+      );
+      owner.register('service:store', Store);
+
+      owner.inject('route', 'store', 'service:store');
+
+      route = owner.lookup('route:index');
+
+      expectNoDeprecation();
+
       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');