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

Support prefixed paths #329

Merged
merged 5 commits into from
Oct 6, 2015
Merged

Support prefixed paths #329

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6c1a25d
Support base paths
mquandalle Sep 21, 2015
bbba4e3
Set the pagejs configuration in the correct order
mquandalle Oct 3, 2015
e030416
Handle the prefixed paths in the `path` method
mquandalle Oct 3, 2015
b21b8c6
Add a test for the prefixed path generation
mquandalle Oct 3, 2015
cafa42e
Document prefixed paths
mquandalle Oct 3, 2015
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
45 changes: 28 additions & 17 deletions client/router.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ Router = function () {
// holds onRoute callbacks
this._onRouteCallbacks = [];

// if _askedToWait is true. We don't automatically start the router
// if _askedToWait is true. We don't automatically start the router
// in Meteor.startup callback. (see client/_init.js)
// Instead user need to call `.initialize()
this._askedToWait = false;
Expand All @@ -27,11 +27,16 @@ Router = function () {
this.notFound = this.notfound = null;
// indicate it's okay (or not okay) to run the tracker
// when doing subscriptions
// using a number and increment it help us to support FlowRouter.go()
// using a number and increment it help us to support FlowRouter.go()
// and legitimate reruns inside tracker on the same event loop.
// this is a solution for #145
this.safeToRun = 0;

// Meteor exposes to the client the path prefix that was defined using the
// ROOT_URL environement variable on the server using the global runtime
// configuration. See #315.
this._basePath = __meteor_runtime_config__.ROOT_URL_PATH_PREFIX || '';

// this is a chain contains a list of old routes
// most of the time, there is only one old route
// but when it's the time for a trigger redirect we've a chain
Expand Down Expand Up @@ -69,9 +74,9 @@ Router.prototype.route = function(pathDef, options, group) {
self._oldRouteChain.push(oldRoute);

var queryParams = self._qs.parse(context.querystring);
// _qs.parse() gives us a object without prototypes,
// _qs.parse() gives us a object without prototypes,
// created with Object.create(null)
// Meteor's check doesn't play nice with it.
// Meteor's check doesn't play nice with it.
// So, we need to fix it by cloning it.
// see more: https://github.com/meteorhacks/flow-router/issues/164
queryParams = JSON.parse(JSON.stringify(queryParams));
Expand All @@ -94,9 +99,9 @@ Router.prototype.route = function(pathDef, options, group) {

var triggers = self._triggersEnter.concat(route._triggersEnter);
Triggers.runTriggers(
triggers,
self._current,
self._redirectFn,
triggers,
self._current,
self._redirectFn,
afterAllTriggersRan
);
};
Expand Down Expand Up @@ -143,7 +148,7 @@ Router.prototype.path = function(pathDef, fields, queryParams) {

// this is to allow page js to keep the custom characters as it is
// we need to encode 2 times otherwise "/" char does not work properly
// So, in that case, when I includes "/" it will think it's a part of the
// So, in that case, when I includes "/" it will think it's a part of the
// route. encoding 2times fixes it
return encodeURIComponent(encodeURIComponent(fields[key] || ""));
});
Expand All @@ -169,7 +174,7 @@ Router.prototype.path = function(pathDef, fields, queryParams) {

Router.prototype.go = function(pathDef, fields, queryParams) {
var path = this.path(pathDef, fields, queryParams);

var useReplaceState = this.env.replaceState.get();
if(useReplaceState) {
this._page.replace(path);
Expand Down Expand Up @@ -234,7 +239,7 @@ Router.prototype.current = function() {

// Implementing Reactive APIs
var reactiveApis = [
'getParam', 'getQueryParam',
'getParam', 'getQueryParam',
'getRouteName', 'watchPathChange'
];
reactiveApis.forEach(function(api) {
Expand Down Expand Up @@ -343,11 +348,11 @@ Router.prototype.initialize = function(options) {
// by overriding page.js`s "show" method.
// Why?
// It is impossible to bypass exit triggers,
// becuase they execute before the handler and
// because they execute before the handler and
// can not know what the next path is, inside exit trigger.
//
// we need override both show, replace to make this work
// since we use redirect when we are talking about withReplaceState
// since we use redirect when we are talking about withReplaceState
_.each(['show', 'replace'], function(fnName) {
var original = self._page[fnName];
self._page[fnName] = function(path, state, dispatch, push) {
Expand All @@ -364,7 +369,13 @@ Router.prototype.initialize = function(options) {
// in unpredicatable manner. See #168
// this is the default behaviour and we need keep it like that
// we are doing a hack. see .path()
this._page({decodeURLComponents: true, hashbang: !!options.hashbang});
this._page({
basePath: this._basePath,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if this is useful, the below call to this._page.base seems to be enough on my browser but maybe basePath is used for compatibility with old IE?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to set this._page.base() as well? I try to run this on IE.

decodeURLComponents: true,
hashbang: !!options.hashbang
});
this._page.base(this._basePath);

this._initialized = true;
};

Expand Down Expand Up @@ -423,7 +434,7 @@ Router.prototype._buildTracker = function() {
if(isRouteChange) {
// We need to trigger that route (definition itself) has changed.
// So, we need to re-run all the register callbacks to current route
// This is pretty important, otherwise tracker
// This is pretty important, otherwise tracker
// can't identify new route's items

// We also need to afterFlush, otherwise this will re-run
Expand Down Expand Up @@ -468,9 +479,9 @@ Router.prototype._invalidateTracker = function() {
// XXX: fix this with a proper solution by removing subscription mgt.
// from the router. Then we don't need to run invalidate using a tracker

// this happens when we are trying to invoke a route change
// this happens when we are trying to invoke a route change
// with inside a route chnage. (eg:- Template.onCreated)
// Since we use page.js and tracker, we don't have much control
// Since we use page.js and tracker, we don't have much control
// over this process.
// only solution is to defer route execution.

Expand Down Expand Up @@ -542,7 +553,7 @@ Router.prototype.onRouteRegister = function(cb) {
Router.prototype._triggerRouteRegister = function(currentRoute) {
// We should only need to send a safe set of fields on the route
// object.
// This is not to hide what's inside the route object, but to show
// This is not to hide what's inside the route object, but to show
// these are the public APIs
var routePublicApi = _.pick(currentRoute, 'name', 'pathDef', 'path');
var omittingOptionFields = [
Expand Down
53 changes: 47 additions & 6 deletions test/client/router.core.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,7 @@ Tinytest.addAsync('Client - Router - notFound', function (test, done) {
}, 50);
});

Tinytest.addAsync('Client - Router - withReplaceState - enabled',
Tinytest.addAsync('Client - Router - withReplaceState - enabled',
function (test, done) {
var pathDef = "/" + Random.id() + "/:id";
var originalRedirect = FlowRouter._page.replace;
Expand All @@ -362,7 +362,7 @@ function (test, done) {
test.equal(params.id, "awesome");
test.equal(callCount, 1);
FlowRouter._page.replace = originalRedirect;
// We don't use Meteor.defer here since it carries
// We don't use Meteor.defer here since it carries
// Meteor.Environment vars too
// Which breaks our test below
setTimeout(done, 0);
Expand All @@ -374,7 +374,7 @@ function (test, done) {
});
});

Tinytest.addAsync('Client - Router - withReplaceState - disabled',
Tinytest.addAsync('Client - Router - withReplaceState - disabled',
function (test, done) {
var pathDef = "/" + Random.id() + "/:id";
var originalRedirect = FlowRouter._page.replace;
Expand Down Expand Up @@ -537,7 +537,7 @@ function (test, next) {
});

Tinytest.addAsync(
'Client - Router - wait - before initialize',
'Client - Router - wait - before initialize',
function(test, done) {
FlowRouter._initialized = false;
FlowRouter.wait();
Expand All @@ -549,7 +549,7 @@ function(test, done) {
});

Tinytest.addAsync(
'Client - Router - wait - after initialized',
'Client - Router - wait - after initialized',
function(test, done) {
try {
FlowRouter.wait();
Expand All @@ -560,7 +560,7 @@ function(test, done) {
});

Tinytest.addAsync(
'Client - Router - initialize - after initialized',
'Client - Router - initialize - after initialized',
function(test, done) {
try {
FlowRouter.initialize();
Expand All @@ -570,6 +570,47 @@ function(test, done) {
}
});

Tinytest.addAsync(
'Client - Router - base path - url updated',
function(test, done) {
var simulatedBasePath = '/flow';
var previousBasePath = FlowRouter._basePath;
var rand = Random.id();
FlowRouter.route('/' + rand, { action: function() {} });

setBasePath(simulatedBasePath);
FlowRouter.go('/' + rand);
setTimeout(function() {
test.equal(location.pathname, simulatedBasePath + '/' + rand);
setBasePath(previousBasePath);
done();
}, 100);
});

Tinytest.addAsync(
'Client - Router - base path - route action called',
function(test, done) {
var simulatedBasePath = '/flow';
var previousBasePath = FlowRouter._basePath;
var rand = Random.id();
FlowRouter.route('/' + rand, {
action: function() {
setBasePath(previousBasePath);
done();
}
});

setBasePath(simulatedBasePath);
FlowRouter.go('/' + rand);
});


function setBasePath(path) {
FlowRouter._initialized = false;
FlowRouter._basePath = path;
FlowRouter.initialize();
}

function bind(obj, method) {
return function() {
obj[method].apply(obj, arguments);
Expand Down