Skip to content

Commit 5ea6238

Browse files
committed
urlPatternOptions bugfixes; merge from parent contextual routers
1 parent a95f64d commit 5ea6238

File tree

5 files changed

+132
-29
lines changed

5 files changed

+132
-29
lines changed

docs/override-url-pattern.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ Compile an object matching these fields and pass it on your `<Location>` or `<Lo
99
defaults.
1010

1111
It's possible to set more specific rules on individual `<Location>` components. These rules will be merged
12-
with parent rules.
12+
with parent rules. Rules are even merged from parent contextual routers!
1313

1414
For example:
1515

lib/RouterMixin.js

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ var RouterMixin = {
7979
path = '/' + path;
8080
}
8181

82-
var match = matchRoutes(this.getRoutes(props), path);
82+
var match = matchRoutes(this.getRoutes(props), path, this.getURLPatternOptions());
8383

8484
return {
8585
match: match,
@@ -124,6 +124,16 @@ var RouterMixin = {
124124
return this.state.match;
125125
},
126126

127+
getURLPatternOptions: function() {
128+
var parent = this.getParentRouter();
129+
var parentOptions = parent && parent.getURLPatternOptions();
130+
// Check existence so we don't return an empty object if there are no options.
131+
if (parentOptions) {
132+
return assign({}, this.props.urlPatternOptions, parentOptions);
133+
}
134+
return this.props.urlPatternOptions;
135+
},
136+
127137
/**
128138
* Make href scoped for the current router.
129139
*/
@@ -155,7 +165,7 @@ var RouterMixin = {
155165
* @param {Callback} cb
156166
*/
157167
setPath: function(path, navigation, cb) {
158-
var match = matchRoutes(this.getRoutes(this.props), path, this.props);
168+
var match = matchRoutes(this.getRoutes(this.props), path, this.getURLPatternOptions());
159169

160170
var state = {
161171
match: match,

lib/matchRoutes.js

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -12,16 +12,12 @@ var patternCache = {};
1212
/**
1313
* Match routes against a path
1414
*
15-
* @param {Array.<Route>} routes
16-
* @param {String} path
17-
* @param {[Object]} properties of parent router
15+
* @param {Array.<Route>} routes Available Routes.
16+
* @param {String} path Path to match.
17+
* @param {[Object|Array]} routerURLPatternOptions URLPattern options from parent router (and its parent and so on).
1818
*/
19-
function matchRoutes(routes, path, routerProps) {
20-
var match, page, notFound, queryObj;
21-
22-
if (!routerProps) { // optional
23-
routerProps = {};
24-
}
19+
function matchRoutes(routes, path, routerURLPatternOptions) {
20+
var match, page, notFound, queryObj, urlPatternOptions;
2521

2622
if (!Array.isArray(routes)) {
2723
routes = [routes];
@@ -47,12 +43,12 @@ function matchRoutes(routes, path, routerProps) {
4743
// Allow passing compiler options to url-pattern, see
4844
// https://github.com/snd/url-pattern#customize-the-pattern-syntax
4945
// Note that this blows up if you provide an empty object on a regex path
50-
var urlPatternOptions;
46+
urlPatternOptions = null;
5147
if (Array.isArray(current.props.urlPatternOptions) || current.props.path instanceof RegExp) {
5248
// If an array is passed, it takes precedence - assumed these are regexp keys
5349
urlPatternOptions = current.props.urlPatternOptions;
54-
} else if (routerProps.urlPatternOptions || current.props.urlPatternOptions) {
55-
urlPatternOptions = assign({}, routerProps.urlPatternOptions, current.props.urlPatternOptions);
50+
} else if (routerURLPatternOptions || current.props.urlPatternOptions) {
51+
urlPatternOptions = assign({}, routerURLPatternOptions, current.props.urlPatternOptions);
5652
}
5753

5854
// matchKeys is deprecated
@@ -122,6 +118,9 @@ function Match(path, route, match, query) {
122118
}
123119

124120
Match.prototype.getProps = function() {
121+
if (!this.route) {
122+
throw new Error("React-router-component: No route matched! Did you define a NotFound route?");
123+
}
125124
var props = assign({}, this.route.props, this.match);
126125
// Querystring is assigned as _query.
127126
props._query = this.query || {};

tests/matchRoutes.js

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ var React = require('react');
55
var Router = require('../');
66
var Location = React.createFactory(Router.Location);
77
var NotFound = React.createFactory(Router.NotFound);
8-
var URLPattern = require('url-pattern');
98

109
describe('matchRoutes', function() {
1110

@@ -15,15 +14,10 @@ describe('matchRoutes', function() {
1514
Location({path: '/mod/*', handler: React.createElement('div', {name: 'mod'})}),
1615
Location({path: /\/regex\/([a-zA-Z]*)$/, handler: React.createElement('div', {name: 'regex'})}),
1716
Location({path: /\/(.*?)\/(\d)\/([a-zA-Z]*)$/, handler: React.createElement('div', {name: 'regexMatch'}),
18-
matchKeys: ['name', 'num', 'text']}),
17+
urlPatternOptions: ['name', 'num', 'text']}),
1918
NotFound({handler: React.createElement('div', {name: 'notfound'})})
2019
];
2120

22-
afterEach(function() {
23-
// In case we overrode this, reset it.
24-
Router.createURLPatternCompiler = function () { return new URLPattern.Compiler(); };
25-
});
26-
2721
it('matches ""', function() {
2822
var match = matchRoutes(routes, '');
2923
assert(match.route);
@@ -65,9 +59,9 @@ describe('matchRoutes', function() {
6559
});
6660

6761
it('matches /cat/:id with a custom url-pattern options and periods in param', function() {
68-
var match = matchRoutes(routes, '/cat/hello.with.periods', {urlPatternOptions: {
62+
var match = matchRoutes(routes, '/cat/hello.with.periods', {
6963
segmentValueCharset: 'a-zA-Z0-9_\\- %\\.'
70-
}});
64+
});
7165
assert(match.route);
7266
assert.strictEqual(match.route.props.handler.props.name, 'cat');
7367
assert.deepEqual(match.match, {id: 'hello.with.periods'});
@@ -91,9 +85,7 @@ describe('matchRoutes', function() {
9185
wildcardChar: '?'
9286
};
9387

94-
var match = matchRoutes([route], 'https://www.github.com/strml/react-router-component', {
95-
urlPatternOptions: urlPatternOptions
96-
});
88+
var match = matchRoutes([route], 'https://www.github.com/strml/react-router-component', urlPatternOptions);
9789
assert(match.route);
9890
assert.strictEqual(match.route.props.handler.props.name, 'parseDomain');
9991
assert.deepEqual(match.match, {sub_domain: 'www', domain: 'github', 'toplevel-domain': 'com',

tests/server.js

Lines changed: 104 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ describe('react-router-component (on server)', function() {
3838
}),
3939
React.createElement(Location, {
4040
path: /\/z\/(.*)\/(.*)/,
41-
matchKeys: ['match1', 'match2'],
41+
urlPatternOptions: ['match1', 'match2'],
4242
handler: React.createClass({
4343
render: function() {
4444
return React.createElement('div', null, this.props.match1 + this.props.match2);
@@ -71,7 +71,7 @@ describe('react-router-component (on server)', function() {
7171
assert(markup.match(/ohhai/));
7272
});
7373

74-
it('renders with regex and matchKeys', function() {
74+
it('renders with regex and urlPatternOptions(matchKeys)', function() {
7575
var markup = ReactDOMServer.renderToString(React.createElement(App, {path: '/z/one/two'}));
7676
assert(markup.match(/class="App"/));
7777
assert(markup.match(/onetwo/));
@@ -318,4 +318,106 @@ describe('react-router-component (on server)', function() {
318318
});
319319
});
320320

321+
describe('urlPatternOptions hierarchy', function() {
322+
var Inner = React.createClass({
323+
displayName: 'Inner',
324+
325+
render: function() {
326+
return React.createElement('div', {}, this.props.foo + '|' + this.props.bar + this.props._);
327+
}
328+
});
329+
330+
var App = React.createClass({
331+
332+
getRoutes: function() {
333+
return [
334+
// Direct passthrough from Locations
335+
React.createElement(Location, {
336+
path: '/1/$foo/$bar',
337+
handler: Inner
338+
}),
339+
// Merge one property
340+
React.createElement(Location, {
341+
path: '/2/$foo/$bar',
342+
handler: Inner,
343+
urlPatternOptions: {
344+
segmentValueCharset: 'A-Z'
345+
}
346+
}),
347+
// Override a property
348+
React.createElement(Location, {
349+
path: '/3/!foo/!bar',
350+
handler: Inner,
351+
urlPatternOptions: {
352+
segmentNameStartChar: '!'
353+
}
354+
}),
355+
// Inherit from parent contextual
356+
React.createElement(Location, {
357+
path: '/4/[foo]?',
358+
handler: Inner,
359+
urlPatternOptions: {
360+
optionalSegmentStartChar: '[',
361+
optionalSegmentEndChar: ']'
362+
}
363+
}),
364+
// Parent props
365+
React.createElement(NotFound, {
366+
handler: React.createElement('div', null, 'not found')
367+
})
368+
]
369+
},
370+
371+
render: function() {
372+
return React.createElement('div', {className: 'App'},
373+
React.createElement(Locations, {
374+
path: this.props.path,
375+
urlPatternOptions: {
376+
wildcardChar: '?'
377+
}
378+
},
379+
React.createElement(Location, {
380+
path: '/start?',
381+
handler: React.createElement('div', null,
382+
React.createElement(Locations, {
383+
contextual: true,
384+
children: this.getRoutes(),
385+
urlPatternOptions: {
386+
segmentNameStartChar: '$'
387+
}
388+
})
389+
)
390+
}),
391+
React.createElement(NotFound, {
392+
handler: React.createElement('div', null, 'not found')
393+
})
394+
)
395+
);
396+
}
397+
});
398+
399+
400+
it('passes urlPatternOptions from parent <Locations>', function() {
401+
var markup = ReactDOMServer.renderToString(React.createElement(App, {path: '/start/1/biff/baz'}));
402+
assert(markup.match(/biff\|baz/));
403+
});
404+
405+
it('merges urlPatternOptions from parent <Locations> and a <Location>', function() {
406+
var markup = ReactDOMServer.renderToString(React.createElement(App, {path: '/start/2/BIFF/BA'}));
407+
assert(markup.match(/BIFF\|BA/));
408+
markup = ReactDOMServer.renderToString(React.createElement(App, {path: '/start/2/biff/ba'}));
409+
assert(markup.match(/not found/));
410+
});
411+
412+
it('gives urlPatternOptions on route precedence over router', function() {
413+
var markup = ReactDOMServer.renderToString(React.createElement(App, {path: '/start/3/biff/boff'}));
414+
assert(markup.match(/biff\|boff/));
415+
});
416+
417+
it('inherits from parent contextual router', function() {
418+
var markup = ReactDOMServer.renderToString(React.createElement(App, {path: '/start/4/foobar'}));
419+
assert(markup.match(/undefined\|undefinedbar/));
420+
});
421+
});
422+
321423
});

0 commit comments

Comments
 (0)