[RFC-774] Deprecate implicit record loading in Ember Route

Author: wagenet

packages/@ember/routing/route.ts

@@ -17,7 +17,7 @@ import { isProxy, lookupDescriptor } from '@ember/-internals/utils';
 import type { AnyFn } from '@ember/-internals/utility-types';
 import Controller from '@ember/controller';
 import type { ControllerQueryParamType } from '@ember/controller';
-import { assert, info, isTesting } from '@ember/debug';
+import { assert, deprecate, info, isTesting } from '@ember/debug';
 import EngineInstance from '@ember/engine/instance';
 import { dependentKeyCompat } from '@ember/object/compat';
 import { once } from '@ember/runloop';
@@ -66,6 +66,16 @@ type RouteTransitionState<R extends Route> = TransitionState<R> & {
 type MaybeParameters<T> = T extends AnyFn ? Parameters<T> : unknown[];
 type MaybeReturnType<T> = T extends AnyFn ? ReturnType<T> : unknown;
 
+interface StoreLike {
+  find(type: string, value: unknown): unknown;
+}
+
+function isStoreLike(store: unknown): store is StoreLike {
+  return (
+    typeof store === 'object' && store !== null && typeof (store as StoreLike).find === 'function'
+  );
+}
+
 export const ROUTE_CONNECTIONS = new WeakMap();
 const RENDER = Symbol('render');
 
@@ -1479,8 +1489,22 @@ class Route<T = unknown>
     @param {Object} value the value passed to find
     @private
   */
-  findModel(...args: any[]) {
-    return (get(this, 'store') as any).find(...args);
+  findModel(type: string, value: unknown) {
+    deprecate(
+      `The implicit model loading behavior for routes is deprecated. Please define an explicit model hook for ${this.fullRouteName}.`,
+      false,
+      {
+        id: 'deprecate-implicit-route-model',
+        for: 'ember-source',
+        since: { available: '4.12.0' },
+        until: '6.0.0',
+      }
+    );
+
+    const store = get(this, 'store');
+    assert('Expected route to have a store with a find method', isStoreLike(store));
+
+    return store.find(type, value);
   }
 
   /**

packages/@ember/routing/tests/system/route_test.js

@@ -23,7 +23,7 @@ moduleFor(
     }
 
     ['@test default store utilizes the container to acquire the model factory'](assert) {
-      assert.expect(4);
+      assert.expect(5);
 
       let Post = EmberObject.extend();
       let post = {};
@@ -55,8 +55,14 @@ moduleFor(
       let owner = buildOwner(ownerOptions);
       setOwner(route, owner);
 
-      assert.equal(route.model({ post_id: 1 }), post);
-      assert.equal(route.findModel('post', 1), post, '#findModel returns the correct post');
+      expectDeprecation(
+        () =>
+          ignoreAssertion(() => {
+            assert.equal(route.model({ post_id: 1 }), post);
+            assert.equal(route.findModel('post', 1), post, '#findModel returns the correct post');
+          }),
+        /The implicit model loading behavior for routes is deprecated./
+      );
 
       runDestroy(owner);
     }
@@ -77,21 +83,23 @@ moduleFor(
 
       route = owner.lookup('route:index');
 
-      expectAssertion(function () {
-        route.findModel('post', 1);
-      }, `You used the dynamic segment \`post_id\` in your route ` +
-        `\`index\` for which Ember requires you provide a ` +
-        `data-loading implementation. Commonly, that is done by ` +
-        `adding a model hook implementation on the route ` +
-        `(\`model({post_id}) {\`) or by injecting an implemention of ` +
-        `a data store: \`@service store;\`.\n\n` +
-        `Rarely, applications may attempt to use a legacy behavior where ` +
-        `the model class (in this case \`post\`) is resolved and the ` +
-        `\`find\` method on that class is invoked to load data. In this ` +
-        `application, a model of \`post\` was found but it did not ` +
-        `provide a \`find\` method. You should not add a \`find\` ` +
-        `method to your model. Instead, please implement an appropriate ` +
-        `\`model\` hook on the \`index\` route.`);
+      ignoreDeprecation(() =>
+        expectAssertion(function () {
+          route.findModel('post', 1);
+        }, `You used the dynamic segment \`post_id\` in your route ` +
+          `\`index\` for which Ember requires you provide a ` +
+          `data-loading implementation. Commonly, that is done by ` +
+          `adding a model hook implementation on the route ` +
+          `(\`model({post_id}) {\`) or by injecting an implemention of ` +
+          `a data store: \`@service store;\`.\n\n` +
+          `Rarely, applications may attempt to use a legacy behavior where ` +
+          `the model class (in this case \`post\`) is resolved and the ` +
+          `\`find\` method on that class is invoked to load data. In this ` +
+          `application, a model of \`post\` was found but it did not ` +
+          `provide a \`find\` method. You should not add a \`find\` ` +
+          `method to your model. Instead, please implement an appropriate ` +
+          `\`model\` hook on the \`index\` route.`)
+      );
 
       runDestroy(owner);
     }
@@ -109,14 +117,16 @@ moduleFor(
 
       route = owner.lookup('route:index');
 
-      expectAssertion(function () {
-        route.model({ post_id: 1 });
-      }, `You used the dynamic segment \`post_id\` in your route ` +
-        `\`index\` for which Ember requires you provide a ` +
-        `data-loading implementation. Commonly, that is done by ` +
-        `adding a model hook implementation on the route ` +
-        `(\`model({post_id}) {\`) or by injecting an implemention of ` +
-        `a data store: \`@service store;\`.`);
+      ignoreDeprecation(() =>
+        expectAssertion(function () {
+          route.model({ post_id: 1 });
+        }, `You used the dynamic segment \`post_id\` in your route ` +
+          `\`index\` for which Ember requires you provide a ` +
+          `data-loading implementation. Commonly, that is done by ` +
+          `adding a model hook implementation on the route ` +
+          `(\`model({post_id}) {\`) or by injecting an implemention of ` +
+          `a data store: \`@service store;\`.`)
+      );
 
       runDestroy(owner);
     }
@@ -130,9 +140,7 @@ moduleFor(
 
       route = owner.lookup('route:index');
 
-      ignoreAssertion(function () {
-        route.model({ post_id: 1 });
-      });
+      ignoreDeprecation(() => ignoreAssertion(() => route.model({ post_id: 1 })));
 
       assert.ok(true, 'no error was raised');
 

packages/ember/tests/routing/decoupled_basic_test.js

@@ -130,17 +130,29 @@ moduleFor(
 
       this.add('model:menu_item', MenuItem);
 
+      let SpecialRoute = class extends Route {
+        model({ menu_item_id }) {
+          return MenuItem.find(menu_item_id);
+        }
+      };
+
+      this.add('route:special', SpecialRoute);
+
       this.addTemplate('special', '<p>{{@model.id}}</p>');
       this.addTemplate('loading', '<p>LOADING!</p>');
 
-      let visited = runTask(() => this.visit('/specials/1'));
-      this.assertText('LOADING!', 'The app is in the loading state');
+      let promise;
+      ignoreDeprecation(() => {
+        let visited = runTask(() => this.visit('/specials/1'));
+        this.assertText('LOADING!', 'The app is in the loading state');
 
-      resolve(menuItem);
+        resolve(menuItem);
 
-      return visited.then(() => {
-        this.assertText('1', 'The app is now in the specials state');
+        promise = visited.then(() => {
+          this.assertText('1', 'The app is now in the specials state');
+        });
       });
+      return promise;
     }
 
     [`@test The loading state doesn't get entered for promises that resolve on the same run loop`](
@@ -160,6 +172,14 @@ moduleFor(
 
       this.add('model:menu_item', MenuItem);
 
+      let SpecialRoute = class extends Route {
+        model({ menu_item_id }) {
+          return MenuItem.find(menu_item_id);
+        }
+      };
+
+      this.add('route:special', SpecialRoute);
+
       this.add(
         'route:loading',
         Route.extend({
@@ -218,7 +238,9 @@ moduleFor(
         })
       );
 
-      runTask(() => handleURLRejectsWith(this, assert, 'specials/1', 'Setup error'));
+      ignoreDeprecation(() => {
+        runTask(() => handleURLRejectsWith(this, assert, 'specials/1', 'Setup error'));
+      });
 
       resolve(menuItem);
     }
@@ -262,6 +284,10 @@ moduleFor(
       this.add(
         'route:special',
         Route.extend({
+          model({ menu_item_id }) {
+            return MenuItem.find(menu_item_id);
+          },
+
           setup() {
             throw new Error('Setup error');
           },

packages/ember/tests/routing/model_loading_test.js

@@ -361,6 +361,14 @@ moduleFor(
       });
       this.add('model:menu_item', MenuItem);
 
+      let SpecialRoute = class extends Route {
+        model({ menu_item_id }) {
+          return MenuItem.find(menu_item_id);
+        }
+      };
+
+      this.add('route:special', SpecialRoute);
+
       this.router.map(function () {
         this.route('home', { path: '/' });
         this.route('special', { path: '/specials/:menu_item_id' });
@@ -389,6 +397,13 @@ moduleFor(
       });
       this.add('model:menu_item', MenuItem);
 
+      let SpecialRoute = class extends Route {
+        model({ menu_item_id }) {
+          return MenuItem.find(menu_item_id);
+        }
+      };
+      this.add('route:special', SpecialRoute);
+
       this.addTemplate('home', '<h3>Home</h3>');
       this.addTemplate('special', '<p>{{@model.id}}</p>');
 
@@ -813,13 +828,20 @@ moduleFor(
     ['@test Route model hook finds the same model as a manual find'](assert) {
       let post;
       let Post = EmberObject.extend();
-      this.add('model:post', Post);
       Post.reopenClass({
         find() {
           post = this;
           return {};
         },
       });
+      this.add('model:post', Post);
+
+      let PostRoute = class extends Route {
+        model({ post_id }) {
+          return Post.find(post_id);
+        }
+      };
+      this.add('route:post', PostRoute);
 
       this.router.map(function () {
         this.route('post', { path: '/post/:post_id' });