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

OnPush component in dev mode will only trigger change detection once. #39545

Closed
JiaLiPassion opened this issue Nov 3, 2020 · 2 comments
Closed
Assignees
Labels
area: core Issues related to the framework runtime core: change detection P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent
Milestone

Comments

@JiaLiPassion
Copy link
Contributor

Consider the following case.

      @Component({
        template: `
          <button (click)="noop()">Trigger change detection</button>
          <div>{{increment('componentView')}}</div>
        `,
        changeDetection: ChangeDetectionStrategy.OnPush
      })
      class App {
        increment() {
          counters++;
        }
        noop() {}
      }
      const fixture = TestBed.createComponent(App);
      fixture.detectChanges();
      expect(counters).toEqual(2); // current behavior is 1 

The reason is App is a OnPush component with a hostView, calling fixture.detectChanges() will run a detectChanges() and checkNoChanges() .
And the App component is onPush, so the checkNoChanges() run will ignore App component, so the counters will be 1
Which is not correct.

Another case is:

    it('works', () => {
      @Component({
        template: '{{message}}',
        changeDetection: ChangeDetectionStrategy.OnPush
      })
      class OnPushCmp {
        @Input() message = 'initial';
        ngAfterViewInit() {
          this.message = 'updated';
        }
      }
      const comp = TestBed.configureTestingModule({declarations: [OnPushCmp]}).createComponent(OnPushCmp);
      expect(() => comp.detectChanges()).toThrow(); // current behavior is not throw.
    });

The reason is the same, the checkNoChanges() will skip the OnPushCmp, so the ExpressionChanged error is not thrown.

@JiaLiPassion JiaLiPassion self-assigned this Nov 3, 2020
@JiaLiPassion JiaLiPassion added core: change detection area: core Issues related to the framework runtime labels Nov 3, 2020
@ngbot ngbot bot added this to the needsTriage milestone Nov 3, 2020
@jelbourn jelbourn added the P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent label Nov 4, 2020
@ngbot ngbot bot modified the milestones: needsTriage, Backlog Nov 4, 2020
@pkozlowski-opensource
Copy link
Member

This issues is raising 2 separate problems for which we've got tracking issues already:

I'm going to close this issue in favour of the existing ones as those are more specific.

Duplicate of #12313
Duplicate of #45612

@angular-automatic-lock-bot
Copy link

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.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Jun 4, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: core Issues related to the framework runtime core: change detection P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent
Projects
None yet
Development

No branches or pull requests

3 participants