diff --git a/packages/@ember/routing/route.ts b/packages/@ember/routing/route.ts index 94e3d15ea58..ab86db15a3c 100644 --- a/packages/@ember/routing/route.ts +++ b/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'; @@ -59,6 +59,16 @@ type RouteTransitionState = TransitionState & { type MaybeParameters = T extends AnyFn ? Parameters : unknown[]; type MaybeReturnType = T extends AnyFn ? ReturnType : 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'); @@ -1109,15 +1119,6 @@ class Route extends EmberObject.extend(ActionHandler, Evented) export default Router; ``` - The model for the `post` route is `store.findRecord('post', params.post_id)`. - - By default, if your route has a dynamic segment ending in `_id`: - - * The model class is determined from the segment (`post_id`'s - class is `App.Post`) - * The find method is called on the model class with the value of - the dynamic segment. - Note that for routes with dynamic segments, this hook is not always executed. If the route is entered through a transition (e.g. when using the `link-to` Handlebars helper or the `transitionTo` method @@ -1151,12 +1152,21 @@ class Route extends EmberObject.extend(ActionHandler, Evented) if a promise returned from `model` fails, the error will be handled by the `error` hook on `Route`. + Note that the legacy behavior of automatically defining a model + hook when a dynamic segment ending in `_id` is present is + [deprecated](https://deprecations.emberjs.com/v5.x#toc_deprecate-implicit-route-model). + You should explicitly define a model hook whenever any segments are + present. + Example ```app/routes/post.js import Route from '@ember/routing/route'; + import { service } from '@ember/service'; export default class PostRoute extends Route { + @service store; + model(params) { return this.store.findRecord('post', params.post_id); } @@ -1233,14 +1243,24 @@ class Route extends EmberObject.extend(ActionHandler, Evented) @param {Object} value the value passed to find @private */ - findModel(...args: any[]) { - // SAFETY: it's all absurd lies; there is no explicit contract for `store` - // and we allow people to register *anything* here and we call it: GLHF! The - // fallback path here means we correctly handle the case where there is no - // explicit store injection on the route subclass. - // eslint-disable-next-line @typescript-eslint/no-explicit-any - let store = ('store' in this ? this.store : get(this, '_store')) as any; - return store.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: '5.3.0' }, + until: '6.0.0', + } + ); + + const store = 'store' in this ? this.store : get(this, '_store'); + assert('Expected route to have a store with a find method', isStoreLike(store)); + + // SAFETY: We don't actually know it will return this, but this code path is also deprecated. + return store.find(type, value) as Model | PromiseLike | undefined; } /** @@ -1259,8 +1279,11 @@ class Route extends EmberObject.extend(ActionHandler, Evented) ```app/routes/photos.js import Route from '@ember/routing/route'; + import { service } from '@ember/service'; export default class PhotosRoute extends Route { + @service store; + model() { return this.store.findAll('photo'); } @@ -1564,20 +1587,7 @@ class Route extends EmberObject.extend(ActionHandler, Evented) return params; } - /** - Store property provides a hook for data persistence libraries to inject themselves. - - By default, this store property provides the exact same functionality previously - in the model hook. - - Currently, the required interface is: - - `store.find(modelName, findArguments)` - - @property store - @type {Object} - @private - */ + /** @deprecated Manually define your own store, such as with `@service store` */ @computed protected get _store() { const owner = getOwner(this); diff --git a/packages/@ember/routing/tests/system/route_test.js b/packages/@ember/routing/tests/system/route_test.js index 5777aa1270b..9f9ab0cfb4d 100644 --- a/packages/@ember/routing/tests/system/route_test.js +++ b/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,12 +55,34 @@ 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); } + ['@test default store can be overridden'](assert) { + runDestroy(route); + + let calledFind = false; + route = EmberRoute.extend({ + store: { + find() { + calledFind = true; + }, + }, + }).create(); + + route.store.find(); + assert.true(calledFind, 'store.find was called'); + } + ["@test assert if 'store.find' method is not found"]() { runDestroy(route); @@ -77,21 +99,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 +133,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 +156,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'); diff --git a/packages/ember/tests/routing/decoupled_basic_test.js b/packages/ember/tests/routing/decoupled_basic_test.js index 1a8c744a693..69b07d43d27 100644 --- a/packages/ember/tests/routing/decoupled_basic_test.js +++ b/packages/ember/tests/routing/decoupled_basic_test.js @@ -131,17 +131,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', '

{{@model.id}}

'); this.addTemplate('loading', '

LOADING!

'); - 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`]( @@ -161,6 +173,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({ @@ -219,7 +239,9 @@ moduleFor( }) ); - runTask(() => handleURLRejectsWith(this, assert, 'specials/1', 'Setup error')); + ignoreDeprecation(() => { + runTask(() => handleURLRejectsWith(this, assert, 'specials/1', 'Setup error')); + }); resolve(menuItem); } @@ -263,6 +285,10 @@ moduleFor( this.add( 'route:special', Route.extend({ + model({ menu_item_id }) { + return MenuItem.find(menu_item_id); + }, + setup() { throw new Error('Setup error'); }, diff --git a/packages/ember/tests/routing/model_loading_test.js b/packages/ember/tests/routing/model_loading_test.js index ce047894d1c..a952ef12c35 100644 --- a/packages/ember/tests/routing/model_loading_test.js +++ b/packages/ember/tests/routing/model_loading_test.js @@ -362,6 +362,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' }); @@ -390,6 +398,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', '

Home

'); this.addTemplate('special', '

{{@model.id}}

'); @@ -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' }); diff --git a/tests/docs/expected.js b/tests/docs/expected.js index d2a29723fda..16d1939c344 100644 --- a/tests/docs/expected.js +++ b/tests/docs/expected.js @@ -484,7 +484,6 @@ module.exports = { 'sort', 'sortBy', 'startRouting', - 'store', 'subscribe', 'sum', 'tagName',