Skip to content
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

fix(aio): activate ServiceWorker updates asap #17699

Merged
merged 1 commit into from Jul 7, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 0 additions & 10 deletions aio/src/app/app.component.spec.ts
Expand Up @@ -18,14 +18,12 @@ import { Logger } from 'app/shared/logger.service';
import { MockLocationService } from 'testing/location.service';
import { MockLogger } from 'testing/logger.service';
import { MockSearchService } from 'testing/search.service';
import { MockSwUpdateNotificationsService } from 'testing/sw-update-notifications.service';
import { NavigationNode } from 'app/navigation/navigation.service';
import { ScrollService } from 'app/shared/scroll.service';
import { SearchBoxComponent } from 'app/search/search-box/search-box.component';
import { SearchResultsComponent } from 'app/search/search-results/search-results.component';
import { SearchService } from 'app/search/search.service';
import { SelectComponent, Option } from 'app/shared/select/select.component';
import { SwUpdateNotificationsService } from 'app/sw-updates/sw-update-notifications.service';
import { TocComponent } from 'app/embedded/toc/toc.component';
import { TocItem, TocService } from 'app/shared/toc.service';

Expand Down Expand Up @@ -68,13 +66,6 @@ describe('AppComponent', () => {
expect(component).toBeDefined();
});

describe('ServiceWorker update notifications', () => {
it('should be enabled', () => {
const swUpdateNotifications = TestBed.get(SwUpdateNotificationsService) as SwUpdateNotificationsService;
expect(swUpdateNotifications.enable).toHaveBeenCalled();
});
});

