Skip to content

Commit

Permalink
Merge pull request #14176 from trentmwillis/prefix-route-name
Browse files Browse the repository at this point in the history
Ensure Controller#transitionToRoute and Route#intermediateTransitionTo work in Engines
  • Loading branch information
rwjblue authored Sep 10, 2016
2 parents c45d219 + 63df7db commit 7ce7f4e
Show file tree
Hide file tree
Showing 5 changed files with 161 additions and 38 deletions.
9 changes: 5 additions & 4 deletions packages/ember-routing/lib/ext/controller.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { get } from 'ember-metal';
import { ControllerMixin } from 'ember-runtime';
import { prefixRouteNameArg } from '../utils';

/**
@module ember
Expand Down Expand Up @@ -110,11 +111,11 @@ ControllerMixin.reopen({
@method transitionToRoute
@public
*/
transitionToRoute() {
transitionToRoute(...args) {
// target may be either another controller or a router
let target = get(this, 'target');
let method = target.transitionToRoute || target.transitionTo;
return method.apply(target, arguments);
return method.apply(target, prefixRouteNameArg(this, args));
},

/**
Expand Down Expand Up @@ -173,11 +174,11 @@ ControllerMixin.reopen({
@method replaceRoute
@private
*/
replaceRoute() {
replaceRoute(...args) {
// target may be either another controller or a router
let target = get(this, 'target');
let method = target.replaceRoute || target.replaceWith;
return method.apply(target, arguments);
return method.apply(target, prefixRouteNameArg(target, args));
}
});

