Skip to content

Commit 17392f6

Browse files
btfordbtford
committed
fix(router): sort possible routes by cost
1 parent 8b6fa1c commit 17392f6

6 files changed

Lines changed: 78 additions & 33 deletions

File tree

‎modules/angular2/src/router/instruction.js‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,15 +20,18 @@ export class Instruction {
2020
matchedUrl:string;
2121
params:Map<string, string>;
2222
reuse:boolean;
23+
cost:number;
2324

24-
constructor({params, component, children, matchedUrl}:{params:StringMap, component:any, children:Map, matchedUrl:string} = {}) {
25+
constructor({params, component, children, matchedUrl, parentCost}:{params:StringMap, component:any, children:Map, matchedUrl:string, cost:int} = {}) {
2526
this.reuse = false;
2627
this.matchedUrl = matchedUrl;
28+
this.cost = parentCost;
2729
if (isPresent(children)) {
2830
this._children = children;
2931
var childUrl;
3032
StringMapWrapper.forEach(this._children, (child, _) => {
3133
childUrl = child.matchedUrl;
34+
this.cost += child.cost;
3235
});
3336
if (isPresent(childUrl)) {
3437
this.matchedUrl += childUrl;

‎modules/angular2/src/router/path_recognizer.js‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ class StarSegment {
5252
var paramMatcher = RegExpWrapper.create("^:([^\/]+)$");
5353
var wildcardMatcher = RegExpWrapper.create("^\\*([^\/]+)$");
5454

55-
function parsePathString(route:string):List {
55+
function parsePathString(route:string) {
5656
// normalize route as not starting with a "/". Recognition will
5757
// also normalize.
5858
if (route[0] === "/") {
@@ -61,21 +61,25 @@ function parsePathString(route:string):List {
6161

6262
var segments = splitBySlash(route);
6363
var results = ListWrapper.create();
64+
var cost = 0;
6465

6566
for (var i=0; i<segments.length; i++) {
6667
var segment = segments[i],
67-
match;
68+
match;
6869

6970
if (isPresent(match = RegExpWrapper.firstMatch(paramMatcher, segment))) {
7071
ListWrapper.push(results, new DynamicSegment(match[1]));
72+
cost += 100;
7173
} else if (isPresent(match = RegExpWrapper.firstMatch(wildcardMatcher, segment))) {
7274
ListWrapper.push(results, new StarSegment(match[1]));
75+
cost += 10000;
7376
} else if (segment.length > 0) {
7477
ListWrapper.push(results, new StaticSegment(segment));
78+
cost += 1;
7579
}
7680
}
7781

78-
return results;
82+
return {segments: results, cost};
7983
}
8084

8185
var SLASH_RE = RegExpWrapper.create('/');
@@ -89,12 +93,17 @@ export class PathRecognizer {
8993
segments:List;
9094
regex:RegExp;
9195
handler:any;
96+
cost:number;
9297

9398
constructor(path:string, handler:any) {
9499
this.handler = handler;
95100
this.segments = ListWrapper.create();
96101

97-
var segments = parsePathString(path);
102+
// TODO: use destructuring assignment
103+
// see https://github.com/angular/ts2dart/issues/158
104+
var parsed = parsePathString(path);
105+
var cost = parsed['cost'];
106+
var segments = parsed['segments'];
98107
var regexString = '^';
99108

100109
ListWrapper.forEach(segments, (segment) => {
@@ -103,6 +112,7 @@ export class PathRecognizer {
103112

104113
this.regex = RegExpWrapper.create(regexString);
105114
this.segments = segments;
115+
this.cost = cost;
106116
}
107117

108118
parseParams(url:string):StringMap {

‎modules/angular2/src/router/route_recognizer.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ export class RouteRecognizer {
3939
var match;
4040
if (isPresent(match = RegExpWrapper.firstMatch(regex, url))) {
4141
var solution = StringMapWrapper.create();
42+
StringMapWrapper.set(solution, 'cost', pathRecognizer.cost);
4243
StringMapWrapper.set(solution, 'handler', pathRecognizer.handler);
4344
StringMapWrapper.set(solution, 'params', pathRecognizer.parseParams(url));
4445

‎modules/angular2/src/router/route_registry.js‎

Lines changed: 37 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -77,39 +77,46 @@ export class RouteRegistry {
7777
return null;
7878
}
7979

80-
var solutions = componentRecognizer.recognize(url);
80+
var componentSolutions = componentRecognizer.recognize(url);
81+
var fullSolutions = ListWrapper.create();
8182

82-
for(var i = 0; i < solutions.length; i++) {
83-
var candidate = solutions[i];
83+
for(var i = 0; i < componentSolutions.length; i++) {
84+
var candidate = componentSolutions[i];
8485
if (candidate['unmatchedUrl'].length == 0) {
85-
return handlerToLeafInstructions(candidate, parentComponent);
86-
}
87-
88-
var children = StringMapWrapper.create(),
89-
allMapped = true;
86+
ListWrapper.push(fullSolutions, handlerToLeafInstructions(candidate, parentComponent));
87+
} else {
88+
var children = StringMapWrapper.create(),
89+
allMapped = true;
90+
91+
StringMapWrapper.forEach(candidate['handler']['components'], (component, name) => {
92+
if (!allMapped) {
93+
return;
94+
}
95+
var childInstruction = this.recognize(candidate['unmatchedUrl'], component);
96+
if (isPresent(childInstruction)) {
97+
childInstruction.params = candidate['params'];
98+
children[name] = childInstruction;
99+
} else {
100+
allMapped = false;
101+
}
102+
});
90103

91-
StringMapWrapper.forEach(candidate['handler']['components'], (component, name) => {
92-
if (!allMapped) {
93-
return;
94-
}
95-
var childInstruction = this.recognize(candidate['unmatchedUrl'], component);
96-
if (isPresent(childInstruction)) {
97-
childInstruction.params = candidate['params'];
98-
children[name] = childInstruction;
99-
} else {
100-
allMapped = false;
104+
if (allMapped) {
105+
ListWrapper.push(fullSolutions, new Instruction({
106+
component: parentComponent,
107+
children: children,
108+
matchedUrl: candidate['matchedUrl'],
109+
parentCost: candidate['cost']
110+
}));
101111
}
102-
});
103-
104-
if (allMapped) {
105-
return new Instruction({
106-
component: parentComponent,
107-
children: children,
108-
matchedUrl: candidate['matchedUrl']
109-
});
110112
}
111113
}
112114

115+
if (fullSolutions.length > 0) {
116+
ListWrapper.sort(fullSolutions, (a, b) => a.cost < b.cost ? -1 : 1);
117+
return fullSolutions[0];
118+
}
119+
113120
return null;
114121
}
115122

@@ -127,13 +134,15 @@ function handlerToLeafInstructions(context, parentComponent) {
127134
StringMapWrapper.forEach(context['handler']['components'], (component, outletName) => {
128135
children[outletName] = new Instruction({
129136
component: component,
130-
params: context['params']
137+
params: context['params'],
138+
parentCost: 0
131139
});
132140
});
133141
return new Instruction({
134142
component: parentComponent,
135143
children: children,
136-
matchedUrl: context['matchedUrl']
144+
matchedUrl: context['matchedUrl'],
145+
parentCost: context['cost']
137146
});
138147
}
139148

‎modules/angular2/test/router/route_recognizer_spec.js‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ export function main() {
2323
recognizer.addConfig('/test', handler);
2424

2525
expect(recognizer.recognize('/test')[0]).toEqual({
26+
'cost' : 1,
2627
'handler': { 'components': { 'a': 'b' } },
2728
'params': {},
2829
'matchedUrl': '/test',
@@ -34,6 +35,7 @@ export function main() {
3435
recognizer.addConfig('/', handler);
3536

3637
expect(recognizer.recognize('/')[0]).toEqual({
38+
'cost': 0,
3739
'handler': { 'components': { 'a': 'b' } },
3840
'params': {},
3941
'matchedUrl': '/',
@@ -44,6 +46,7 @@ export function main() {
4446
it('should work with a dynamic segment', () => {
4547
recognizer.addConfig('/user/:name', handler);
4648
expect(recognizer.recognize('/user/brian')[0]).toEqual({
49+
'cost': 101,
4750
'handler': handler,
4851
'params': { 'name': 'brian' },
4952
'matchedUrl': '/user/brian',
@@ -57,6 +60,7 @@ export function main() {
5760
var solutions = recognizer.recognize('/a');
5861
expect(solutions.length).toBe(1);
5962
expect(solutions[0]).toEqual({
63+
'cost': 1,
6064
'handler': handler,
6165
'params': {},
6266
'matchedUrl': '/b',

‎modules/angular2/test/router/route_registry_spec.js‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,24 @@ export function main() {
2727
expect(instruction.getChildInstruction('default').component).toBe(DummyCompB);
2828
});
2929

30+
it('should prefer static segments to dynamic', () => {
31+
registry.config(rootHostComponent, {'path': '/:site', 'component': DummyCompB});
32+
registry.config(rootHostComponent, {'path': '/home', 'component': DummyCompA});
33+
34+
var instruction = registry.recognize('/home', rootHostComponent);
35+
36+
expect(instruction.getChildInstruction('default').component).toBe(DummyCompA);
37+
});
38+
39+
it('should prefer dynamic segments to star', () => {
40+
registry.config(rootHostComponent, {'path': '/:site', 'component': DummyCompA});
41+
registry.config(rootHostComponent, {'path': '/*site', 'component': DummyCompB});
42+
43+
var instruction = registry.recognize('/home', rootHostComponent);
44+
45+
expect(instruction.getChildInstruction('default').component).toBe(DummyCompA);
46+
});
47+
3048
it('should match the full URL recursively', () => {
3149
registry.config(rootHostComponent, {'path': '/first', 'component': DummyParentComp});
3250

0 commit comments

Comments
 (0)