From 6abbaaed89eda18b79ed861e5e08f8a487dd4fc5 Mon Sep 17 00:00:00 2001 From: George Kalpakas Date: Tue, 27 Nov 2018 17:16:08 +0200 Subject: [PATCH] fix(docs-infra): do not use an Angular element in hard-coded `FETCH_ERROR` document (#27250) The `FETCH_ERROR` document is used when we are unable to retrieve a document (except for 404 errors), which includes when there is no internet connection. Using the `` element in the document's template to show the path of the page we failed to retrieve assumes that the element's bundle is available (e.g. cached by the SW) or can be fetched from the server. When none of these conditions is met, the `DocViewer` is unable to prepare the document and fails, never showing the `FETCH_ERROR` page to the user. Furthermore, the path we are looking to retrieve via `` is essentially the document ID, which we already have. Thus, loading and instantiating a whole component just for that is overkill. This commit addresses both issues by getting rid of the `` component and directly embedding the document ID into the `FETCH_ERROR` content. PR Close #27250 --- aio/content/marketing/test.html | 2 -- aio/scripts/_payload-limits.json | 4 +-- .../current-location.component.spec.ts | 35 ------------------- .../current-location.component.ts | 12 ------- .../current-location.module.ts | 13 ------- .../app/custom-elements/element-registry.ts | 4 --- .../app/documents/document.service.spec.ts | 1 + aio/src/app/documents/document.service.ts | 8 ++--- 8 files changed, 7 insertions(+), 72 deletions(-) delete mode 100644 aio/src/app/custom-elements/current-location/current-location.component.spec.ts delete mode 100644 aio/src/app/custom-elements/current-location/current-location.component.ts delete mode 100644 aio/src/app/custom-elements/current-location/current-location.module.ts diff --git a/aio/content/marketing/test.html b/aio/content/marketing/test.html index 3788b00e2d..6b88523746 100644 --- a/aio/content/marketing/test.html +++ b/aio/content/marketing/test.html @@ -1,7 +1,5 @@

Test Code Page

-

Current location is

-

<code-tabs>

No linenums at code-tabs level

diff --git a/aio/scripts/_payload-limits.json b/aio/scripts/_payload-limits.json index 37ac38652b..28394ecb8a 100755 --- a/aio/scripts/_payload-limits.json +++ b/aio/scripts/_payload-limits.json @@ -2,8 +2,8 @@ "aio": { "master": { "uncompressed": { - "runtime": 3881, - "main": 502188, + "runtime": 3799, + "main": 501972, "polyfills": 53926 } } diff --git a/aio/src/app/custom-elements/current-location/current-location.component.spec.ts b/aio/src/app/custom-elements/current-location/current-location.component.spec.ts deleted file mode 100644 index 58dac0445f..0000000000 --- a/aio/src/app/custom-elements/current-location/current-location.component.spec.ts +++ /dev/null @@ -1,35 +0,0 @@ -import { ComponentFixture, TestBed } from '@angular/core/testing'; -import { LocationService } from 'app/shared/location.service'; -import { MockLocationService } from 'testing/location.service'; -import { CurrentLocationComponent } from './current-location.component'; - - -describe('CurrentLocationComponent', () => { - let element: HTMLElement; - let fixture: ComponentFixture; - let locationService: MockLocationService; - - beforeEach(() => { - locationService = new MockLocationService('initial/url'); - - TestBed.configureTestingModule({ - declarations: [ CurrentLocationComponent ], - providers: [ - { provide: LocationService, useValue: locationService } - ] - }); - - fixture = TestBed.createComponent(CurrentLocationComponent); - element = fixture.nativeElement; - }); - - it('should render the current location', () => { - fixture.detectChanges(); - expect(element.textContent).toEqual('initial/url'); - - locationService.urlSubject.next('next/url'); - - fixture.detectChanges(); - expect(element.textContent).toEqual('next/url'); - }); -}); diff --git a/aio/src/app/custom-elements/current-location/current-location.component.ts b/aio/src/app/custom-elements/current-location/current-location.component.ts deleted file mode 100644 index 38af7b148d..0000000000 --- a/aio/src/app/custom-elements/current-location/current-location.component.ts +++ /dev/null @@ -1,12 +0,0 @@ -/* tslint:disable component-selector */ -import { Component } from '@angular/core'; -import { LocationService } from 'app/shared/location.service'; - -/** Renders the current location path. */ -@Component({ - selector: 'current-location', - template: '{{ location.currentPath | async }}' -}) -export class CurrentLocationComponent { - constructor(public location: LocationService) { } -} diff --git a/aio/src/app/custom-elements/current-location/current-location.module.ts b/aio/src/app/custom-elements/current-location/current-location.module.ts deleted file mode 100644 index dce7885408..0000000000 --- a/aio/src/app/custom-elements/current-location/current-location.module.ts +++ /dev/null @@ -1,13 +0,0 @@ -import { NgModule, Type } from '@angular/core'; -import { CommonModule } from '@angular/common'; -import { CurrentLocationComponent } from './current-location.component'; -import { WithCustomElementComponent } from '../element-registry'; - -@NgModule({ - imports: [ CommonModule ], - declarations: [ CurrentLocationComponent ], - entryComponents: [ CurrentLocationComponent ] -}) -export class CurrentLocationModule implements WithCustomElementComponent { - customElementComponent: Type = CurrentLocationComponent; -} diff --git a/aio/src/app/custom-elements/element-registry.ts b/aio/src/app/custom-elements/element-registry.ts index 0a4d480938..1359e22752 100644 --- a/aio/src/app/custom-elements/element-registry.ts +++ b/aio/src/app/custom-elements/element-registry.ts @@ -36,10 +36,6 @@ export const ELEMENT_MODULE_PATHS_AS_ROUTES = [ selector: 'code-tabs', loadChildren: './code/code-tabs.module#CodeTabsModule' }, - { - selector: 'current-location', - loadChildren: './current-location/current-location.module#CurrentLocationModule' - }, { selector: 'expandable-section', loadChildren: './expandable-section/expandable-section.module#ExpandableSectionModule' diff --git a/aio/src/app/documents/document.service.spec.ts b/aio/src/app/documents/document.service.spec.ts index 1896c1c9a8..32789edc8d 100644 --- a/aio/src/app/documents/document.service.spec.ts +++ b/aio/src/app/documents/document.service.spec.ts @@ -113,6 +113,7 @@ describe('DocumentService', () => { httpMock.expectOne({}).flush(null, {status: 500, statusText: 'Server Error'}); expect(latestDocument.id).toEqual(FETCHING_ERROR_ID); + expect(latestDocument.contents).toContain('We are unable to retrieve the "initial/doc" page at this time.'); expect(logger.output.error).toEqual([ [jasmine.any(Error)] ]); diff --git a/aio/src/app/documents/document.service.ts b/aio/src/app/documents/document.service.ts index 432e1c4ab2..83a5da9e33 100644 --- a/aio/src/app/documents/document.service.ts +++ b/aio/src/app/documents/document.service.ts @@ -15,13 +15,13 @@ export const FETCHING_ERROR_ID = 'fetching-error'; export const CONTENT_URL_PREFIX = 'generated/'; export const DOC_CONTENT_URL_PREFIX = CONTENT_URL_PREFIX + 'docs/'; -const FETCHING_ERROR_CONTENTS = ` +const FETCHING_ERROR_CONTENTS = (path: string) => `
error_outline

Request for document failed.

- We are unable to retrieve the "" page at this time. + We are unable to retrieve the "${path}" page at this time. Please check your connection and try again later.

@@ -46,7 +46,7 @@ export class DocumentService { private getDocument(url: string) { const id = url || 'index'; this.logger.log('getting document', id); - if ( !this.cache.has(id)) { + if (!this.cache.has(id)) { this.cache.set(id, this.fetchDocument(id)); } return this.cache.get(id)!; @@ -93,7 +93,7 @@ export class DocumentService { this.cache.delete(id); return of({ id: FETCHING_ERROR_ID, - contents: FETCHING_ERROR_CONTENTS + contents: FETCHING_ERROR_CONTENTS(id), }); } }