Skip to content

Commit 785b068

Browse files
committed
refactor(cdk/overlay): use signal for attached overlays (#33882)
Reworks `getAttachedOverlays` to use a signal instead of a getter function. (cherry picked from commit 198f43d)
1 parent a7a31a6 commit 785b068

2 files changed

Lines changed: 161 additions & 24 deletions

File tree

‎src/cdk/overlay/overlay-ref.ts‎

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ import {
1515
NgZone,
1616
Renderer2,
1717
afterNextRender,
18+
signal,
19+
untracked,
1820
} from '@angular/core';
1921
import {Observable, Subject, Subscription, SubscriptionLike} from 'rxjs';
2022
import {Direction, Directionality} from '../bidi';
@@ -37,12 +39,10 @@ export function isElement(value: any): value is Element {
3739
return value && (value as Element).nodeType === 1;
3840
}
3941

40-
const attachedOverlays = new Set<OverlayRef>();
42+
const attachedOverlaysInternal = signal<readonly OverlayRef[]>([]);
4143

42-
/** Gets all overlays that are currently attached. */
43-
export function getAttachedOverlays(): OverlayRef[] {
44-
return Array.from(attachedOverlays);
45-
}
44+
/** Tracks all currently-attached overlays. */
45+
export const attachedOverlays = attachedOverlaysInternal.asReadonly();
4646

4747
/**
4848
* Reference to an overlay that has been created with the Overlay service.
@@ -149,7 +149,11 @@ export class OverlayRef implements PortalOutlet {
149149
this._updateStackingOrder();
150150
this._updateElementSize();
151151
this._updateElementDirection();
152-
attachedOverlays.add(this);
152+
153+
// Needs to be untracked in case an overlay is opened as a part of template rendering.
154+
untracked(() => {
155+
attachedOverlaysInternal.update(prev => (prev.includes(this) ? prev : [...prev, this]));
156+
});
153157

154158
if (this._scrollStrategy) {
155159
this._scrollStrategy.enable();
@@ -256,7 +260,11 @@ export class OverlayRef implements PortalOutlet {
256260
this._detachContentWhenEmpty();
257261
this._locationChanges.unsubscribe();
258262
this._outsideClickDispatcher.remove(this);
259-
attachedOverlays.delete(this);
263+
264+
untracked(() => {
265+
attachedOverlaysInternal.update(prev => prev.filter(o => o !== this));
266+
});
267+
260268
return detachmentResult;
261269
}
262270

@@ -293,7 +301,10 @@ export class OverlayRef implements PortalOutlet {
293301
this._detachments.complete();
294302
this._completeDetachContent();
295303
this._disposed = true;
296-
attachedOverlays.delete(this);
304+
305+
untracked(() => {
306+
attachedOverlaysInternal.update(prev => prev.filter(o => o !== this));
307+
});
297308
}
298309

299310
/** Whether the overlay has attached content. */

‎src/cdk/overlay/overlay.spec.ts‎

Lines changed: 142 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ import {
1313
WritableSignal,
1414
inject,
1515
signal,
16+
computed,
17+
effect,
1618
ChangeDetectionStrategy,
1719
} from '@angular/core';
1820
import {ComponentFixture, TestBed} from '@angular/core/testing';
@@ -30,7 +32,7 @@ import {
3032
ScrollStrategy,
3133
createOverlayRef,
3234
} from './index';
33-
import {getAttachedOverlays} from './overlay-ref';
35+
import {attachedOverlays} from './overlay-ref';
3436

3537
describe('Overlay', () => {
3638
let injector: Injector;
@@ -479,26 +481,150 @@ describe('Overlay', () => {
479481
expect(document.querySelector('.cdk-overlay-pane')).toBeFalsy();
480482
});
481483

482-
it('should track when an overlay is attached and detached', () => {
483-
const overlayRef = createOverlayRef(injector);
484-
expect(getAttachedOverlays()).toEqual([]);
484+
describe('attachedOverlays signal', () => {
485+
it('should track when an overlay is attached and detached', () => {
486+
const overlayRef = createOverlayRef(injector);
487+
expect(attachedOverlays()).toEqual([]);
485488

486-
overlayRef.attach(componentPortal);
487-
expect(getAttachedOverlays()).toEqual([overlayRef]);
489+
overlayRef.attach(componentPortal);
490+
expect(attachedOverlays()).toEqual([overlayRef]);
488491

489-
overlayRef.detach();
490-
expect(getAttachedOverlays()).toEqual([]);
491-
});
492+
overlayRef.detach();
493+
expect(attachedOverlays()).toEqual([]);
494+
});
492495

493-
it('should track when an overlay is attached and disposed', () => {
494-
const overlayRef = createOverlayRef(injector);
495-
expect(getAttachedOverlays()).toEqual([]);
496+
it('should track when an overlay is attached and disposed', () => {
497+
const overlayRef = createOverlayRef(injector);
498+
expect(attachedOverlays()).toEqual([]);
496499

497-
overlayRef.attach(componentPortal);
498-
expect(getAttachedOverlays()).toEqual([overlayRef]);
500+
overlayRef.attach(componentPortal);
501+
expect(attachedOverlays()).toEqual([overlayRef]);
499502

500-
overlayRef.dispose();
501-
expect(getAttachedOverlays()).toEqual([]);
503+
overlayRef.dispose();
504+
expect(attachedOverlays()).toEqual([]);
505+
});
506+
507+
it('should update computed signals derived from the attached overlays', () => {
508+
const count = computed(() => attachedOverlays().length);
509+
const first = createOverlayRef(injector);
510+
const second = createOverlayRef(injector);
511+
expect(count()).toBe(0);
512+
513+
first.attach(componentPortal);
514+
expect(count()).toBe(1);
515+
516+
second.attach(templatePortal);
517+
expect(count()).toBe(2);
518+
519+
first.detach();
520+
expect(count()).toBe(1);
521+
522+
second.dispose();
523+
expect(count()).toBe(0);
524+
});
525+
526+
it('should preserve the attachment order', () => {
527+
const first = createOverlayRef(injector);
528+
const second = createOverlayRef(injector);
529+
530+
first.attach(componentPortal);
531+
second.attach(templatePortal);
532+
expect(attachedOverlays()).toEqual([first, second]);
533+
534+
first.detach();
535+
first.attach(componentPortal);
536+
expect(attachedOverlays()).toEqual([second, first]);
537+
538+
first.dispose();
539+
second.dispose();
540+
});
541+
542+
it('should re-run effects when overlays are attached and detached', () => {
543+
const spy = jasmine.createSpy('effect spy');
544+
const effectRef = effect(() => spy(attachedOverlays()), {injector});
545+
const overlayRef = createOverlayRef(injector);
546+
547+
TestBed.tick();
548+
expect(spy).toHaveBeenCalledTimes(1);
549+
expect(spy).toHaveBeenCalledWith([]);
550+
551+
overlayRef.attach(componentPortal);
552+
TestBed.tick();
553+
expect(spy).toHaveBeenCalledTimes(2);
554+
expect(spy).toHaveBeenCalledWith([overlayRef]);
555+
556+
overlayRef.detach();
557+
TestBed.tick();
558+
expect(spy).toHaveBeenCalledTimes(3);
559+
expect(spy).toHaveBeenCalledWith([]);
560+
561+
overlayRef.attach(componentPortal);
562+
TestBed.tick();
563+
expect(spy).toHaveBeenCalledTimes(4);
564+
expect(spy).toHaveBeenCalledWith([overlayRef]);
565+
566+
overlayRef.dispose();
567+
TestBed.tick();
568+
expect(spy).toHaveBeenCalledTimes(5);
569+
expect(spy).toHaveBeenCalledWith([]);
570+
571+
effectRef.destroy();
572+
});
573+
574+
it('should not throw when attaching and detaching inside a computed', () => {
575+
const overlayRef = createOverlayRef(injector);
576+
const trigger = signal(false);
577+
const attached = computed(() => {
578+
if (trigger()) {
579+
overlayRef.attach(componentPortal);
580+
} else {
581+
overlayRef.detach();
582+
}
583+
return overlayRef.hasAttached();
584+
});
585+
586+
expect(() => attached()).not.toThrow();
587+
expect(attached()).toBe(false);
588+
589+
trigger.set(true);
590+
expect(() => attached()).not.toThrow();
591+
expect(attached()).toBe(true);
592+
expect(attachedOverlays()).toEqual([overlayRef]);
593+
594+
trigger.set(false);
595+
expect(() => attached()).not.toThrow();
596+
expect(attached()).toBe(false);
597+
expect(attachedOverlays()).toEqual([]);
598+
});
599+
600+
it('should not make effects that attach overlays depend on the attached overlays', () => {
601+
const spy = jasmine.createSpy('effect spy');
602+
const overlayRef = createOverlayRef(injector);
603+
const otherOverlayRef = createOverlayRef(injector);
604+
605+
const effectRef = effect(
606+
() => {
607+
spy();
608+
overlayRef.attach(componentPortal);
609+
},
610+
{injector},
611+
);
612+
613+
TestBed.tick();
614+
expect(spy).toHaveBeenCalledTimes(1);
615+
expect(attachedOverlays()).toEqual([overlayRef]);
616+
617+
// Changing the attached overlays shouldn't cause the effect to re-run,
618+
// because the write inside `attach` is untracked.
619+
otherOverlayRef.attach(templatePortal);
620+
TestBed.tick();
621+
otherOverlayRef.dispose();
622+
TestBed.tick();
623+
expect(spy).toHaveBeenCalledTimes(1);
624+
625+
effectRef.destroy();
626+
overlayRef.dispose();
627+
});
502628
});
503629

504630
describe('positioning', () => {

0 commit comments

Comments
 (0)