Skip to content

Commit 9c1318d

Browse files
gkalpakmhevery
authored andcommitted
fix(aio): strip leading slashes from path (and improve DRY-ness) (angular#16238)
Previously, the path returned by `LocationService.path()` preserved leading slashes, which resulted in requests with consequtive slashes in the URL. Such requests would fail (with a 404) on staging. This commit fixes it, by removing leading slashes from the path. It also refactors `LocationService` a bit, converting path to an observable, `currentPath` (similar to `currentUrl`), and applies certain clean-ups (e.g. stripping slashes, query, hash) in one place, which simplifies consumption. Closes angular#16230
1 parent 062fc4a commit 9c1318d

File tree

9 files changed

+228
-97
lines changed

9 files changed

+228
-97
lines changed

aio/src/app/app.component.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ export class AppComponent implements OnInit {
7676
});
7777

7878
// scroll even if only the hash fragment changed
79-
this.locationService.currentUrl.subscribe(url => this.autoScroll());
79+
this.locationService.currentUrl.subscribe(() => this.autoScroll());
8080

8181
this.navigationService.currentNode.subscribe(currentNode => {
8282
this.currentNode = currentNode;

aio/src/app/documents/document.service.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ export class DocumentService {
3939
private http: Http,
4040
location: LocationService) {
4141
// Whenever the URL changes we try to get the appropriate doc
42-
this.currentDocument = location.currentUrl.switchMap(() => this.getDocument(location.path()));
42+
this.currentDocument = location.currentPath.switchMap(path => this.getDocument(path));
4343
}
4444

4545
private getDocument(url: string) {
Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,19 @@
11
import { async, ComponentFixture, TestBed } from '@angular/core/testing';
22
import { LocationService } from 'app/shared/location.service';
3+
import { MockLocationService } from 'testing/location.service';
34
import { CurrentLocationComponent } from './current-location.component';
45

5-
let currentPath: string;
6-
class MockLocation {
7-
path() { return currentPath; }
8-
}
96

107
describe('CurrentLocationComponent', () => {
8+
let locationService: MockLocationService;
9+
1110
beforeEach(async(() => {
11+
locationService = new MockLocationService('initial/url');
12+
1213
TestBed.configureTestingModule({
1314
declarations: [ CurrentLocationComponent ],
1415
providers: [
15-
{ provide: LocationService, useClass: MockLocation }
16+
{ provide: LocationService, useValue: locationService }
1617
]
1718
});
1819
TestBed.compileComponents();
@@ -21,8 +22,13 @@ describe('CurrentLocationComponent', () => {
2122
it('should render the current location', () => {
2223
const fixture = TestBed.createComponent(CurrentLocationComponent);
2324
const element: HTMLElement = fixture.nativeElement;
24-
currentPath = 'a/b/c';
25+
26+
fixture.detectChanges();
27+
expect(element.innerText).toEqual('initial/url');
28+
29+
locationService.urlSubject.next('next/url');
30+
2531
fixture.detectChanges();
26-
expect(element.innerText).toEqual('a/b/c');
32+
expect(element.innerText).toEqual('next/url');
2733
});
2834
});

aio/src/app/embedded/current-location.component.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import { LocationService } from 'app/shared/location.service';
77
*/
88
@Component({
99
selector: 'current-location',
10-
template: '{{location.path()}}'
10+
template: '{{ location.currentPath | async }}'
1111
})
1212
export class CurrentLocationComponent {
1313
constructor(public location: LocationService) {

aio/src/app/navigation/navigation.service.spec.ts

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -178,9 +178,6 @@ describe('NavigationService', () => {
178178
locationService.go('c');
179179
expect(currentNode).toEqual(cnode, 'location: c');
180180

181-
locationService.go('c/');
182-
expect(currentNode).toEqual(cnode, 'location: c/');
183-
184181
locationService.go('c#foo');
185182
expect(currentNode).toEqual(cnode, 'location: c#foo');
186183

aio/src/app/navigation/navigation.service.ts

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,6 @@ export { CurrentNode, NavigationNode, NavigationResponse, NavigationViews, Versi
1616

1717
const navigationPath = 'content/navigation.json';
1818

19-
const urlParser = document.createElement('a');
20-
function cleanUrl(url: string) {
21-
// remove hash (#) and query params (?)
22-
urlParser.href = '/' + url;
23-
// strip leading and trailing slashes
24-
return urlParser.pathname.replace(/^\/+|\/$/g, '');
25-
}
26-
2719
@Injectable()
2820
export class NavigationService {
2921
/**
@@ -91,10 +83,9 @@ export class NavigationService {
9183
private getCurrentNode(navigationViews: Observable<NavigationViews>): Observable<CurrentNode> {
9284
const currentNode = combineLatest(
9385
navigationViews.map(this.computeUrlToNavNodesMap),
94-
this.location.currentUrl,
86+
this.location.currentPath,
9587
(navMap, url) => {
96-
let urlKey = cleanUrl(url);
97-
urlKey = urlKey.startsWith('api/') ? 'api' : urlKey;
88+
const urlKey = url.startsWith('api/') ? 'api' : url;
9889
return navMap[urlKey] || { view: '', url: urlKey, nodes: [] };
9990
})
10091
.publishReplay(1);

0 commit comments

Comments
 (0)