describe('hasFloatingToc', () => {
it('should initially be true', () => {
const fixture2 = TestBed.createComponent(AppComponent);
Expand Down Expand Up @@ -904,7 +895,6 @@ function createTestingModule(initialUrl: string) {
{ provide: LocationService, useFactory: () => new MockLocationService(initialUrl) },
{ provide: Logger, useClass: MockLogger },
{ provide: SearchService, useClass: MockSearchService },
{ provide: SwUpdateNotificationsService, useClass: MockSwUpdateNotificationsService },
]
});
}
Expand Down
4 changes: 0 additions & 4 deletions aio/src/app/app.component.ts
Expand Up @@ -11,7 +11,6 @@ import { ScrollService } from 'app/shared/scroll.service';
import { SearchResultsComponent } from 'app/search/search-results/search-results.component';
import { SearchBoxComponent } from 'app/search/search-box/search-box.component';
import { SearchService } from 'app/search/search.service';
import { SwUpdateNotificationsService } from 'app/sw-updates/sw-update-notifications.service';
import { TocService } from 'app/shared/toc.service';

import { BehaviorSubject } from 'rxjs/BehaviorSubject';
Expand Down Expand Up @@ -106,7 +105,6 @@ export class AppComponent implements OnInit {
private navigationService: NavigationService,
private scrollService: ScrollService,
private searchService: SearchService,
private swUpdateNotifications: SwUpdateNotificationsService,
private tocService: TocService
) { }

Expand Down Expand Up @@ -177,8 +175,6 @@ export class AppComponent implements OnInit {

this.navigationService.versionInfo.subscribe( vi => this.versionInfo = vi );

this.swUpdateNotifications.enable();

const hasNonEmptyToc = this.tocService.tocList.map(tocList => tocList.length > 0);
combineLatest(hasNonEmptyToc, this.showFloatingToc)
.subscribe(([hasToc, showFloatingToc]) => this.hasFloatingToc = hasToc && showFloatingToc);
Expand Down
3 changes: 2 additions & 1 deletion aio/src/app/embedded/embedded.module.ts
Expand Up @@ -9,7 +9,7 @@ import { PrettyPrinter } from './code/pretty-printer.service';
// It is not enough just to import them inside the AppModule

// Reusable components (used inside embedded components)
import { MdIconModule, MdTabsModule } from '@angular/material';
import { MdIconModule, MdSnackBarModule, MdTabsModule } from '@angular/material';
import { CodeComponent } from './code/code.component';
import { SharedModule } from 'app/shared/shared.module';

Expand Down Expand Up @@ -42,6 +42,7 @@ export class EmbeddedComponents {
imports: [
CommonModule,
MdIconModule,
MdSnackBarModule,
MdTabsModule,
SharedModule
],
Expand Down
26 changes: 25 additions & 1 deletion aio/src/app/shared/location.service.spec.ts
@@ -1,26 +1,31 @@
import { ReflectiveInjector } from '@angular/core';
import { Location, LocationStrategy, PlatformLocation } from '@angular/common';
import { MockLocationStrategy } from '@angular/common/testing';
import { Subject } from 'rxjs/Subject';

import { GaService } from 'app/shared/ga.service';
import { SwUpdatesService } from 'app/sw-updates/sw-updates.service';
import { LocationService } from './location.service';

describe('LocationService', () => {
let injector: ReflectiveInjector;
let location: MockLocationStrategy;
let service: LocationService;
let swUpdates: MockSwUpdatesService;

beforeEach(() => {
injector = ReflectiveInjector.resolveAndCreate([
LocationService,
Location,
{ provide: GaService, useClass: TestGaService },
{ provide: LocationStrategy, useClass: MockLocationStrategy },
{ provide: PlatformLocation, useClass: MockPlatformLocation }
{ provide: PlatformLocation, useClass: MockPlatformLocation },
{ provide: SwUpdatesService, useClass: MockSwUpdatesService }
]);

location = injector.get(LocationStrategy);
service = injector.get(LocationService);
swUpdates = injector.get(SwUpdatesService);
});

describe('currentUrl', () => {
Expand Down Expand Up @@ -289,6 +294,21 @@ describe('LocationService', () => {
expect(goExternalSpy).toHaveBeenCalledWith(externalUrl);
});

it('should do a "full page navigation" if a ServiceWorker update has been activated', () => {
const goExternalSpy = spyOn(service, 'goExternal');

// Internal URL - No ServiceWorker update
service.go('some-internal-url');
expect(goExternalSpy).not.toHaveBeenCalled();
expect(location.path(true)).toEqual('some-internal-url');

// Internal URL - ServiceWorker update
swUpdates.updateActivated.next('foo');
service.go('other-internal-url');
expect(goExternalSpy).toHaveBeenCalledWith('other-internal-url');
expect(location.path(true)).toEqual('some-internal-url');
});

it('should not update currentUrl for external url that starts with "http"', () => {
let localUrl: string;
spyOn(service, 'goExternal');
Expand Down Expand Up @@ -588,6 +608,10 @@ class MockPlatformLocation {
replaceState = jasmine.createSpy('PlatformLocation.replaceState');
}

class MockSwUpdatesService {
updateActivated = new Subject<string>();
}

class TestGaService {
locationChanged = jasmine.createSpy('locationChanged');
}
13 changes: 10 additions & 3 deletions aio/src/app/shared/location.service.ts
Expand Up @@ -6,37 +6,44 @@ import { ReplaySubject } from 'rxjs/ReplaySubject';
import 'rxjs/add/operator/do';

import { GaService } from 'app/shared/ga.service';
import { SwUpdatesService } from 'app/sw-updates/sw-updates.service';

@Injectable()
export class LocationService {

private readonly urlParser = document.createElement('a');
private urlSubject = new ReplaySubject<string>(1);
private swUpdateActivated = false;

currentUrl = this.urlSubject
.map(url => this.stripSlashes(url));

currentPath = this.currentUrl
.map(url => url.match(/[^?#]*/)[0]) // strip query and hash
.do(url => this.gaService.locationChanged(url));
.do(path => this.gaService.locationChanged(path));

constructor(
private gaService: GaService,
private location: Location,
private platformLocation: PlatformLocation) {
private platformLocation: PlatformLocation,
swUpdates: SwUpdatesService) {

this.urlSubject.next(location.path(true));

this.location.subscribe(state => {
return this.urlSubject.next(state.url);
});

swUpdates.updateActivated.subscribe(() => this.swUpdateActivated = true);
}

// TODO?: ignore if url-without-hash-or-search matches current location?
go(url: string) {
if (!url) { return; }
url = this.stripSlashes(url);
if (/^http/.test(url)) {
if (/^http/.test(url) || this.swUpdateActivated) {
// Has http protocol so leave the site
// (or do a "full page navigation" if a ServiceWorker update has been activated)
this.goExternal(url);
} else {
this.location.go(url);
Expand Down
18 changes: 0 additions & 18 deletions aio/src/app/sw-updates/global.value.spec.ts

This file was deleted.

8 changes: 0 additions & 8 deletions aio/src/app/sw-updates/global.value.ts

This file was deleted.