Skip to content

Commit

Permalink
Merge pull request #17038 from emberjs/transition-hand-slapping
Browse files Browse the repository at this point in the history
[FEATURE Router Service] Deprecate touching transition state
  • Loading branch information
chadhietala authored Oct 4, 2018
2 parents ebb15e4 + 8d98f12 commit 71fec6e
Show file tree
Hide file tree
Showing 8 changed files with 126 additions and 19 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@
"puppeteer": "^1.3.0",
"qunit": "^2.5.0",
"route-recognizer": "^0.3.4",
"router_js": "^5.2.0",
"router_js": "^6.0.0",
"rsvp": "^4.8.2",
"semver": "^5.5.0",
"serve-static": "^1.12.2",
Expand Down
21 changes: 14 additions & 7 deletions packages/@ember/-internals/routing/lib/system/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,14 @@ import { once } from '@ember/runloop';
import { classify } from '@ember/string';
import { DEBUG } from '@glimmer/env';
import { TemplateFactory } from '@glimmer/opcode-compiler';
import { InternalRouteInfo, Route as IRoute, Transition, TransitionState } from 'router_js';
import {
InternalRouteInfo,
PARAMS_SYMBOL,
Route as IRoute,
STATE_SYMBOL,
Transition,
TransitionState,
} from 'router_js';
import {
calculateCacheKey,
normalizeControllerQueryParams,
Expand Down Expand Up @@ -219,7 +226,7 @@ class Route extends EmberObject implements IRoute {
}

let transition = this._router._routerMicrolib.activeTransition;
let state = transition ? transition.state : this._router._routerMicrolib.state;
let state = transition ? transition[STATE_SYMBOL] : this._router._routerMicrolib.state;

let fullName = route.fullRouteName;
let params = assign({}, state!.params[fullName]);
Expand Down Expand Up @@ -899,10 +906,10 @@ class Route extends EmberObject implements IRoute {

if (transition) {
// Update the model dep values used to calculate cache keys.
stashParamNames(this._router, transition.state!.routeInfos);
stashParamNames(this._router, transition[STATE_SYMBOL]!.routeInfos);

let cache = this._bucketCache;
let params = transition.params;
let params = transition[PARAMS_SYMBOL];
let allParams = queryParams.propertyNames;

allParams.forEach((prop: string) => {
Expand All @@ -914,7 +921,7 @@ class Route extends EmberObject implements IRoute {
set(controller, prop, value);
});

let qpValues = getQueryParamsFor(this, transition.state!);
let qpValues = getQueryParamsFor(this, transition[STATE_SYMBOL]!);
setProperties(controller, qpValues);
}

Expand Down Expand Up @@ -1152,7 +1159,7 @@ class Route extends EmberObject implements IRoute {
if (transition.resolveIndex < 1) {
return;
}
return transition.state!.routeInfos[transition.resolveIndex - 1].context;
return transition[STATE_SYMBOL]!.routeInfos[transition.resolveIndex - 1].context;
}
}

Expand Down Expand Up @@ -2432,7 +2439,7 @@ Route.reopen(ActionHandler, Evented, {
return;
}

let routeInfos = transition.state!.routeInfos;
let routeInfos = transition[STATE_SYMBOL]!.routeInfos;
let router = this._router;
let qpMeta = router._queryParamsFor(routeInfos);
let changes = router._qpUpdates;
Expand Down
61 changes: 57 additions & 4 deletions packages/@ember/-internals/routing/lib/system/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { getOwner, Owner } from '@ember/-internals/owner';
import { A as emberA, Evented, Object as EmberObject, typeOf } from '@ember/-internals/runtime';
import { EMBER_ROUTING_ROUTER_SERVICE } from '@ember/canary-features';
import { assert, deprecate, info } from '@ember/debug';
import { HANDLER_INFOS, ROUTER_EVENTS } from '@ember/deprecated-features';
import { HANDLER_INFOS, ROUTER_EVENTS, TRANSITION_STATE } from '@ember/deprecated-features';
import EmberError from '@ember/error';
import { assign } from '@ember/polyfills';
import { cancel, once, run, scheduleOnce } from '@ember/runloop';
Expand All @@ -27,6 +27,9 @@ import Router, {
InternalRouteInfo,
InternalTransition,
logAbort,
PARAMS_SYMBOL,
QUERY_PARAMS_SYMBOL,
STATE_SYMBOL,
Transition,
TransitionError,
TransitionState,
Expand Down Expand Up @@ -73,6 +76,56 @@ function defaultWillTransition(
}
}

if (TRANSITION_STATE) {
Object.defineProperty(InternalTransition.prototype, 'state', {
get() {
if (EMBER_ROUTING_ROUTER_SERVICE) {
deprecate(
'You attempted to read "transition.state" which is a private API. You should read the `RouteInfo` object on "transition.to" or "transition.from" which has the public state on it.',
false,
{
id: 'transition-state',
until: '3.9.0',
}
);
}
return this[STATE_SYMBOL];
},
});

Object.defineProperty(InternalTransition.prototype, 'queryParams', {
get() {
if (EMBER_ROUTING_ROUTER_SERVICE) {
deprecate(
'You attempted to read "transition.queryParams" which is a private API. You should read the `RouteInfo` object on "transition.to" or "transition.from" which has the queryParams on it.',
false,
{
id: 'transition-state',
until: '3.9.0',
}
);
}
return this[QUERY_PARAMS_SYMBOL];
},
});

Object.defineProperty(InternalTransition.prototype, 'params', {
get() {
if (EMBER_ROUTING_ROUTER_SERVICE) {
deprecate(
'You attempted to read "transition.params" which is a private API. You should read the `RouteInfo` object on "transition.to" or "transition.from" which has the params on it.',
false,
{
id: 'transition-state',
until: '3.9.0',
}
);
}
return this[PARAMS_SYMBOL];
},
});
}

if (HANDLER_INFOS) {
Object.defineProperty(InternalRouteInfo.prototype, 'handler', {
get() {
Expand Down Expand Up @@ -944,7 +997,7 @@ class EmberRouter extends EmberObject {

let unchangedQPs = {};
let qpUpdates = this._qpUpdates;
let params = this._routerMicrolib.activeTransition.queryParams;
let params = this._routerMicrolib.activeTransition[QUERY_PARAMS_SYMBOL];
for (let key in params) {
if (!qpUpdates.has(key)) {
unchangedQPs[key] = params[key];
Expand Down Expand Up @@ -1203,7 +1256,7 @@ class EmberRouter extends EmberObject {
let targetState = new RouterState(
this,
this._routerMicrolib,
this._routerMicrolib.activeTransition.state!
this._routerMicrolib.activeTransition[STATE_SYMBOL]!
);
this.set('targetState', targetState);

Expand Down Expand Up @@ -1668,7 +1721,7 @@ EmberRouter.reopenClass({
});

function didBeginTransition(transition: Transition, router: EmberRouter) {
let routerState = new RouterState(router, router._routerMicrolib, transition.state!);
let routerState = new RouterState(router, router._routerMicrolib, transition[STATE_SYMBOL]!);

if (!router.currentState) {
router.set('currentState', routerState);
Expand Down
4 changes: 2 additions & 2 deletions packages/@ember/-internals/routing/lib/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { get } from '@ember/-internals/metal';
import { getOwner } from '@ember/-internals/owner';
import EmberError from '@ember/error';
import { assign } from '@ember/polyfills';
import Router from 'router_js';
import Router, { STATE_SYMBOL } from 'router_js';
import Route from './system/route';
import EmberRouter, { PrivateRouteInfo, QueryParam } from './system/router';

Expand All @@ -26,7 +26,7 @@ export function extractRouteArgs(args: any[]) {

export function getActiveTargetName(router: Router<Route>) {
let routeInfos = router.activeTransition
? router.activeTransition.state!.routeInfos
? router.activeTransition[STATE_SYMBOL]!.routeInfos
: router.state!.routeInfos;
return routeInfos[routeInfos.length - 1].name;
}
Expand Down
1 change: 1 addition & 0 deletions packages/@ember/deprecated-features/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,4 @@ export const ORDERED_SET = !!'3.3.0-beta.1';
export const MERGE = !!'3.6.0-beta.1';
export const HANDLER_INFOS = !!'3.9.0';
export const ROUTER_EVENTS = !!'3.9.0';
export const TRANSITION_STATE = !!'3.9.0';
45 changes: 45 additions & 0 deletions packages/ember/tests/routing/deprecated_transition_state_test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
import { RouterTestCase, moduleFor } from 'internal-test-helpers';
import { EMBER_ROUTING_ROUTER_SERVICE } from '@ember/canary-features';

if (EMBER_ROUTING_ROUTER_SERVICE) {
moduleFor(
'Deprecated Transition State',
class extends RouterTestCase {
'@test touching transition.state is deprecated'(assert) {
assert.expect(1);
return this.visit('/').then(() => {
this.routerService.on('routeWillChange', transition => {
expectDeprecation(() => {
transition.state;
}, 'You attempted to read "transition.state" which is a private API. You should read the `RouteInfo` object on "transition.to" or "transition.from" which has the public state on it.');
});
return this.routerService.transitionTo('/child');
});
}

'@test touching transition.queryParams is deprecated'(assert) {
assert.expect(1);
return this.visit('/').then(() => {
this.routerService.on('routeWillChange', transition => {
expectDeprecation(() => {
transition.queryParams;
}, 'You attempted to read "transition.queryParams" which is a private API. You should read the `RouteInfo` object on "transition.to" or "transition.from" which has the queryParams on it.');
});
return this.routerService.transitionTo('/child');
});
}

'@test touching transition.params is deprecated'(assert) {
assert.expect(1);
return this.visit('/').then(() => {
this.routerService.on('routeWillChange', transition => {
expectDeprecation(() => {
transition.params;
}, 'You attempted to read "transition.params" which is a private API. You should read the `RouteInfo` object on "transition.to" or "transition.from" which has the params on it.');
});
return this.routerService.transitionTo('/child');
});
}
}
);
}
3 changes: 2 additions & 1 deletion packages/ember/tests/routing/query_params_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { run } from '@ember/runloop';
import { peekMeta } from '@ember/-internals/meta';
import { get, computed } from '@ember/-internals/metal';
import { Route } from '@ember/-internals/routing';
import { PARAMS_SYMBOL } from 'router_js';

import { QueryParamTestCase, moduleFor, getTextOf } from 'internal-test-helpers';

Expand Down Expand Up @@ -1332,7 +1333,7 @@ moduleFor(
'route:other',
Route.extend({
model(p, trans) {
let m = peekMeta(trans.params.application);
let m = peekMeta(trans[PARAMS_SYMBOL].application);
assert.ok(m === undefined, "A meta object isn't constructed for this params POJO");
},
})
Expand Down
8 changes: 4 additions & 4 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -7385,10 +7385,10 @@ route-recognizer@^0.3.4:
resolved "https://registry.yarnpkg.com/route-recognizer/-/route-recognizer-0.3.4.tgz#39ab1ffbce1c59e6d2bdca416f0932611e4f3ca3"
integrity sha512-2+MhsfPhvauN1O8KaXpXAOfR/fwe8dnUXVM+xw7yt40lJRfPVQxV6yryZm0cgRvAj5fMF/mdRZbL2ptwbs5i2g==

router_js@^5.2.0:
version "5.2.0"
resolved "https://registry.yarnpkg.com/router_js/-/router_js-5.2.0.tgz#8796d0ad7ab8a9d0ffbf5b02e5e00d2472a53e7d"
integrity sha512-v+gjYRwDWJpJW0jPB9tFphbcp0pD7R/ZRqu/tno9TXgQxanRArw/weyGFZnbpR95tY9B5SpFonAZk5opPNQUvQ==
router_js@^6.0.0:
version "6.0.0"
resolved "https://registry.yarnpkg.com/router_js/-/router_js-6.0.0.tgz#a9dd8918987f0464d31cfb409d8ea1496bf38ba2"
integrity sha512-2P0Be0+IMnYwLfv7SDsRs7PZCs5LWF1FTy4/m8FB5NdCCmIViCb829cfVYAdx40oeoHtRho+hiVUDWv+czqF9A==
dependencies:
"@types/node" "^10.5.5"

Expand Down

0 comments on commit 71fec6e

Please sign in to comment.