Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(core): markDirty() should only mark flags when really scheduling tick #39316

Closed

Commits on Oct 20, 2020

  1. fix(core): markDirty() should only mark flags when really scheduling …

    …tick.
    
    Close angular#39296
    
    Fix an issue that `markDirty()` will not trigger change detection.
    
    The case is for example we have the following component.
    
    ```
    export class AppComponent implements OnInit {
      constructor(private router: Router) {}
    
      ngOnInit() {
        this.router.events
          .pipe(filter((e) => e instanceof NavigationEnd))
          .subscribe(() => ɵmarkDirty(this));
      }
    }
    
    export class CounterComponent implements OnInit, OnDestroy {
      ngOnInit() {
        this.countSubject.pipe(takeUntil(this.destroy)).subscribe((count) => {
          this.count = count;
          ɵmarkDirty(this);
        });
      }
    ```
    
    Then the app navigate from `AppComponent` to `CounterComponent`,
    so there are 2 `markDirty()` call at in a row.
    
    The `1st` call is from `AppComponent` when router changed, the
    `2nd` call is from `CounterComponent.ngOnInit()`.
    
    And the `markDirty()->scheduleTick()` code look like this
    
    ```
    function scheduleTick(rootContext, flags) {
        const nothingScheduled = rootContext.flags === 0 /* Empty */;
        rootContext.flags |= flags;
        if (nothingScheduled && rootContext.clean == _CLEAN_PROMISE) {
          rootContext.schedule(() => {
    	...
            if (rootContext.flags & RootContextFlags.DetectChanges)
              rootContext.flags &= ~RootContextFlags.DetectChanges;
              tickContext();
    
            rootContext.clean = _CLEAN_PROMISE;
            ...
          });
    ```
    
    So in this case, the `1st` markDirty() will
    1. set rootContext.flags = 1
    2. before `tickContext()`, reset rootContext.flags = 0
    3. inside `tickContext()`, it will call `CounterComponent.ngOnint()`,
       so the `2nd` markDirty() is called.
    4. and the `2nd` scheduleTick is called, `nothingScheduled` is true,
       but rootContext.clean is not `_CLEAN_PROMISE` yet, since the `1st` markDirty tick
       is still running.
    5. So nowhere will reset the `rootContext.flags`.
    6. then in the future, any other `markDirty()` call will not trigger the tick, since
       `nothingScheduled` is always false.
    
    So `nothingScheduled` means no tick is scheduled, `rootContext.clean === _CLEAN_PROMISE`
    means no tick is running.
    So we should set the flags to `rootContext` only when `no tick is scheudled or running`.
    JiaLiPassion committed Oct 20, 2020
    Configuration menu
    Copy the full SHA
    1a82495 View commit details
    Browse the repository at this point in the history