diff --git a/adev/shared-docs/components/table-of-contents/table-of-contents.component.ts b/adev/shared-docs/components/table-of-contents/table-of-contents.component.ts index e2ef35e83f8..f9e36039c36 100644 --- a/adev/shared-docs/components/table-of-contents/table-of-contents.component.ts +++ b/adev/shared-docs/components/table-of-contents/table-of-contents.component.ts @@ -6,7 +6,14 @@ * found in the LICENSE file at https://angular.dev/license */ -import {ChangeDetectionStrategy, Component, Input, computed, inject} from '@angular/core'; +import { + ChangeDetectionStrategy, + Component, + DestroyRef, + Input, + computed, + inject, +} from '@angular/core'; import {RouterLink} from '@angular/router'; import {TableOfContentsLevel} from '../../interfaces/index'; import {TableOfContentsLoader} from '../../services/table-of-contents-loader.service'; @@ -27,6 +34,8 @@ export class TableOfContents { private readonly scrollSpy = inject(TableOfContentsScrollSpy); private readonly tableOfContentsLoader = inject(TableOfContentsLoader); + private readonly destroyRef = inject(DestroyRef); + tableOfContentItems = this.tableOfContentsLoader.tableOfContentItems; activeItemId = this.scrollSpy.activeItemId; @@ -35,7 +44,7 @@ export class TableOfContents { ngAfterViewInit() { this.tableOfContentsLoader.buildTableOfContent(this.contentSourceElement); - this.scrollSpy.startListeningToScroll(this.contentSourceElement); + this.scrollSpy.startListeningToScroll(this.contentSourceElement, this.destroyRef); } scrollToTop(): void { diff --git a/adev/shared-docs/components/viewers/docs-viewer/docs-viewer.component.ts b/adev/shared-docs/components/viewers/docs-viewer/docs-viewer.component.ts index 4b88f650114..e99843e64d5 100644 --- a/adev/shared-docs/components/viewers/docs-viewer/docs-viewer.component.ts +++ b/adev/shared-docs/components/viewers/docs-viewer/docs-viewer.component.ts @@ -75,7 +75,6 @@ export class DocViewer implements OnChanges { private readonly elementRef = inject(ElementRef); private readonly location = inject(Location); private readonly navigationState = inject(NavigationState); - private readonly platformId = inject(PLATFORM_ID); private readonly router = inject(Router); private readonly viewContainer = inject(ViewContainerRef); private readonly environmentInjector = inject(EnvironmentInjector); @@ -86,6 +85,8 @@ export class DocViewer implements OnChanges { private animateContent = false; private readonly pendingTasks = inject(PendingTasks); + private readonly isBrowser = isPlatformBrowser(inject(PLATFORM_ID)); + private countOfExamples = 0; async ngOnChanges(changes: SimpleChanges): Promise { @@ -97,11 +98,10 @@ export class DocViewer implements OnChanges { } async renderContentsAndRunClientSetup(content?: string): Promise { - const isBrowser = isPlatformBrowser(this.platformId); const contentContainer = this.elementRef.nativeElement; if (content) { - if (isBrowser && !(this.document as any).startViewTransition) { + if (this.isBrowser && !(this.document as any).startViewTransition) { // Apply a special class to the host node to trigger animation. // Note: when a page is hydrated, the `content` would be empty, // so we don't trigger an animation to avoid a content flickering @@ -112,7 +112,7 @@ export class DocViewer implements OnChanges { contentContainer.innerHTML = content; } - if (isBrowser) { + if (this.isBrowser) { // First we setup event listeners on the HTML we just loaded. // We want to do this before things like the example viewers are loaded. this.setupAnchorListeners(contentContainer); @@ -301,6 +301,14 @@ export class DocViewer implements OnChanges { // during SSG. this.appRef.attachView(componentRef.hostView); + // This is wrapped with `isBrowser` in for hydration purposes. + if (this.isBrowser) { + // The `docs-viewer` may be rendered multiple times when navigating + // between pages, which will create new components that need to be + // destroyed for gradual cleanup. + this.destroyRef.onDestroy(() => componentRef.destroy()); + } + return componentRef; } diff --git a/adev/shared-docs/services/table-of-contents-scroll-spy.service.spec.ts b/adev/shared-docs/services/table-of-contents-scroll-spy.service.spec.ts index d4ec27cb0de..6e045a20c1f 100644 --- a/adev/shared-docs/services/table-of-contents-scroll-spy.service.spec.ts +++ b/adev/shared-docs/services/table-of-contents-scroll-spy.service.spec.ts @@ -6,17 +6,19 @@ * found in the LICENSE file at https://angular.dev/license */ +import {DestroyRef} from '@angular/core'; +import {DOCUMENT, ViewportScroller} from '@angular/common'; import {TestBed, discardPeriodicTasks, fakeAsync, tick} from '@angular/core/testing'; import {WINDOW} from '../providers'; import {TableOfContentsLoader} from './table-of-contents-loader.service'; import {SCROLL_EVENT_DELAY, TableOfContentsScrollSpy} from './table-of-contents-scroll-spy.service'; -import {DOCUMENT, ViewportScroller} from '@angular/common'; import {TableOfContentsLevel} from '../interfaces'; describe('TableOfContentsScrollSpy', () => { let service: TableOfContentsScrollSpy; + let destroyRef: DestroyRef; const fakeWindow = { addEventListener: () => {}, removeEventListener: () => {}, @@ -68,6 +70,7 @@ describe('TableOfContentsScrollSpy', () => { const tableOfContentsLoaderSpy = TestBed.inject(TableOfContentsLoader); tableOfContentsLoaderSpy.tableOfContentItems.set(fakeToCItems); service = TestBed.inject(TableOfContentsScrollSpy); + destroyRef = TestBed.inject(DestroyRef); }); it('should be created', () => { @@ -94,7 +97,7 @@ describe('TableOfContentsScrollSpy', () => { const scrollableContainer = doc; const setActiveItemIdSpy = spyOn(service as any, 'setActiveItemId'); - service.startListeningToScroll(doc.querySelector('fake-selector')); + service.startListeningToScroll(doc.querySelector('fake-selector'), destroyRef); scrollableContainer.dispatchEvent(new Event('scroll')); tick(SCROLL_EVENT_DELAY - 2); @@ -118,7 +121,7 @@ describe('TableOfContentsScrollSpy', () => { const doc = TestBed.inject(DOCUMENT); const scrollableContainer = doc; - service.startListeningToScroll(doc.querySelector('fake-selector')); + service.startListeningToScroll(doc.querySelector('fake-selector'), destroyRef); fakeWindow.scrollY = 1238; scrollableContainer.dispatchEvent(new Event('scroll')); @@ -133,7 +136,7 @@ describe('TableOfContentsScrollSpy', () => { const doc = TestBed.inject(DOCUMENT); const scrollableContainer = doc; - service.startListeningToScroll(doc.querySelector('fake-selector')); + service.startListeningToScroll(doc.querySelector('fake-selector'), destroyRef); fakeWindow.scrollY = 99; scrollableContainer.dispatchEvent(new Event('scroll')); diff --git a/adev/shared-docs/services/table-of-contents-scroll-spy.service.ts b/adev/shared-docs/services/table-of-contents-scroll-spy.service.ts index 17f66d82ab1..040d0d7ee12 100644 --- a/adev/shared-docs/services/table-of-contents-scroll-spy.service.ts +++ b/adev/shared-docs/services/table-of-contents-scroll-spy.service.ts @@ -29,7 +29,6 @@ export const SCROLL_FINISH_DELAY = SCROLL_EVENT_DELAY * 2; // The service is responsible for listening for scrolling and resizing, // thanks to which it sets the active item in the Table of contents export class TableOfContentsScrollSpy { - private readonly destroyRef = inject(DestroyRef); private readonly tableOfContentsLoader = inject(TableOfContentsLoader); private readonly document = inject(DOCUMENT); private readonly window = inject(WINDOW); @@ -41,12 +40,25 @@ export class TableOfContentsScrollSpy { activeItemId = signal(null); scrollbarThumbOnTop = signal(true); - startListeningToScroll(contentSourceElement: HTMLElement | null): void { + startListeningToScroll( + contentSourceElement: HTMLElement | null, + // `destroyRef` is required because the caller may invoke `startListeningToScroll` + // multiple times. Without it, previous event listeners would not be disposed of, + // leading to the accumulation of new event listeners. + destroyRef: DestroyRef, + ) { this.contentSourceElement = contentSourceElement; this.lastContentWidth = this.getContentWidth(); - this.setScrollEventHandlers(); - this.setResizeEventHandlers(); + this.setScrollEventHandlers(destroyRef); + this.setResizeEventHandlers(destroyRef); + + destroyRef.onDestroy(() => { + // We also need to clean up the source element once the view that calls + // `startListeningToScroll` is destroyed, as this will keep a reference + // to an element that has been removed from the DOM. + this.contentSourceElement = null; + }); } scrollToTop(): void { @@ -72,9 +84,9 @@ export class TableOfContentsScrollSpy { } // After window resize, we should update top value of each table content item - private setResizeEventHandlers() { + private setResizeEventHandlers(destroyRef: DestroyRef) { fromEvent(this.window, 'resize') - .pipe(debounceTime(RESIZE_EVENT_DELAY), takeUntilDestroyed(this.destroyRef), startWith()) + .pipe(debounceTime(RESIZE_EVENT_DELAY), takeUntilDestroyed(destroyRef), startWith()) .subscribe(() => { this.updateHeadingsTopAfterResize(); }); @@ -83,14 +95,16 @@ export class TableOfContentsScrollSpy { // assets (fonts, images) are loaded. They can (and will) change the y-position of the headings. const docsViewer = this.document.querySelector('docs-viewer'); if (docsViewer) { - afterNextRender( + const ref = afterNextRender( () => { const resizeObserver = new ResizeObserver(() => this.updateHeadingsTopAfterResize()); resizeObserver.observe(docsViewer); - this.destroyRef.onDestroy(() => resizeObserver.disconnect()); + destroyRef.onDestroy(() => resizeObserver.disconnect()); }, - {injector: this.injector}, + {injector: this.injector, manualCleanup: true}, ); + + destroyRef.onDestroy(() => ref.destroy()); } } @@ -104,10 +118,10 @@ export class TableOfContentsScrollSpy { } } - private setScrollEventHandlers(): void { + private setScrollEventHandlers(destroyRef: DestroyRef): void { const scroll$ = fromEvent(this.document, 'scroll').pipe( auditTime(SCROLL_EVENT_DELAY), - takeUntilDestroyed(this.destroyRef), + takeUntilDestroyed(destroyRef), ); scroll$.subscribe(() => this.setActiveItemId());