4

I'm rewriting an old directive I have to use signals instead of bloated RxJS but I'm facing an issue which I don't fully comprehend. This is the directive, which is a directly which listens for media query changes:

@Directive({
  standalone: true,
})
export class SDKMediaQueriesMixin extends SDKSubscriptionsManager implements OnInit {

  private readonly breakpointSignal = signal<Nullable<SDKBreakpoint>>(null);

  public readonly breakpoints: LibBreakpointMap<string> = {};
  public readonly breakpointNames: LibBreakpointName[] = [];

  private matches$: Observable<(MediaQueryListEvent | Partial<MediaQueryListEvent>)[]> = EMPTY;

  constructor(@Inject(DOCUMENT) private document: Document, private applicationManager: SDKApplicationManager) {
    super();

    if (this.applicationManager.isBrowser) {
      this.breakpoints.mobile = getComputedStyle(this.document.documentElement).getPropertyValue('--mobile');
      this.breakpoints.tablet = getComputedStyle(this.document.documentElement).getPropertyValue('--tablet');
      this.breakpoints.desktop = getComputedStyle(this.document.documentElement).getPropertyValue('--desktop');
      this.breakpoints.fullhd = getComputedStyle(this.document.documentElement).getPropertyValue('--fullhd');

      this.breakpointNames = Object.keys(this.breakpoints) as LibBreakpointName[];

      const bpValues = Object.values(this.breakpoints);
      const queries = bpValues.map((bp) => `(min-width: ${bp})`);

      const matches$ = queries.map((query) => {
        const matchMedia = window.matchMedia(query);

        return fromEventPattern<Partial<MediaQueryListEvent>>(
          matchMedia.addListener.bind(matchMedia),
          matchMedia.removeListener.bind(matchMedia)
        ).pipe(startWith({ matches: matchMedia.matches, media: matchMedia.media }));
      });

      this.matches$ = combineLatest(matches$);
    }
  }

  ngOnInit() {

    if (this.applicationManager.isBrowser && this.matches$) {
      this.newSubscription = this.matches$.pipe(
        map((events) => events.map((event) => event.matches)),
        map((matches) => new SDKBreakpoint(this.breakpointNames[matches.lastIndexOf(true)]))
      )
      .subscribe((breakpoint) => {
        console.log('breakpoint is changing', breakpoint);
        this.breakpointSignal.set(breakpoint);
      });
    }
  }

  public get activeBreakpoint() {
    return this.breakpointSignal();
  };
}

In my AppComponent I want to listen to changes to the active breakpoint, so I added an effect:

@Component({
  selector: 'my-app',
  templateUrl: './app.component.html',
  styleUrls: ['./app.component.scss'],
  standalone: true,
  imports: [RouterModule, LibAnchorComponent],
  hostDirectives: [SDKMediaQueriesMixin]
})
export class AppComponent {

  constructor(private mediaQueriesMixin: SDKMediaQueriesMixin) {

    effect(() => {
      console.log('breakpoint changed', this.mediaQueriesMixin.activeBreakpoint);
    });
  }
}

This runs twice on load to log the following:

app.component.ts:21 breakpoint changed null
media-queries.mixin.ts:67 breakpoint is changing SDKBreakpoint {name: 'fullhd'}

When I change the size of the browser window it logs the following:

media-queries.mixin.ts:67 breakpoint is changing SDKBreakpoint {name: 'desktop'}
media-queries.mixin.ts:67 breakpoint is changing SDKBreakpoint {name: 'tablet'}
media-queries.mixin.ts:67 breakpoint is changing SDKBreakpoint {name: 'fullhd'}

However as you can see I do not get the logs in the effect to run. The funny thing is though, if I add an interval instead which just randomises a breakpoint then everything works as expected:

  interval(1000).pipe(
    map(() => new SDKBreakpoint(this.breakpointNames[Math.floor(Math.random() * this.breakpointNames.length)])),
  )
  .subscribe((breakpoint) => {
    console.log('breakpoint is changing', breakpoint);
    this.breakpointSignal.set(breakpoint);
  });

With this code, I get the following logs:

media-queries.mixin.ts:67 breakpoint is changing SDKBreakpoint {name: 'mobile'}
app.component.ts:21 breakpoint changed SDKBreakpoint {name: 'mobile'}
media-queries.mixin.ts:67 breakpoint is changing SDKBreakpoint {name: 'fullhd'}
app.component.ts:21 breakpoint changed SDKBreakpoint {name: 'fullhd'}
media-queries.mixin.ts:67 breakpoint is changing SDKBreakpoint {name: 'tablet'}
app.component.ts:21

Since this.breakpointSignal.set(breakpoint); runs the same in both scenarios I don't understand why it only works with an interval... What am I doing wrong?

Chrillewoodz
  • 27,055
  • 21
  • 92
  • 175
  • Angular docs say "Effects always execute asynchronously, during the change detection process." Maybe interval triggers change detection and matchMedia listener does not? What happens if you read the activeBreakpoint value in the AppComponent template? – kvetis Aug 23 '23 at 12:52
  • @kvetis Not sure, I added `

    {{this.mediaQueriesMixin.activeBreakpoint?.name}}

    ` to the template but it remains unchanged and no additional logs either. Same if I call the signal getter directly with `

    {{this.mediaQueriesMixin.breakpointSignal()?.name}}

    `.
    – Chrillewoodz Aug 23 '23 at 12:56
  • 1
    @Chrillewoodz Please check if your Angular version is 16.2.1 - effect() scheduling was changed in 16.2. – OZ_ Aug 23 '23 at 13:01
  • @OZ_ I'm on 16.2.1 yes. – Chrillewoodz Aug 23 '23 at 13:05
  • 1
    @OZ_ If you're talking about this PR : https://github.com/angular/angular/pull/51049 it hasn't landed – Matthieu Riegler Aug 23 '23 at 13:30

2 Answers2

3

As this PR says: "Previously effects were queued as they became dirty, and this queue was flushed at various checkpoints during the change detection cycle. The result was that change detection was the effect runner, and without executing CD, effects would not execute."

I've created a reproduction for you: https://stackblitz.com/edit/stackblitz-starters-aaf1qy?devToolsHeight=33&file=src%2Fmain.ts

Before the PR is landed, you'll need to trigger CD (line 62: this.cdr.detectChanges();)

After, effects will be scheduled via the microtask queue.

OZ_
  • 12,492
  • 7
  • 50
  • 68
2

While I totally agree with OZ's anwser, which look valid to me; I can bring another one !

Talking about a similar issue, Alex Rickabaugh (One of the mastermind behind the Angular's signal implementation) stated in a tweet :

Angular today assumes that data flow is unidirectional and top-down, and that such a backwards propagation is an error rather than a signal that change detection needs to run again. Thus the usual setTimeout hack is the "right answer" here.

Matthieu Riegler
  • 31,918
  • 20
  • 95
  • 134