Expand Down
37 changes: 3 additions & 34 deletions packages/ember-routing/lib/system/route.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@ import {
import {
stashParamNames,
normalizeControllerQueryParams,
calculateCacheKey
calculateCacheKey,
prefixRouteNameArg
} from '../utils';
import { getOwner } from 'container';
const { slice } = Array.prototype;
Expand Down Expand Up @@ -1089,7 +1090,7 @@ let Route = EmberObject.extend(ActionHandler, Evented, {
*/
intermediateTransitionTo() {
let router = this.router;
router.intermediateTransitionTo(...arguments);
router.intermediateTransitionTo(...prefixRouteNameArg(this, arguments));
},

/**
Expand Down Expand Up @@ -2300,38 +2301,6 @@ function deprecateQueryParamDefaultValuesSetOnController(controllerName, routeNa
deprecate(`Configuring query parameter default values on controllers is deprecated. Please move the value for the property '${propName}' from the '${controllerName}' controller to the '${routeName}' route in the format: {queryParams: ${propName}: {defaultValue: <default value> }}`, false, { id: 'ember-routing.deprecate-query-param-default-values-set-on-controller', until: '3.0.0' });
}

/*
Returns an arguments array where the route name arg is prefixed based on the mount point
@private
*/
function prefixRouteNameArg(route, args) {
let routeName = args[0];
let owner = getOwner(route);
let prefix = owner.mountPoint;

// only alter the routeName if it's actually referencing a route.
if (owner.routable && typeof routeName === 'string') {
if (resemblesURL(routeName)) {
throw new EmberError('Route#transitionTo cannot be used for URLs. Please use the route name instead.');
} else {
routeName = `${prefix}.${routeName}`;
args[0] = routeName;
}
}

return args;
}

/*
Check if a routeName resembles a url instead
@private
*/
function resemblesURL(str) {
return typeof str === 'string' && ( str === '' || str.charAt(0) === '/');
}

function getEngineRouteName(engine, routeName) {
if (engine.routable) {
let prefix = engine.mountPoint;
Expand Down
34 changes: 34 additions & 0 deletions packages/ember-routing/lib/utils.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import { assign, get } from 'ember-metal';
import { getOwner } from 'container';
import { Error as EmberError } from 'ember-metal';

const ALL_PERIODS_REGEX = /\./g;

Expand Down Expand Up @@ -164,3 +166,35 @@ function accumulateQueryParamDescriptors(_desc, accum) {
accum[key] = tmp;
}
}

/*
Check if a routeName resembles a url instead
@private
*/
function resemblesURL(str) {
return typeof str === 'string' && ( str === '' || str.charAt(0) === '/');
}

/*
Returns an arguments array where the route name arg is prefixed based on the mount point
@private
*/
export function prefixRouteNameArg(route, args) {
let routeName = args[0];
let owner = getOwner(route);
let prefix = owner.mountPoint;

// only alter the routeName if it's actually referencing a route.
if (owner.routable && typeof routeName === 'string') {
if (resemblesURL(routeName)) {
throw new EmberError('Programmatic transitions by URL cannot be used within an Engine. Please use the route name instead.');
} else {
routeName = `${prefix}.${routeName}`;
args[0] = routeName;
}
}

return args;
}
32 changes: 32 additions & 0 deletions packages/ember-routing/tests/ext/controller_test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import { buildOwner } from 'internal-test-helpers';
import { Controller } from 'ember-runtime';
import { setOwner } from 'container';

QUnit.module('ember-routing/ext/controller');

QUnit.test('transitionToRoute considers an engine\'s mountPoint', function() {
expect(4);

let router = {
transitionTo(route) {
return route;
}
};

let engineInstance = buildOwner({
ownerOptions: {
routable: true,
mountPoint: 'foo.bar'
}
});

let controller = Controller.create({ target: router });
setOwner(controller, engineInstance);

strictEqual(controller.transitionToRoute('application'), 'foo.bar.application', 'properly prefixes application route');
strictEqual(controller.transitionToRoute('posts'), 'foo.bar.posts', 'properly prefixes child routes');
throws(() => controller.transitionToRoute('/posts'), 'throws when trying to use a url');

let queryParams = {};
strictEqual(controller.transitionToRoute(queryParams), queryParams, 'passes query param only transitions through');
});
87 changes: 87 additions & 0 deletions packages/ember-routing/tests/system/route_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -417,3 +417,90 @@ QUnit.test('modelFor considers an engine\'s mountPoint', function() {
strictEqual(route.modelFor('application'), applicationModel);
strictEqual(route.modelFor('posts'), postsModel);
});

QUnit.test('transitionTo considers an engine\'s mountPoint', function() {
expect(4);

let router = {
transitionTo(route) {
return route;
}
};

let engineInstance = buildOwner({
ownerOptions: {
routable: true,
mountPoint: 'foo.bar'
}
});

let route = EmberRoute.create({ router });
setOwner(route, engineInstance);

strictEqual(route.transitionTo('application'), 'foo.bar.application', 'properly prefixes application route');
strictEqual(route.transitionTo('posts'), 'foo.bar.posts', 'properly prefixes child routes');
throws(() => route.transitionTo('/posts'), 'throws when trying to use a url');

let queryParams = {};
strictEqual(route.transitionTo(queryParams), queryParams, 'passes query param only transitions through');
});

QUnit.test('intermediateTransitionTo considers an engine\'s mountPoint', function() {
expect(4);

let lastRoute;
let router = {
intermediateTransitionTo(route) {
lastRoute = route;
}
};

let engineInstance = buildOwner({
ownerOptions: {
routable: true,
mountPoint: 'foo.bar'
}
});

let route = EmberRoute.create({ router });
setOwner(route, engineInstance);

route.intermediateTransitionTo('application');
strictEqual(lastRoute, 'foo.bar.application', 'properly prefixes application route');

route.intermediateTransitionTo('posts');
strictEqual(lastRoute, 'foo.bar.posts', 'properly prefixes child routes');

throws(() => route.intermediateTransitionTo('/posts'), 'throws when trying to use a url');

let queryParams = {};
route.intermediateTransitionTo(queryParams);
strictEqual(lastRoute, queryParams, 'passes query param only transitions through');
});

QUnit.test('replaceWith considers an engine\'s mountPoint', function() {
expect(4);

let router = {
replaceWith(route) {
return route;
}
};

let engineInstance = buildOwner({
ownerOptions: {
routable: true,
mountPoint: 'foo.bar'
}
});

let route = EmberRoute.create({ router });
setOwner(route, engineInstance);

strictEqual(route.replaceWith('application'), 'foo.bar.application', 'properly prefixes application route');
strictEqual(route.replaceWith('posts'), 'foo.bar.posts', 'properly prefixes child routes');
throws(() => route.replaceWith('/posts'), 'throws when trying to use a url');

let queryParams = {};
strictEqual(route.replaceWith(queryParams), queryParams, 'passes query param only transitions through');
});

0 comments on commit 7ce7f4e

Please sign in to comment.