Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clean up deprecate-implicit-route-model deprecation #20817

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Clean up deprecate-implicit-route-model deprecation #20817

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 0 additions & 7 deletions packages/@ember/-internals/deprecations/index.ts
View file
Open in desktop
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
Open in desktop
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: 1 addition & 40 deletions packages/@ember/routing/route.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
} 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 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 @@ -61,16 +59,6 @@
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 @@ -1188,8 +1176,8 @@
model(
params: Record<string, unknown>,
transition: Transition
): Model | PromiseLike<Model> | undefined {

Check failure on line 1179 in packages/@ember/routing/route.ts

View workflow job for this annotation

GitHub Actions / Type Checking (current version) 

Not all code paths return a value.
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 @@ -1201,7 +1189,6 @@
let match = prop.match(/^(.*)_id$/);
if (match !== null) {
name = match[1];
value = params[prop];
}
sawParams = true;
}
Expand All @@ -1221,8 +1208,6 @@
| undefined;
}
}

return this.findModel(name, value);
}

/**
Expand All @@ -1238,30 +1223,6 @@
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
Open in desktop
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
Open in desktop
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
Loading