From 4690f6363cffbe4c2960300fbb224b457434bd56 Mon Sep 17 00:00:00 2001 From: cibernox Date: Fri, 2 Mar 2018 19:27:36 +0100 Subject: [PATCH] Rename private property Route#router to Router#_router The fact that this private property was named router was preventing user from injecting the router service into routes like this: ```js export default Route.extend({ router: service() }); ``` I consider that the above code is pretty reasonable, and it should be the private property the one that should be renamed to a clearer `_router` name that is less likely to collide with user-defined properties or methods. If you deem this an _intimate_ API we could consider adding a deprecated alias, but I don't have evidence to consider it such, as `router` is too common of a term to search for in emberobserver. --- .../ember-application/lib/system/engine.js | 2 +- .../tests/system/application_test.js | 2 +- .../tests/system/engine_test.js | 2 +- packages/ember-routing/lib/system/route.js | 59 +++++++++++-------- packages/ember-routing/lib/system/router.js | 6 +- .../ember-routing/tests/system/route_test.js | 26 +++++--- .../routing/router_service_test/basic_test.js | 2 +- 7 files changed, 58 insertions(+), 41 deletions(-) diff --git a/packages/ember-application/lib/system/engine.js b/packages/ember-application/lib/system/engine.js index d35971a4671..c72132d6233 100644 --- a/packages/ember-application/lib/system/engine.js +++ b/packages/ember-application/lib/system/engine.js @@ -502,7 +502,7 @@ function commonSetupRegistry(registry) { registry.injection('router', '_bucketCache', P`-bucket-cache:main`); registry.injection('route', '_bucketCache', P`-bucket-cache:main`); - registry.injection('route', 'router', 'router:main'); + registry.injection('route', '_router', 'router:main'); // Register the routing service... registry.register('service:-routing', RoutingService); diff --git a/packages/ember-application/tests/system/application_test.js b/packages/ember-application/tests/system/application_test.js index 9e1bc83f3a5..b4737fd7c35 100644 --- a/packages/ember-application/tests/system/application_test.js +++ b/packages/ember-application/tests/system/application_test.js @@ -135,7 +135,7 @@ moduleFor('Application', class extends ApplicationTestCase { verifyInjection(assert, application, 'router', '_bucketCache', P`-bucket-cache:main`); verifyInjection(assert, application, 'route', '_bucketCache', P`-bucket-cache:main`); - verifyInjection(assert, application, 'route', 'router', 'router:main'); + verifyInjection(assert, application, 'route', '_router', 'router:main'); verifyRegistration(assert, application, 'component:-text-field'); verifyRegistration(assert, application, 'component:-text-area'); diff --git a/packages/ember-application/tests/system/engine_test.js b/packages/ember-application/tests/system/engine_test.js index 493a405025b..302dff5411c 100644 --- a/packages/ember-application/tests/system/engine_test.js +++ b/packages/ember-application/tests/system/engine_test.js @@ -54,7 +54,7 @@ moduleFor('Engine', class extends TestCase { verifyInjection(assert, engine, 'router', '_bucketCache', P`-bucket-cache:main`); verifyInjection(assert, engine, 'route', '_bucketCache', P`-bucket-cache:main`); - verifyInjection(assert, engine, 'route', 'router', 'router:main'); + verifyInjection(assert, engine, 'route', '_router', 'router:main'); verifyRegistration(assert, engine, 'component:-text-field'); verifyRegistration(assert, engine, 'component:-text-area'); diff --git a/packages/ember-routing/lib/system/route.js b/packages/ember-routing/lib/system/route.js index cb1d7224bd1..c3a533876dc 100644 --- a/packages/ember-routing/lib/system/route.js +++ b/packages/ember-routing/lib/system/route.js @@ -8,7 +8,7 @@ import { run, isEmpty } from 'ember-metal'; -import { assert, info, isTesting } from 'ember-debug'; +import { assert, info, isTesting, deprecate } from 'ember-debug'; import { DEBUG } from 'ember-env-flags'; import { typeOf, @@ -117,6 +117,15 @@ let Route = EmberObject.extend(ActionHandler, Evented, { */ queryParams: {}, + router: computed('_router', function() { + deprecate( + 'Route#router is an intimate API that has been renamed to Route#_router. However you might want to consider using the router service', + false, + { id: 'ember-routing.route-router', until: '3.5.0', url: 'https://emberjs.com/deprecations/v3.x#toc_ember-routing-route-router' } + ); + return this._router; + }), + /** The name of the route, dot-delimited. @@ -307,7 +316,7 @@ let Route = EmberObject.extend(ActionHandler, Evented, { @property _activeQPChanged */ _activeQPChanged(qp, value) { - this.router._activeQPChanged(qp.scopedPropertyName, value); + this._router._activeQPChanged(qp.scopedPropertyName, value); }, /** @@ -315,7 +324,7 @@ let Route = EmberObject.extend(ActionHandler, Evented, { @method _updatingQPChanged */ _updatingQPChanged(qp) { - this.router._updatingQPChanged(qp.urlKey); + this._router._updatingQPChanged(qp.urlKey); }, mergedProperties: ['queryParams'], @@ -378,8 +387,8 @@ let Route = EmberObject.extend(ActionHandler, Evented, { return {}; } - let transition = this.router._routerMicrolib.activeTransition; - let state = transition ? transition.state : this.router._routerMicrolib.state; + let transition = this._router._routerMicrolib.activeTransition; + let state = transition ? transition.state : this._router._routerMicrolib.state; let fullName = route.fullRouteName; let params = assign({}, state.params[fullName]); @@ -416,7 +425,7 @@ let Route = EmberObject.extend(ActionHandler, Evented, { // urlKey isn't used here, but anyone overriding // can use it to provide serialization specific // to a certain query param. - return this.router._serializeQueryParam(value, defaultValueType); + return this._router._serializeQueryParam(value, defaultValueType); }, /** @@ -432,7 +441,7 @@ let Route = EmberObject.extend(ActionHandler, Evented, { // urlKey isn't used here, but anyone overriding // can use it to provide deserialization specific // to a certain query param. - return this.router._deserializeQueryParam(value, defaultValueType); + return this._router._deserializeQueryParam(value, defaultValueType); }, /** @@ -807,7 +816,7 @@ let Route = EmberObject.extend(ActionHandler, Evented, { let totalChanged = Object.keys(changed).concat(Object.keys(removed)); for (let i = 0; i < totalChanged.length; ++i) { let qp = qpMap[totalChanged[i]]; - if (qp && get(this._optionsForQueryParam(qp), 'refreshModel') && this.router.currentState) { + if (qp && get(this._optionsForQueryParam(qp), 'refreshModel') && this._router.currentState) { this.refresh(); break; } @@ -823,7 +832,7 @@ let Route = EmberObject.extend(ActionHandler, Evented, { if (!transition) { return; } let handlerInfos = transition.state.handlerInfos; - let router = this.router; + let router = this._router; let qpMeta = router._queryParamsFor(handlerInfos); let changes = router._qpUpdates; let replaceUrl; @@ -1118,7 +1127,7 @@ let Route = EmberObject.extend(ActionHandler, Evented, { @public */ transitionTo(name, context) { // eslint-disable-line no-unused-vars - return this.router.transitionTo(...prefixRouteNameArg(this, arguments)); + return this._router.transitionTo(...prefixRouteNameArg(this, arguments)); }, /** @@ -1139,7 +1148,7 @@ let Route = EmberObject.extend(ActionHandler, Evented, { @public */ intermediateTransitionTo() { - this.router.intermediateTransitionTo(...prefixRouteNameArg(this, arguments)); + this._router.intermediateTransitionTo(...prefixRouteNameArg(this, arguments)); }, /** @@ -1165,7 +1174,7 @@ let Route = EmberObject.extend(ActionHandler, Evented, { @public */ refresh() { - return this.router._routerMicrolib.refresh(this); + return this._router._routerMicrolib.refresh(this); }, /** @@ -1211,7 +1220,7 @@ let Route = EmberObject.extend(ActionHandler, Evented, { @public */ replaceWith() { - return this.router.replaceWith(...prefixRouteNameArg(this, arguments)); + return this._router.replaceWith(...prefixRouteNameArg(this, arguments)); }, /** @@ -1263,8 +1272,8 @@ let Route = EmberObject.extend(ActionHandler, Evented, { @public */ send(...args) { - if ((this.router && this.router._routerMicrolib) || !isTesting()) { - this.router.send(...args); + if ((this._router && this._router._routerMicrolib) || !isTesting()) { + this._router.send(...args); } else { let name = args.shift(); let action = this.actions[name]; @@ -1308,7 +1317,7 @@ let Route = EmberObject.extend(ActionHandler, Evented, { if (transition) { // Update the model dep values used to calculate cache keys. - stashParamNames(this.router, transition.state.handlerInfos); + stashParamNames(this._router, transition.state.handlerInfos); let cache = this._bucketCache; let params = transition.params; @@ -1606,7 +1615,7 @@ let Route = EmberObject.extend(ActionHandler, Evented, { store: computed(function() { let owner = getOwner(this); let routeName = this.routeName; - let namespace = get(this, 'router.namespace'); + let namespace = get(this, '_router.namespace'); return { find(name, value) { @@ -1865,7 +1874,7 @@ let Route = EmberObject.extend(ActionHandler, Evented, { modelFor(_name) { let name; let owner = getOwner(this); - let transition = this.router ? this.router._routerMicrolib.activeTransition : null; + let transition = this._router ? this._router._routerMicrolib.activeTransition : null; // Only change the route name when there is an active transition. // Otherwise, use the passed in route name. @@ -2069,7 +2078,7 @@ let Route = EmberObject.extend(ActionHandler, Evented, { let renderOptions = buildRenderOptions(this, isDefaultRender, name, options); this.connections.push(renderOptions); - run.once(this.router, '_setOutlets'); + run.once(this._router, '_setOutlets'); }, /** @@ -2148,7 +2157,7 @@ let Route = EmberObject.extend(ActionHandler, Evented, { outletName = outletName || 'main'; this._disconnectOutlet(outletName, parentView); - let handlerInfos = this.router._routerMicrolib.currentHandlerInfos; + let handlerInfos = this._router._routerMicrolib.currentHandlerInfos; for (let i = 0; i < handlerInfos.length; i++) { // This non-local state munging is sadly necessary to maintain // backward compatibility with our existing semantics, which allow @@ -2180,7 +2189,7 @@ let Route = EmberObject.extend(ActionHandler, Evented, { controller: undefined, template: undefined, }; - run.once(this.router, '_setOutlets'); + run.once(this._router, '_setOutlets'); } } }, @@ -2197,7 +2206,7 @@ let Route = EmberObject.extend(ActionHandler, Evented, { teardownViews() { if (this.connections && this.connections.length > 0) { this.connections = []; - run.once(this.router, '_setOutlets'); + run.once(this._router, '_setOutlets'); } } }); @@ -2207,7 +2216,7 @@ Route.reopenClass({ }); function parentRoute(route) { - let handlerInfo = handlerInfoFor(route, route.router._routerMicrolib.state.handlerInfos, -1); + let handlerInfo = handlerInfoFor(route, route._router._routerMicrolib.state.handlerInfos, -1); return handlerInfo && handlerInfo.handler; } @@ -2284,7 +2293,7 @@ function buildRenderOptions(route, isDefaultRender, _name, options) { }; if (DEBUG) { - let LOG_VIEW_LOOKUPS = get(route.router, 'namespace.LOG_VIEW_LOOKUPS'); + let LOG_VIEW_LOOKUPS = get(route._router, 'namespace.LOG_VIEW_LOOKUPS'); if (LOG_VIEW_LOOKUPS && !template) { info(`Could not find "${name}" template. Nothing will be rendered`, { fullName: `template:${name}` }); } @@ -2309,7 +2318,7 @@ function getQueryParamsFor(route, state) { if (state.queryParamsFor[name]) { return state.queryParamsFor[name]; } - let fullQueryParams = getFullQueryParams(route.router, state); + let fullQueryParams = getFullQueryParams(route._router, state); let params = state.queryParamsFor[name] = {}; diff --git a/packages/ember-routing/lib/system/router.js b/packages/ember-routing/lib/system/router.js index d2fd90025f0..48920f4ac0c 100644 --- a/packages/ember-routing/lib/system/router.js +++ b/packages/ember-routing/lib/system/router.js @@ -1194,7 +1194,7 @@ function logError(_error, initialMessage) { */ function findRouteSubstateName(route, state) { let owner = getOwner(route); - let { routeName, fullRouteName, router } = route; + let { routeName, fullRouteName, _router: router } = route; let substateName = `${routeName}_${state}`; let substateNameFull = `${fullRouteName}_${state}`; @@ -1216,7 +1216,7 @@ function findRouteSubstateName(route, state) { */ function findRouteStateName(route, state) { let owner = getOwner(route); - let { routeName, fullRouteName, router } = route; + let { routeName, fullRouteName, _router: router } = route; let stateName = routeName === 'application' ? state : `${routeName}.${state}`; let stateNameFull = fullRouteName === 'application' ? state : `${fullRouteName}.${state}`; @@ -1264,7 +1264,7 @@ export function triggerEvent(handlerInfos, ignoreFailure, args) { } else { // Should only hit here if a non-bubbling error action is triggered on a route. if (name === 'error') { - handler.router._markErrorAsHandled(args[0]); + handler._router._markErrorAsHandled(args[0]); } return; } diff --git a/packages/ember-routing/tests/system/route_test.js b/packages/ember-routing/tests/system/route_test.js index 6bef8e3f54e..b6a93bea79c 100644 --- a/packages/ember-routing/tests/system/route_test.js +++ b/packages/ember-routing/tests/system/route_test.js @@ -327,9 +327,9 @@ moduleFor('Route with engines', class extends AbstractTestCase { } }); - let applicationRoute = EmberRoute.create({ router, routeName: 'application', fullRouteName: 'foo.bar' }); - let postsRoute = EmberRoute.create({ router, routeName: 'posts', fullRouteName: 'foo.bar.posts' }); - let route = EmberRoute.create({ router }); + let applicationRoute = EmberRoute.create({ _router: router, routeName: 'application', fullRouteName: 'foo.bar' }); + let postsRoute = EmberRoute.create({ _router: router, routeName: 'posts', fullRouteName: 'foo.bar.posts' }); + let route = EmberRoute.create({ _router: router }); setOwner(applicationRoute, engineInstance); setOwner(postsRoute, engineInstance); @@ -370,9 +370,9 @@ moduleFor('Route with engines', class extends AbstractTestCase { } }); - let applicationRoute = EmberRoute.create({ router, routeName: 'application' }); - let postsRoute = EmberRoute.create({ router, routeName: 'posts' }); - let route = EmberRoute.create({ router }); + let applicationRoute = EmberRoute.create({ _router: router, routeName: 'application' }); + let postsRoute = EmberRoute.create({ _router: router, routeName: 'posts' }); + let route = EmberRoute.create({ _router: router }); setOwner(applicationRoute, engineInstance); setOwner(postsRoute, engineInstance); @@ -396,7 +396,7 @@ moduleFor('Route with engines', class extends AbstractTestCase { } }); - let route = EmberRoute.create({ router }); + let route = EmberRoute.create({ _router: router }); setOwner(route, engineInstance); assert.strictEqual(route.transitionTo('application'), 'foo.bar.application', 'properly prefixes application route'); @@ -422,7 +422,7 @@ moduleFor('Route with engines', class extends AbstractTestCase { } }); - let route = EmberRoute.create({ router }); + let route = EmberRoute.create({ _router: router }); setOwner(route, engineInstance); route.intermediateTransitionTo('application'); @@ -452,7 +452,7 @@ moduleFor('Route with engines', class extends AbstractTestCase { } }); - let route = EmberRoute.create({ router }); + let route = EmberRoute.create({ _router: router }); setOwner(route, engineInstance); assert.strictEqual(route.replaceWith('application'), 'foo.bar.application', 'properly prefixes application route'); @@ -462,4 +462,12 @@ moduleFor('Route with engines', class extends AbstractTestCase { let queryParams = {}; assert.strictEqual(route.replaceWith(queryParams), queryParams, 'passes query param only transitions through'); } + + ['@test `router` is a deprecated one-way alias to `_router`'](assert) { + let router = {}; + let route = EmberRoute.create({ _router: router }); + expectDeprecation(function() { + return assert.equal(route.router, router); + }, 'Route#router is an intimate API that has been renamed to Route#_router. However you might want to consider using the router service'); + } }); diff --git a/packages/ember/tests/routing/router_service_test/basic_test.js b/packages/ember/tests/routing/router_service_test/basic_test.js index e0a35d7d26d..d7cea3dd32c 100644 --- a/packages/ember/tests/routing/router_service_test/basic_test.js +++ b/packages/ember/tests/routing/router_service_test/basic_test.js @@ -72,7 +72,7 @@ if (EMBER_ROUTING_ROUTER_SERVICE) { this.add('route:parent.index', Route.extend({ init() { this._super(); - set(this.router, 'rootURL', '/homepage'); + set(this._router, 'rootURL', '/homepage'); } }));