Angular Routing Bug - eventCoalescing true cause scroll to top

[keatkeat87]

Which @angular/* package(s) are the source of the bug?

router

Is this a regression?

No

Description

git clone https://github.com/keatkeat87/ng-routing-zone-v18.git

yarn install ng serve

when navigate, it will scroll to the top.

change eventCoalescing to false

export const appConfig: ApplicationConfig = {
  providers: [provideZoneChangeDetection({ eventCoalescing: false }), provideRouter(routes)]
};

test again

no scroll to the top anymore.

change to zoneless

export const appConfig: ApplicationConfig = {
  providers: [provideExperimentalZonelessChangeDetection(), provideRouter(routes)]
};

test again

scroll to the top again.

Problem: In version 18, provideZoneChangeDetection and provideExperimentalZonelessChangeDetection were introduced. This means that in version 17 and earlier the behavior is like eventCoalescing to false -- won't scroll to the top". so I will say "no scroll to the top" is the correct behavior.

Starting from version 18, Angular sets eventCoalescing to true by default, and zoneless mode is expected to become the default in the future. This change in behavior could cause problems. If the Angular Team does not consider this a bug, it should be clearly highlighted in the documentation.

Why it happened? (I guess) After insertView, ChangeDetectionScheduler.notify is triggered, but refreshView does not run immediately. instead, it runs after a setTimeout or requestAnimationFrame. In the example above, the template contains an @for. Due to the delay, the content is removed first, and then the browser renders. At this point, the @for hasn't executed yet, so there are no elements in the body, causing scrollTop jump to 0. After the timeout, the @for runs, but scrollTop can no longer automatically return to its previous position.

Please provide a link to a minimal reproduction of the bug

https://github.com/keatkeat87/ng-routing-zone-v18.git

Please provide the exception or error you saw

No response

Please provide the environment you discovered this bug in (run ng version)

Angular CLI: 18.2.0
Node: 20.11.1
Package Manager: yarn 1.22.19
OS: win32 x64

Angular: 18.2.0
... animations, cli, common, compiler, compiler-cli, core, forms
... platform-browser, platform-browser-dynamic, router

Package                         Version
---------------------------------------------------------
@angular-devkit/architect       0.1802.0
@angular-devkit/build-angular   18.2.0
@angular-devkit/core            18.2.0
@angular-devkit/schematics      18.2.0
@schematics/angular             18.2.0
rxjs                            7.8.1
typescript                      5.5.4
zone.js                         0.14.10

Anything else?

No response

[atscott]

so I will say "no scroll to the top" is the correct behavior.

There really is no “correct behavior” with respect to scrolling on navigation, but is more a coincidence of timing. In fact, the scrolling to the top with coalescing would be considered more in line with the behavior of MPAs and typically this is the behavior that SPAs try to emulate (and generally considered “correct” because this follows browser specs). With that, the behavior with coalescing enabled should be considered correct here while the behavior without is the one with the bug.

[keatkeat87]

well, expected response. fine, follow your theory.

change the @for to below

<!-- @for (color of ['lightyellow', 'lightblue', 'lightgreen']; track color) {
  <section [style.--bg-color]="color" >{{ page() }} section {{ $index + 1 }}</section>
} -->

<section [style.--bg-color]="'lightyellow'" >{{ page() }} section {{ 0 + 1 }}</section>
<section [style.--bg-color]="'lightblue'" >{{ page() }} section {{ 1 + 1 }}</section>
<section [style.--bg-color]="'lightgreen'" >{{ page() }} section {{ 2 + 1 }}</section>

eventCoalescing: true

test again

no scroll to the top. Whether the route scrolls to the top depends on whether @for is used. Do you really think this is how Angular should work?

If your wants to emulate MPAs, you should change the scrollTop by RouterScroller, not eventCoalescing: true.

The key points: It's not just about router scrolling—every dynamic component is experiencing the same issue. Before Angular v18, when using zoneless mode and ViewContainerRef.insert, it wouldn't automatically call refreshView. We would typically call detectChanges or tick ourselves without setTimeout, ensuring the browser rendered everything at the same time. However, starting with v18, insertView automatically triggers refreshView, but it now uses setTimeout, causing the browser to render twice. The routing scroll-to-top issue is one of the side effects of this new behavior.

[atscott]

If your wants to emulate MPAs, you should change the scrollTop by RouterScroller, not eventCoalescing: true

Yes, there is absolutely an argument for enabling or recommending the router’s scrolling behaviors by default.

Do you really think this is how Angular should work?

I’m actually a bit unclear about what you mean by “this” since the issue was initially about scrolling with the router that does not match MPA or spec behavior. I originally attempted to address the report about router scrolling, behavior that isn’t necessarily historically correct.

Before Angular v18, when using zoneless mode and ViewContainerRef.insert, it wouldn't automatically call refreshView. We would typically call detectChanges or tick ourselves

You absolutely can still configure manual change detection without zones this way. The new behavior in v18 which recognizes additional signals as requirements for running change detection does not supersede manual change detection.

uses setTimeout, causing the browser to render twice

The scheduling actually races requestAnimationFrame with setTimeout. Unless you’re already in an animation frame, this is not two browser paints.

There are absolutely tradeoffs between scheduling change detection with a microtask (no event coalescing) versus others. We are interested in gathering more information about these situations where the tradeoffs apply, but it’s difficult for me to follow what those are in this report.

Scrolling behavior is its own issue, and neither the described behavior with event coalescing nor the behavior without it are correct. They do differ and they’re an exact reflection of exiting the event loop or not between the user event and the application tick.

[keatkeat87]

Thank you for the reply, I try to provide more details.

look into my repo example.

Condition 1:

1. eventCoalescing: true (by default)
2. scrollPositionRestoration: 'disabled' (by default)
3. @for used

the behavior is "auto scroll to top", and we don't have any "Way" to prevent this behavior (unless we set eventCoalescing false)

Condition 2:

1. eventCoalescing: true (by default)
2. scrollPositionRestoration: 'disabled' (by default)
3. without using @for

the behavior is "no scroll to top", do you think the @for should affect the routing scroll behavior? If you also feel it is inappropriate, then we continue.

How it happened? I create another 2 repos to highlight this.

repo1 - Without Routing

app.component.html

<ng-template #template let-page>
  @for (color of ['lightyellow', 'lightblue', 'lightgreen']; track color) {
    <section [style.--bg-color]="color" >{{ page }} section {{ $index + 1 }}</section>
  }
</ng-template>
<ng-container #container></ng-container>

<div class="navigation">
  <button (click)="navigate('Home')" >navigate to home</button>
  <button (click)="navigate('About')" >navigate to about</button>
</div>

app.component.ts

import { Component, inject, Injector, TemplateRef, viewChild, ViewContainerRef } from '@angular/core';

@Component({
  selector: 'app-root',
  standalone: true,
  imports: [],
  templateUrl: './app.component.html',
  styleUrl: './app.component.scss'
})
export class AppComponent {

  private readonly templateRef = viewChild.required('template', { read: TemplateRef });
  private readonly viewContainerRef = viewChild.required('container', { read: ViewContainerRef });

  private readonly injector = inject(Injector);
  navigate(page: string) {
    this.viewContainerRef().length !== 0 && this.viewContainerRef().clear();
    this.viewContainerRef().createEmbeddedView(this.templateRef(), { $implicit: page });
    // this.injector.get(ApplicationRef).tick();
  }
}

without ApplicationRef.tick(), it will "auto scroll to top", if we run ApplicationRef.tick() after createEmbeddedView, it won't "auto scroll to top", so the key point is the refreshView timing.

repo2 - Pure HTML, JS

index.html

<body>

  <header>
    <button>click me</button>
  </header>
  <main>
    <section>A</section>
    <section>B</section>
    <section>C</section>
  </main>
  <template>
    <section>A1</section>
    <section>B1</section>
    <section>C1</section>
  </template>
  
  <script src="script.js"></script>
</body>

script.js

const button = document.querySelector('header button');
button.addEventListener('click', () => {
  const main = document.querySelector('main');
  const sectionsToRemove = Array.from(main.querySelectorAll('section'));
  sectionsToRemove.forEach(section => section.remove());

  const template = document.querySelector('template');
  const view = template.content.cloneNode(true);

  // 1. append immediately won't cause scroll to top
  // main.append(view); 

  // 2. delay append will cause scroll to top
  // requestAnimationFrame(() => {
  //   main.append(view); 
  // });

  // 3. delay append will cause scroll to top
  // setTimeout(() => {
  //   main.append(view); 
  // });

 });

section.remove() is similar to ViewContainerRef.createEmbededView, and then after requestAnimationFrame or setTimeout, main.append(view) is similar to @for. in Chrome, it will cause an auto scroll to top, it looks like the browser reflows/repaints twice.

[atscott]

Closing in favor of canonical issue tracking consequences of macrotask-based scheduling (#57528)

[angular-automatic-lock-bot[bot]]

This issue has been automatically locked due to inactivity. Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}