Skip to content

Support for ? in paths #214

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.

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

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
127 changes: 66 additions & 61 deletions modules/helpers/Path.js
Original file line number Diff line number Diff line change
@@ -1,112 +1,117 @@
var invariant = require('react/lib/invariant');
var copyProperties = require('react/lib/copyProperties');
var qs = require('querystring');
var URL = require('./URL');
var merge = require('qs/lib/utils').merge;
var qs = require('qs');

var paramMatcher = /((?::[a-z_$][a-z0-9_$]*)|\*)/ig;
var queryMatcher = /\?(.+)/;

function getParamName(pathSegment) {
return pathSegment === '*' ? 'splat' : pathSegment.substr(1);
function encodeURL(url) {
return encodeURIComponent(url).replace(/%20/g, '+');
}

var _compiledPatterns = {};
function decodeURL(url) {
return decodeURIComponent(url.replace(/\+/g, ' '));
}

function compilePattern(pattern) {
if (_compiledPatterns[pattern])
return _compiledPatterns[pattern];
function encodeURLPath(path) {
return String(path).split('/').map(encodeURL).join('/');
}

var compiled = _compiledPatterns[pattern] = {};
var paramNames = compiled.paramNames = [];
var paramMatcher = /:([a-zA-Z_$][a-zA-Z0-9_$]*)|[*.()\[\]\\+|{}^$]/g;
var queryMatcher = /\?(.+)/;

var source = pattern.replace(paramMatcher, function (match, pathSegment) {
paramNames.push(getParamName(pathSegment));
return pathSegment === '*' ? '(.*?)' : '([^/?#]+)';
});
var _compiledPatterns = {};

compiled.matcher = new RegExp('^' + source + '$', 'i');
function compilePattern(pattern) {
if (!(pattern in _compiledPatterns)) {
var paramNames = [];
var source = pattern.replace(paramMatcher, function (match, paramName) {
if (paramName) {
paramNames.push(paramName);
return '([^./?#]+)';
} else if (match === '*') {
paramNames.push('splat');
return '(.*?)';
} else {
return '\\' + match;
}
});

return compiled;
}
_compiledPatterns[pattern] = {
matcher: new RegExp('^' + source + '$', 'i'),
paramNames: paramNames
};
}

function isDynamicPattern(pattern) {
return pattern.indexOf(':') !== -1 || pattern.indexOf('*') !== -1;
return _compiledPatterns[pattern];
}

var Path = {

/**
* Returns an array of the names of all parameters in the given pattern.
*/
extractParamNames: function (pattern) {
return compilePattern(pattern).paramNames;
},

/**
* Extracts the portions of the given URL path that match the given pattern
* and returns an object of param name => value pairs. Returns null if the
* pattern does not match the given path.
*/
extractParams: function (pattern, path) {
if (!pattern)
return null;

if (!isDynamicPattern(pattern)) {
if (pattern === URL.decode(path))
return {}; // No dynamic segments, but the paths match.

return null;
}

var compiled = compilePattern(pattern);
var match = URL.decode(path).match(compiled.matcher);
var object = compilePattern(pattern);
var match = decodeURL(path).match(object.matcher);

if (!match)
return null;

var params = {};

compiled.paramNames.forEach(function (paramName, index) {
object.paramNames.forEach(function (paramName, index) {
params[paramName] = match[index + 1];
});

return params;
},

/**
* Returns an array of the names of all parameters in the given pattern.
*/
extractParamNames: function (pattern) {
if (!pattern)
return [];
return compilePattern(pattern).paramNames;
},

/**
* Returns a version of the given route path with params interpolated. Throws
* if there is a dynamic segment of the route path for which there is no param.
*/
injectParams: function (pattern, params) {
if (!pattern)
return null;

if (!isDynamicPattern(pattern))
return pattern;

params = params || {};

return pattern.replace(paramMatcher, function (match, pathSegment) {
var paramName = getParamName(pathSegment);
var splatIndex = 0;

return pattern.replace(paramMatcher, function (match, paramName) {
paramName = paramName || 'splat';

invariant(
params[paramName] != null,
'Missing "' + paramName + '" parameter for path "' + pattern + '"'
);

// Preserve forward slashes.
return String(params[paramName]).split('/').map(URL.encode).join('/');
var segment;
if (paramName === 'splat' && Array.isArray(params[paramName])) {
segment = params[paramName][splatIndex++];

invariant(
segment != null,
'Missing splat # ' + splatIndex + ' for path "' + pattern + '"'
);
} else {
segment = params[paramName];
}

return encodeURLPath(segment);
});
},

/**
* Returns an object that is the result of parsing any query string contained in
* the given path, null if the path contains no query string.
* Returns an object that is the result of parsing any query string contained
* in the given path, null if the path contains no query string.
*/
extractQuery: function (path) {
var match = path.match(queryMatcher);
var match = decodeURL(path).match(queryMatcher);
return match && qs.parse(match[1]);
},

Expand All @@ -118,14 +123,14 @@ var Path = {
},

/**
* Returns a version of the given path with the parameters in the given query
* added to the query string.
* Returns a version of the given path with the parameters in the given
* query merged into the query string.
*/
withQuery: function (path, query) {
var existingQuery = Path.extractQuery(path);

if (existingQuery)
query = query ? copyProperties(existingQuery, query) : existingQuery;
query = query ? merge(existingQuery, query) : existingQuery;

var queryString = query && qs.stringify(query);

Expand Down
22 changes: 0 additions & 22 deletions modules/helpers/URL.js

This file was deleted.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
"dependencies": {
"es6-promise": "^1.0.0",
"events": "^1.0.1",
"querystring": "^0.2.0"
"qs": "^1.2.2"
},
"keywords": [
"react",
Expand Down
77 changes: 55 additions & 22 deletions specs/Path.spec.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,26 @@
require('./helper');
var Path = require('../modules/helpers/Path');

describe('Path.extractParamNames', function () {
describe('when a pattern contains no dynamic segments', function () {
it('returns an empty array', function () {
expect(Path.extractParamNames('a/b/c')).toEqual([]);
});
});

describe('when a pattern contains :a and :b dynamic segments', function () {
it('returns the correct names', function () {
expect(Path.extractParamNames('/comments/:a/:b/edit')).toEqual([ 'a', 'b' ]);
});
});

describe('when a pattern has a *', function () {
it('uses the name "splat"', function () {
expect(Path.extractParamNames('/files/*.jpg')).toEqual([ 'splat' ]);
});
});
});

describe('Path.extractParams', function () {
describe('when a pattern does not have dynamic segments', function () {
var pattern = 'a/b/c';
Expand All @@ -19,11 +39,11 @@ describe('Path.extractParams', function () {
});

describe('when a pattern has dynamic segments', function () {
var pattern = 'comments/:id/edit';
var pattern = 'comments/:id.:ext/edit';

describe('and the path matches', function () {
it('returns an object with the params', function () {
expect(Path.extractParams(pattern, 'comments/abc/edit')).toEqual({ id: 'abc' });
expect(Path.extractParams(pattern, 'comments/abc.js/edit')).toEqual({ id: 'abc', ext: 'js' });
Copy link
Member

Choose a reason for hiding this comment

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

what about /comments/:id/edit, can we still have comments/foo.bar/edit or does the . mess it up now?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, :id does not match foo.bar.

});
});

Expand All @@ -35,7 +55,7 @@ describe('Path.extractParams', function () {

describe('and the path matches with a segment containing a .', function () {
it('returns an object with the params', function () {
expect(Path.extractParams(pattern, 'comments/foo.bar/edit')).toEqual({ id: 'foo.bar' });
expect(Path.extractParams(pattern, 'comments/foo.bar/edit')).toEqual({ id: 'foo', ext: 'bar' });
});
});
});
Expand Down Expand Up @@ -73,38 +93,37 @@ describe('Path.extractParams', function () {
});

describe('when a pattern has a *', function () {
var pattern = '/files/*.jpg';

describe('and the path matches', function () {
it('returns an object with the params', function () {
expect(Path.extractParams(pattern, '/files/my/photo.jpg')).toEqual({ splat: 'my/photo' });
expect(Path.extractParams('/files/*', '/files/my/photo.jpg')).toEqual({ splat: 'my/photo.jpg' });
expect(Path.extractParams('/files/*', '/files/my/photo.jpg.zip')).toEqual({ splat: 'my/photo.jpg.zip' });
expect(Path.extractParams('/files/*.jpg', '/files/my/photo.jpg')).toEqual({ splat: 'my/photo' });
});
});

describe('and the path does not match', function () {
it('returns null', function () {
expect(Path.extractParams(pattern, '/files/my/photo.png')).toBe(null);
expect(Path.extractParams('/files/*.jpg', '/files/my/photo.png')).toBe(null);
});
});
});
});

describe('Path.extractParamNames', function () {
describe('when a pattern contains no dynamic segments', function () {
it('returns an empty array', function () {
expect(Path.extractParamNames('a/b/c')).toEqual([]);
});
});
describe('when a pattern has a ?', function () {
var pattern = '/archive/?:name?';

describe('when a pattern contains :a and :b dynamic segments', function () {
it('returns the correct names', function () {
expect(Path.extractParamNames('/comments/:a/:b/edit')).toEqual([ 'a', 'b' ]);
describe('and the path matches', function () {
it('returns an object with the params', function () {
expect(Path.extractParams(pattern, '/archive')).toEqual({ name: undefined });
expect(Path.extractParams(pattern, '/archive/')).toEqual({ name: undefined });
expect(Path.extractParams(pattern, '/archive/foo')).toEqual({ name: 'foo' });
Copy link
Contributor

Choose a reason for hiding this comment

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

what about /archivefoo?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, good catch. Added.

expect(Path.extractParams(pattern, '/archivefoo')).toEqual({ name: 'foo' });
});
});
});

describe('when a pattern has a *', function () {
it('uses the name "splat"', function () {
expect(Path.extractParamNames('/files/*.jpg')).toEqual([ 'splat' ]);
describe('and the path does not match', function () {
it('returns null', function () {
expect(Path.extractParams(pattern, '/archiv')).toBe(null);
});
});
});
});
Expand Down Expand Up @@ -151,12 +170,22 @@ describe('Path.injectParams', function () {
});
});
});

describe('when a pattern has multiple splats', function () {
it('returns the correct path', function () {
expect(Path.injectParams('/a/*/c/*', { splat: [ 'b', 'd' ] })).toEqual('/a/b/c/d');
Copy link
Member

Choose a reason for hiding this comment

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

When would I use splats instead of params?

});
});
});

describe('Path.extractQuery', function () {
describe('when the path contains a query string', function () {
it('returns the parsed query object', function () {
expect(Path.extractQuery('/a/b/c?id=def&show=true')).toEqual({ id: 'def', show: 'true' });
expect(Path.extractQuery('/?id=def&show=true')).toEqual({ id: 'def', show: 'true' });
});

it('properly handles arrays', function () {
expect(Path.extractQuery('/?id%5B%5D=a&id%5B%5D=b')).toEqual({ id: [ 'a', 'b' ] });
Copy link
Member

Choose a reason for hiding this comment

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

I used querystring originally so that we had the same semantics as server side node's query strings.

Namely foo=bar&foo=baz, rather than the rails style foo[]=bar&foo[]=baz. You sure we want to be different than node here when we're after isomorphic apps and node is the obvious majority use-case?

http://nodejs.org/api/querystring.html

Copy link
Member Author

Choose a reason for hiding this comment

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

express, hapi, mach all use qs instead of querystring. Also, PHP, Rails, etc. Core node is the outlier here.

Copy link
Member

Choose a reason for hiding this comment

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

👍

});
});

Expand All @@ -177,6 +206,10 @@ describe('Path.withQuery', function () {
it('appends the query string', function () {
expect(Path.withQuery('/a/b/c', { id: 'def' })).toEqual('/a/b/c?id=def');
});

it('merges two query strings', function () {
expect(Path.withQuery('/path?a=b', { c: [ 'd', 'e' ]})).toEqual('/path?a=b&c%5B0%5D=d&c%5B1%5D=e');
});
});

describe('Path.normalize', function () {
Expand Down