Skip to content

Commit

Permalink
Merge pull request #20817 from bertdeblock/clean-up-implicit-route-mo…
Browse files Browse the repository at this point in the history 
…del-deprecation
  • Loading branch information
@kategengler
kategengler authored Jan 15, 2025
2 parents d5c9e05 + 515b222 commit 7924032
Show file tree
Hide file tree
Showing 5 changed files with 3 additions and 218 deletions.
7 changes: 0 additions & 7 deletions packages/@ember/-internals/deprecations/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
16 changes: 0 additions & 16 deletions packages/@ember/-internals/environment/lib/env.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
41 changes: 2 additions & 39 deletions packages/@ember/routing/route.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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';
Expand Down Expand Up @@ -62,16 +60,6 @@ type RouteTransitionState = TransitionState<Route> & {
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'
);
}

const RENDER = Symbol('render');
const RENDER_STATE = Symbol('render-state');

Expand Down Expand Up @@ -1190,7 +1178,7 @@ class Route<Model = unknown> extends EmberObject.extend(ActionHandler, Evented)
params: Record<string, unknown>,
transition: Transition
): Model | PromiseLike<Model> | 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<Model>['_qp']).map;

Expand All @@ -1202,7 +1190,6 @@ class Route<Model = unknown> extends EmberObject.extend(ActionHandler, Evented)
let match = prop.match(/^(.*)_id$/);
if (match !== null) {
name = match[1];
value = params[prop];
}
sawParams = true;
}
Expand All @@ -1223,7 +1210,7 @@ class Route<Model = unknown> extends EmberObject.extend(ActionHandler, Evented)
}
}

return this.findModel(name, value);
return undefined;
}

/**
Expand All @@ -1239,30 +1226,6 @@ class Route<Model = unknown> 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<Model> | undefined;
}

/**
A hook you can use to setup the controller for the current route.
Expand Down
155 changes: 1 addition & 154 deletions packages/@ember/routing/tests/system/route_test.js
View file
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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);

Expand All @@ -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);
Expand Down
2 changes: 0 additions & 2 deletions tests/docs/expected.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -201,7 +200,6 @@ module.exports = {
'finally',
'find',
'findBy',
'findModel',
'firstObject',
'flushWatchers',
'fn',
Expand Down

0 comments on commit 7924032

Please sign in to comment.