From 515b222edc1757164b9990e08bea090239e233ed Mon Sep 17 00:00:00 2001 From: Bert De Block Date: Fri, 27 Dec 2024 14:57:39 +0100 Subject: [PATCH] Clean up `deprecate-implicit-route-model` deprecation --- .../@ember/-internals/deprecations/index.ts | 7 - .../@ember/-internals/environment/lib/env.ts | 16 -- packages/@ember/routing/route.ts | 41 +---- .../@ember/routing/tests/system/route_test.js | 155 +----------------- tests/docs/expected.js | 2 - 5 files changed, 3 insertions(+), 218 deletions(-) diff --git a/packages/@ember/-internals/deprecations/index.ts b/packages/@ember/-internals/deprecations/index.ts index 9d31040166b..a1c4efcc4e3 100644 --- a/packages/@ember/-internals/deprecations/index.ts +++ b/packages/@ember/-internals/deprecations/index.ts @@ -102,13 +102,6 @@ export const DEPRECATIONS = { ).toLowerCase()}-from-ember`, }); }, - DEPRECATE_IMPLICIT_ROUTE_MODEL: deprecation({ - id: 'deprecate-implicit-route-model', - for: 'ember-source', - since: { available: '5.3.0', enabled: '5.3.0' }, - until: '6.0.0', - url: 'https://deprecations.emberjs.com/v5.x/#toc_deprecate-implicit-route-model', - }), DEPRECATE_TEMPLATE_ACTION: deprecation({ id: 'template-action', url: 'https://deprecations.emberjs.com/id/template-action', diff --git a/packages/@ember/-internals/environment/lib/env.ts b/packages/@ember/-internals/environment/lib/env.ts index d64eac6c7d8..b29975782dc 100644 --- a/packages/@ember/-internals/environment/lib/env.ts +++ b/packages/@ember/-internals/environment/lib/env.ts @@ -132,22 +132,6 @@ export const ENV = { */ _DEFAULT_ASYNC_OBSERVERS: false, - /** - Whether the app still has default record-loading behavior in the model - hook from RFC https://rfcs.emberjs.com/id/0774-implicit-record-route-loading - This will also remove the default store property from the route. - - This is not intended to be set directly, as the implementation may change in - the future. Use `@ember/optional-features` instead. - - @property _NO_IMPLICIT_ROUTE_MODEL - @for EmberENV - @type Boolean - @default false - @private - */ - _NO_IMPLICIT_ROUTE_MODEL: false, - /** Controls the maximum number of scheduled rerenders without "settling". In general, applications should not need to modify this environment variable, but please diff --git a/packages/@ember/routing/route.ts b/packages/@ember/routing/route.ts index 618b7cea1bb..5f5ea0d9676 100644 --- a/packages/@ember/routing/route.ts +++ b/packages/@ember/routing/route.ts @@ -7,7 +7,6 @@ import { } from '@ember/-internals/metal'; import type Owner from '@ember/owner'; import { getOwner } from '@ember/-internals/owner'; -import { ENV } from '@ember/-internals/environment'; import type { default as BucketCache } from './lib/cache'; import EmberObject, { computed, get, set, getProperties, setProperties } from '@ember/object'; import Evented from '@ember/object/evented'; @@ -19,7 +18,6 @@ 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 { DEPRECATIONS, deprecateUntil } from '@ember/-internals/deprecations'; import EngineInstance from '@ember/engine/instance'; import { dependentKeyCompat } from '@ember/object/compat'; import { once } from '@ember/runloop'; @@ -62,16 +60,6 @@ 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' - ); -} - const RENDER = Symbol('render'); const RENDER_STATE = Symbol('render-state'); @@ -1190,7 +1178,7 @@ class Route extends EmberObject.extend(ActionHandler, Evented) params: Record, transition: Transition ): Model | PromiseLike | undefined { - let name, sawParams, value; + let name, sawParams; // SAFETY: Since `_qp` is protected we can't infer the type let queryParams = (get(this, '_qp') as Route['_qp']).map; @@ -1202,7 +1190,6 @@ class Route extends EmberObject.extend(ActionHandler, Evented) let match = prop.match(/^(.*)_id$/); if (match !== null) { name = match[1]; - value = params[prop]; } sawParams = true; } @@ -1223,7 +1210,7 @@ class Route extends EmberObject.extend(ActionHandler, Evented) } } - return this.findModel(name, value); + return undefined; } /** @@ -1239,30 +1226,6 @@ class Route extends EmberObject.extend(ActionHandler, Evented) return this.model(this._paramsFor(this.routeName, _params), transition); } - /** - - @method findModel - @param {String} type the model type - @param {Object} value the value passed to find - @private - */ - findModel(type: string, value: unknown) { - if (ENV._NO_IMPLICIT_ROUTE_MODEL) { - return; - } - deprecateUntil( - `The implicit model loading behavior for routes is deprecated. ` + - `Please define an explicit model hook for ${this.fullRouteName}.`, - DEPRECATIONS.DEPRECATE_IMPLICIT_ROUTE_MODEL - ); - - 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; - } - /** A hook you can use to setup the controller for the current route. diff --git a/packages/@ember/routing/tests/system/route_test.js b/packages/@ember/routing/tests/system/route_test.js index 38b1bb14544..10623c73057 100644 --- a/packages/@ember/routing/tests/system/route_test.js +++ b/packages/@ember/routing/tests/system/route_test.js @@ -1,15 +1,6 @@ import { setOwner } from '@ember/-internals/owner'; -import { ENV } from '@ember/-internals/environment'; -import { DEPRECATIONS } from '@ember/-internals/deprecations'; -import { - runDestroy, - buildOwner, - moduleFor, - testUnless, - AbstractTestCase, -} from 'internal-test-helpers'; +import { runDestroy, buildOwner, moduleFor, AbstractTestCase } from 'internal-test-helpers'; import Service, { service } from '@ember/service'; -import EmberObject from '@ember/object'; import EmberRoute from '@ember/routing/route'; import ObjectProxy from '@ember/object/proxy'; import { getDebugFunction, setDebugFunction } from '@ember/debug'; @@ -30,64 +21,6 @@ moduleFor( route = routeOne = routeTwo = lookupHash = undefined; } - ['@test noops if _NO_IMPLICIT_ROUTE_MODEL is true'](assert) { - this._NO_IMPLICIT_ROUTE_MODEL = ENV._NO_IMPLICIT_ROUTE_MODEL; - ENV._NO_IMPLICIT_ROUTE_MODEL = true; - assert.equal( - route.findModel('post', 1), - undefined, - 'When _NO_IMPLICIT_ROUTE_MODEL is true, findModel does nothing' - ); - ENV._NO_IMPLICIT_ROUTE_MODEL = this._NO_IMPLICIT_ROUTE_MODEL; - } - - [`${testUnless( - DEPRECATIONS.DEPRECATE_IMPLICIT_ROUTE_MODEL.isRemoved - )} default store utilizes the container to acquire the model factory`](assert) { - assert.expect(5); - - let Post = EmberObject.extend(); - let post = {}; - - Post.reopenClass({ - find() { - return post; - }, - }); - - let ownerOptions = { - ownerOptions: { - hasRegistration() { - return true; - }, - factoryFor(fullName) { - assert.equal(fullName, 'model:post', 'correct factory was looked up'); - - return { - class: Post, - create() { - return Post.create(); - }, - }; - }, - }, - }; - - let owner = buildOwner(ownerOptions); - setOwner(route, owner); - - expectDeprecation( - () => { - 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./, - DEPRECATIONS.DEPRECATE_IMPLICIT_ROUTE_MODEL.isEnabled - ); - - runDestroy(owner); - } - ['@test default store can be overridden'](assert) { runDestroy(route); @@ -104,92 +37,6 @@ moduleFor( assert.true(calledFind, 'store.find was called'); } - [`${testUnless( - DEPRECATIONS.DEPRECATE_IMPLICIT_ROUTE_MODEL.isRemoved - )} assert if 'store.find' method is not found`]() { - runDestroy(route); - - let owner = buildOwner(); - let Post = EmberObject.extend(); - - owner.register( - 'route:index', - EmberRoute.extend({ - routeName: 'index', - }) - ); - owner.register('model:post', Post); - - route = owner.lookup('route:index'); - - 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); - } - - [`${testUnless( - DEPRECATIONS.DEPRECATE_IMPLICIT_ROUTE_MODEL.isRemoved - )} asserts if model class is not found`]() { - runDestroy(route); - - let owner = buildOwner(); - owner.register( - 'route:index', - EmberRoute.extend({ - routeName: 'index', - }) - ); - - route = owner.lookup('route:index'); - - 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); - } - - [`${testUnless( - DEPRECATIONS.DEPRECATE_IMPLICIT_ROUTE_MODEL.isRemoved - )} 'store' does not need to be injected`](assert) { - runDestroy(route); - - let owner = buildOwner(); - - owner.register('route:index', EmberRoute); - - route = owner.lookup('route:index'); - - ignoreDeprecation(() => ignoreAssertion(() => route.model({ post_id: 1 }))); - - assert.ok(true, 'no error was raised'); - - runDestroy(owner); - } - ["@test modelFor doesn't require the router"](assert) { let owner = buildOwner(); setOwner(route, owner); diff --git a/tests/docs/expected.js b/tests/docs/expected.js index 550fc9d4772..84a18c635c0 100644 --- a/tests/docs/expected.js +++ b/tests/docs/expected.js @@ -9,7 +9,6 @@ module.exports = { '[]', '_DEBUG_RENDER_TREE', '_DEFAULT_ASYNC_OBSERVERS', - '_NO_IMPLICIT_ROUTE_MODEL', '_RERENDER_LOOP_LIMIT', '_ALL_DEPRECATIONS_ENABLED', '_OVERRIDE_DEPRECATION_VERSION', @@ -201,7 +200,6 @@ module.exports = { 'finally', 'find', 'findBy', - 'findModel', 'firstObject', 'flushWatchers', 'fn',