Skip to content

Commit

Permalink
refactor(docs-infra): cleanup deprecated code (angular#49671)
Browse files Browse the repository at this point in the history
This commit replaces (non material-related) deprecated code present in the aio app.

* `pageYOffset` can be replaced by `scrollY`
*  RxJs' `mapTo()` is just a `map()`
* `createNgModuleRef` can be replaced by `createNgModule`
* HttpEmits `ProgressEvent` not `ErrorEvent`  (see angular#34748)
* `SwUpdate.available` is replaced by  `versionUpdates` with a `filter`
* `SwUpdate.activated` is replaced by the returned promised of `SwUpdate.activateUpdate`.

PR Close angular#49671
  • Loading branch information
JeanMeche authored and thePunderWoman committed Apr 17, 2023
1 parent f3366c6 commit 288f2c8
Show file tree
Hide file tree
Showing 11 changed files with 68 additions and 65 deletions.
1 change: 1 addition & 0 deletions aio/scripts/deploy-to-firebase/pre-deploy-actions.spec.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ describe('deploy-to-firebase/pre-deploy-actions:', () => {

describe('undo.checkPayloadSize()', () => {
// This method is a no-op, so there is nothing to test.
// eslint-disable-next-line jasmine/expect-single-argument
it('does not need tests', () => expect().nothing());
});

Expand Down
6 changes: 3 additions & 3 deletions aio/src/app/app.component.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import { SearchResultsComponent } from 'app/shared/search-results/search-results
import { TocItem, TocService } from 'app/shared/toc.service';
import { SwUpdatesService } from 'app/sw-updates/sw-updates.service';
import { of, Subject, timer } from 'rxjs';
import { first, mapTo } from 'rxjs/operators';
import { first, map } from 'rxjs/operators';
import { MockLocationService } from 'testing/location.service';
import { MockLogger } from 'testing/logger.service';
import { MockSearchService } from 'testing/search.service';
Expand Down Expand Up @@ -1431,7 +1431,7 @@ class TestHttpClient {
};

get(url: string) {
let data;
let data: any;
if (/navigation\.json/.test(url)) {
data = this.navJson;
} else {
Expand All @@ -1445,7 +1445,7 @@ class TestHttpClient {
}

// Preserve async nature of `HttpClient`.
return timer(1).pipe(mapTo(data));
return timer(1).pipe(map(() => data));
}
}

Expand Down
2 changes: 1 addition & 1 deletion aio/src/app/app.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,7 @@ export class AppComponent implements OnInit {
}
}

this.tocMaxHeight = (document.body.scrollHeight - window.pageYOffset - this.tocMaxHeightOffset).toFixed(2);
this.tocMaxHeight = (document.body.scrollHeight - window.scrollY - this.tocMaxHeightOffset).toFixed(2);
}

// Restrain scrolling inside an element, when the cursor is over it
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ describe('AnnouncementBarComponent', () => {
it('should handle a failed request for `announcements.json`', () => {
component.ngOnInit();
const request = httpMock.expectOne('generated/announcements.json');
request.error(new ErrorEvent('404'));
request.error(new ProgressEvent('404'));
expect(component.announcement).toBeUndefined();
expect(mockLogger.output.error).toEqual([
[jasmine.any(Error)]
Expand Down
4 changes: 2 additions & 2 deletions aio/src/app/custom-elements/elements-loader.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { createNgModuleRef, Inject, Injectable, NgModuleRef, Type } from '@angular/core';
import { createNgModule, Inject, Injectable, NgModuleRef, Type } from '@angular/core';
import { ELEMENT_MODULE_LOAD_CALLBACKS_TOKEN, WithCustomElementComponent } from './element-registry';
import { from, Observable, of } from 'rxjs';
import { createCustomElement } from '@angular/elements';
Expand Down Expand Up @@ -46,7 +46,7 @@ export class ElementsLoader {
const loadedAndRegistered =
(modulePathLoader() as Promise<Type<WithCustomElementComponent>>)
.then(elementModule => {
const elementModuleRef = createNgModuleRef(elementModule, this.moduleRef.injector);
const elementModuleRef = createNgModule(elementModule, this.moduleRef.injector);
const injector = elementModuleRef.injector;
const CustomElementComponent = elementModuleRef.instance.customElementComponent;
const CustomElement = createCustomElement(CustomElementComponent, {injector});
Expand Down
4 changes: 2 additions & 2 deletions aio/src/app/custom-elements/events/events.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ describe('EventsService', () => {

it('should handle a failed request for `events.json`', () => {
const request = httpMock.expectOne('generated/events.json');
request.error(new ErrorEvent('404'));
request.error(new ProgressEvent('404'));
expect(mockLogger.output.error).toEqual([
[jasmine.any(Error)]
]);
Expand All @@ -47,7 +47,7 @@ describe('EventsService', () => {

it('should return an empty array on a failed request for `events.json`', done => {
const request = httpMock.expectOne('generated/events.json');
request.error(new ErrorEvent('404'));
request.error(new ProgressEvent('404'));
eventsService.events.subscribe(results => {
expect(results).toEqual([]);
done();
Expand Down
2 changes: 1 addition & 1 deletion aio/src/app/shared/scroll-spy.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ export class ScrollSpyService {
}

private getScrollTop() {
return window && window.pageYOffset || 0;
return window && window.scrollY || 0;
}

private getTopOffset() {
Expand Down
8 changes: 4 additions & 4 deletions aio/src/app/shared/scroll.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ describe('ScrollService', () => {

it('should return `<body>` if unable to find the top-of-page element', () => {
(document.getElementById as jasmine.Spy).and.returnValue(null);
expect(scrollService.topOfPageElement).toBe(document.body as any);
expect(scrollService.topOfPageElement).toBe(document.body);
});
});

Expand Down Expand Up @@ -274,14 +274,14 @@ describe('ScrollService', () => {
it('should scroll all the way to the top if close enough', () => {
const element: HTMLElement = new MockElement() as any;

(window as any).pageYOffset = 25;
window.scrollY = 25;
scrollService.scrollToElement(element);

expect(element.scrollIntoView).toHaveBeenCalled();
expect(window.scrollBy).toHaveBeenCalledWith(0, -scrollService.topOffset);
(window.scrollBy as jasmine.Spy).calls.reset();

(window as any).pageYOffset = 15;
window.scrollY = 15;
scrollService.scrollToElement(element);

expect(element.scrollIntoView).toHaveBeenCalled();
Expand All @@ -297,7 +297,7 @@ describe('ScrollService', () => {

describe('#scrollToTop', () => {
it('should scroll to top', () => {
const topOfPageElement = new MockElement() as any as Element;
const topOfPageElement = new MockElement();
document.getElementById.and.callFake(
(id: string) => id === 'top-of-page' ? topOfPageElement : null);

Expand Down
4 changes: 2 additions & 2 deletions aio/src/app/shared/scroll.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -161,8 +161,8 @@ export class ScrollService implements OnDestroy {

// If we are very close to the top (<20px), then scroll all the way up.
// (This can happen if `element` is at the top of the page, but has a small top-margin.)
if (window.pageYOffset < 20) {
window.scrollBy(0, -window.pageYOffset);
if (window.scrollY < 20) {
window.scrollBy(0, -window.scrollY);
}
}
}
Expand Down
75 changes: 39 additions & 36 deletions aio/src/app/sw-updates/sw-updates.service.spec.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { ApplicationRef, ErrorHandler, Injector } from '@angular/core';
import { discardPeriodicTasks, fakeAsync, tick } from '@angular/core/testing';
import { SwUpdate } from '@angular/service-worker';
import { SwUpdate, VersionEvent, VersionReadyEvent } from '@angular/service-worker';
import { BehaviorSubject, Subject } from 'rxjs';

import { LocationService } from 'app/shared/location.service';
Expand Down Expand Up @@ -89,7 +89,7 @@ describe('SwUpdatesService', () => {
appRef.isStable.next(true);
expect(swu.activateUpdate).not.toHaveBeenCalled();

swu.$$availableSubj.next({available: {hash: 'foo'}});
swu.$$versionUpdatesSubj.next({latestVersion: {hash: 'foo'}, type: 'VERSION_READY'} as VersionReadyEvent);
expect(swu.activateUpdate).toHaveBeenCalled();
})));

Expand All @@ -100,7 +100,7 @@ describe('SwUpdatesService', () => {
tick(checkInterval);
expect(swu.checkForUpdate).toHaveBeenCalledTimes(1);

swu.$$availableSubj.next({available: {hash: 'foo'}});
swu.$$versionUpdatesSubj.next({latestVersion: {hash: 'foo'}, type: 'VERSION_READY'} as VersionReadyEvent);

tick(checkInterval);
expect(swu.checkForUpdate).toHaveBeenCalledTimes(2);
Expand All @@ -111,19 +111,16 @@ describe('SwUpdatesService', () => {
discardPeriodicTasks();
})));

it('should request a full page navigation when an update has been activated', run(() => {
swu.$$availableSubj.next({available: {hash: 'foo'}});
expect(location.fullPageNavigationNeeded).toHaveBeenCalledTimes(0);

swu.$$activatedSubj.next({current: {hash: 'bar'}});
it('should request a full page navigation when an update has been activated', fakeAsync(run(() => {
swu.$$versionUpdatesSubj.next({latestVersion: {hash: 'foo'}, type: 'VERSION_READY'} as VersionReadyEvent);
tick();
expect(location.fullPageNavigationNeeded).toHaveBeenCalledTimes(1);

swu.$$availableSubj.next({available: {hash: 'baz'}});
expect(location.fullPageNavigationNeeded).toHaveBeenCalledTimes(1);

swu.$$activatedSubj.next({current: {hash: 'qux'}});
swu.$$versionUpdatesSubj.next({latestVersion: {hash: 'foo'}, type: 'VERSION_READY'} as VersionReadyEvent);
tick();
expect(location.fullPageNavigationNeeded).toHaveBeenCalledTimes(2);
}));
})));

it('should request a page reload when an unrecoverable state has been detected', run(() => {
expect(location.reloadPage).toHaveBeenCalledTimes(0);
Expand Down Expand Up @@ -162,8 +159,9 @@ describe('SwUpdatesService', () => {
tick(checkInterval);
tick(checkInterval);

swu.$$availableSubj.next({available: {hash: 'foo'}});
swu.$$activatedSubj.next({current: {hash: 'bar'}});
swu.$$versionUpdatesSubj.next({
latestVersion: {hash: 'foo'}, currentVersion: {hash: 'bar'} , type: 'VERSION_READY'
});

tick(checkInterval);
tick(checkInterval);
Expand All @@ -172,15 +170,18 @@ describe('SwUpdatesService', () => {
})));

it('should not activate available updates', fakeAsync(runDeactivated(() => {
swu.$$availableSubj.next({available: {hash: 'foo'}});
swu.$$versionUpdatesSubj.next({latestVersion: {hash: 'foo'}, type: 'VERSION_READY'} as VersionReadyEvent);

expect(swu.activateUpdate).not.toHaveBeenCalled();
})));

it('should never request a full page navigation', runDeactivated(() => {
swu.$$availableSubj.next({available: {hash: 'foo'}});
swu.$$activatedSubj.next({current: {hash: 'bar'}});
swu.$$availableSubj.next({available: {hash: 'baz'}});
swu.$$activatedSubj.next({current: {hash: 'qux'}});
swu.$$versionUpdatesSubj.next({
latestVersion: {hash: 'foo'}, currentVersion: {hash: 'bar'} , type: 'VERSION_READY'
});
swu.$$versionUpdatesSubj.next({
latestVersion: {hash: 'baz'}, currentVersion: {hash: 'qux'} , type: 'VERSION_READY'
});

expect(location.fullPageNavigationNeeded).not.toHaveBeenCalled();
}));
Expand Down Expand Up @@ -215,8 +216,9 @@ describe('SwUpdatesService', () => {
service.disable();
swu.checkForUpdate.calls.reset();

swu.$$availableSubj.next({available: {hash: 'foo'}});
swu.$$activatedSubj.next({current: {hash: 'baz'}});
swu.$$versionUpdatesSubj.next({
latestVersion: {hash: 'foo'}, currentVersion: {hash: 'baz'} , type: 'VERSION_READY'
});

tick(checkInterval);
tick(checkInterval);
Expand All @@ -226,23 +228,26 @@ describe('SwUpdatesService', () => {

it('should not activate available updates', fakeAsync(run(() => {
service.disable();
swu.$$availableSubj.next({available: {hash: 'foo'}});
swu.$$versionUpdatesSubj.next({latestVersion: {hash: 'foo'}} as VersionReadyEvent);

expect(swu.activateUpdate).not.toHaveBeenCalled();
})));

it('should stop requesting full page navigations when updates are activated', run(() => {
swu.$$availableSubj.next({available: {hash: 'foo'}});
swu.$$activatedSubj.next({current: {hash: 'bar'}});
it('should stop requesting full page navigations when updates are activated', fakeAsync(run(() => {
swu.$$versionUpdatesSubj.next({
latestVersion: {hash: 'foo'}, currentVersion: {hash: 'bar'}, type: 'VERSION_READY'
});
tick();
expect(location.fullPageNavigationNeeded).toHaveBeenCalledTimes(1);

service.disable();
location.fullPageNavigationNeeded.calls.reset();

swu.$$availableSubj.next({available: {hash: 'baz'}});
swu.$$activatedSubj.next({current: {hash: 'qux'}});
swu.$$versionUpdatesSubj.next({
latestVersion: {hash: 'baz'}, currentVersion: {hash: 'qux'}, type: 'VERSION_READY'
});
expect(location.fullPageNavigationNeeded).not.toHaveBeenCalled();
}));
})));

it('should stop requesting page reloads when unrecoverable states are detected', run(() => {
swu.$$unrecoverableSubj.next({reason: 'Something bad happened'});
Expand Down Expand Up @@ -276,8 +281,6 @@ describe('SwUpdatesService', () => {
service.enable();
swu.checkForUpdate.calls.reset();

swu.$$availableSubj.next({available: {hash: 'foo'}});
swu.$$activatedSubj.next({current: {hash: 'baz'}});
tick(checkInterval);

expect(swu.checkForUpdate).toHaveBeenCalled();
Expand All @@ -287,7 +290,9 @@ describe('SwUpdatesService', () => {
service.disable();
service.enable();

swu.$$availableSubj.next({available: {hash: 'foo'}});
swu.$$versionUpdatesSubj.next({
latestVersion: {hash: 'foo'}, type: 'VERSION_READY'
} as VersionReadyEvent);

expect(swu.activateUpdate).toHaveBeenCalled();
})));
Expand All @@ -311,16 +316,14 @@ class MockApplicationRef {
}

class MockSwUpdate {
$$availableSubj = new Subject<{available: {hash: string}}>();
$$activatedSubj = new Subject<{current: {hash: string}}>();
$$unrecoverableSubj = new Subject<{reason: string}>();
$$versionUpdatesSubj = new Subject<VersionEvent>();

available = this.$$availableSubj.asObservable();
activated = this.$$activatedSubj.asObservable();
unrecoverable = this.$$unrecoverableSubj.asObservable();
versionUpdates =this.$$versionUpdatesSubj.asObservable();

activateUpdate = jasmine.createSpy('MockSwUpdate.activateUpdate')
.and.callFake(() => Promise.resolve());
.and.callFake(() => Promise.resolve(true));

checkForUpdate = jasmine.createSpy('MockSwUpdate.checkForUpdate')
.and.callFake(() => Promise.resolve());
Expand Down
25 changes: 12 additions & 13 deletions aio/src/app/sw-updates/sw-updates.service.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { ApplicationRef, ErrorHandler, Injectable, OnDestroy } from '@angular/core';
import { SwUpdate } from '@angular/service-worker';
import { concat, interval, Subject } from 'rxjs';
import { first, takeUntil, tap } from 'rxjs/operators';
import { SwUpdate, VersionReadyEvent } from '@angular/service-worker';
import { concat, from, interval, Subject } from 'rxjs';
import { filter, first, switchMap, takeUntil, tap } from 'rxjs/operators';

import { LocationService } from 'app/shared/location.service';
import { Logger } from 'app/shared/logger.service';
Expand Down Expand Up @@ -43,20 +43,19 @@ export class SwUpdatesService implements OnDestroy {
.subscribe(() => this.swu.checkForUpdate());

// Activate available updates.
this.swu.available
this.swu.versionUpdates
.pipe(
filter((evt): evt is VersionReadyEvent => evt.type === 'VERSION_READY'),
tap(evt => this.log(`Update available: ${JSON.stringify(evt)}`)),
takeUntil(this.onDisable),
switchMap(() => from(this.swu.activateUpdate()))
)
.subscribe(() => this.swu.activateUpdate());

// Request a full page navigation once an update has been activated.
this.swu.activated
.pipe(
tap(evt => this.log(`Update activated: ${JSON.stringify(evt)}`)),
takeUntil(this.onDisable),
)
.subscribe(() => this.location.fullPageNavigationNeeded());
.subscribe((isActivated) => {
if(isActivated) {
this.log('Update activated');
this.location.fullPageNavigationNeeded();
}
});

// Request an immediate page reload once an unrecoverable state has been detected.
this.swu.unrecoverable
Expand Down

0 comments on commit 288f2c8

Please sign in to comment.