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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Renderer2 method listen does not work if wrapped with runOutsideAngular method of injected NgZone #33687

Closed
sharikovvladislav opened this issue Nov 8, 2019 · 8 comments

Comments

@sharikovvladislav
Copy link

@sharikovvladislav sharikovvladislav commented Nov 8, 2019

馃悶 bug report

Affected Package

The issue is caused by package @angular/core

Is this a regression?

Yes, the previous version in which this bug was not present was: 9.0.0-rc.1 ViewEngine (without Ivy)

Description

A clear and concise description of the problem...

The problem is around Renderer2 listen method and NgZone runOutsideAngular method.

About repro application: I have simple page with div at the center of it. In ngAfterViewInit lifecycle hook I add listeners using injected Renderer2 and it's listen method.

I also have 3 unit tests:

  1. general test about component is created successfully
  2. unit test for testing renderer.listen property to listen events
  3. unit test same as previous but wrapped into ngZone.runOutsideAngular method.
Actual behavior:

In ViewEngine any of those tests are green and success. In Ivy listen wrapped with runOutsideAngular test fails.

I expect:

It does not matter is listen wrapped in runOutsideAngular or not it must work both in ViewEngine and Ivy.

The code

The component:

export class AppComponent {
  @ViewChild('ref', { static: true }) ref: ElementRef;
  title = 'latest-ng-ivy-boilerplate';

  constructor(private renderer: Renderer2, private ngZone: NgZone) {}

  ngAfterViewInit() {
    this.ngZone.runOutsideAngular(() => {
      this.renderer.listen(this.ref.nativeElement, 'click', this.handlerFromOutsideNgZone);
    });

    this.renderer.listen(this.ref.nativeElement, 'click', this.handlerFromNgZone);
  }

  handlerFromNgZone(evt: MouseEvent) {
    console.log('>> clicked from NgZone', evt);
  }

  handlerFromOutsideNgZone(evt: MouseEvent) {
    console.log('>> clicked from outside NgZone', evt);
  }
}

The tests:


  it(`should listen click without runOutsideAngular wrapper`, () => {
    const fixture = TestBed.createComponent(AppComponent);
    const debugElement = fixture.debugElement;
    const component = fixture.componentInstance;

    spyOn(component, 'handlerFromNgZone').and.callThrough();
    fixture.detectChanges();

    const elementWithListenerDebugElement = debugElement.query(By.css('.click-me'));

    const event = {};

    elementWithListenerDebugElement.triggerEventHandler('click', event);

    expect(component.handlerFromNgZone).toHaveBeenCalledWith(event);
  });

  it(`should listen click wrapped with runOutsideAngular`, () => {
    const fixture = TestBed.createComponent(AppComponent);
    const debugElement = fixture.debugElement;
    const component = fixture.componentInstance;

    spyOn(component, 'handlerFromOutsideNgZone').and.callThrough();
    fixture.detectChanges();

    const elementWithListenerDebugElement = debugElement.query(By.css('.click-me'));

    const event = {};

    elementWithListenerDebugElement.triggerEventHandler('click', event);

    expect(component.handlerFromOutsideNgZone).toHaveBeenCalledWith(event);
  });

ViewEngine output (npm run test with enableIvy: false option in tsconfig.spec.json:

npm run test

> latest-ng-ivy-boilerplate@0.0.0 test /Users/vladislav.sharikov/tmp/latest-ng-ivy-boilerplate
> ng test

30% building 21/21 modules 0 active08 11 2019 15:35:08.414:WARN [karma]: No captured browser, open http://localhost:9877/
08 11 2019 15:35:09.254:INFO [karma-server]: Karma v4.1.0 server started at http://0.0.0.0:9877/
08 11 2019 15:35:09.255:INFO [launcher]: Launching browsers Chrome with concurrency unlimited
30% building 22/22 modules 0 active08 11 2019 15:35:09.278:INFO [launcher]: Starting browser Chrome
08 11 2019 15:35:14.764:WARN [karma]: No captured browser, open http://localhost:9877/
08 11 2019 15:35:15.588:INFO [Chrome 78.0.3904 (Mac OS X 10.14.6)]: Connected on socket huG1ssfGRkY6loYTAAAA with id 6143106
LOG: '>> clicked from outside NgZone', Object{}
Chrome 78.0.3904 (Mac OS X 10.14.6): Executed 1 of 3 SUCCESS (0 secs / 0.05 secs)
LOG: '>> clicked from NgZone', Object{}
Chrome 78.0.3904 (Mac OS X 10.14.6): Executed 1 of 3 SUCCESS (0 secs / 0.05 secs)
LOG: '>> clicked from outside NgZone', Object{}
Chrome 78.0.3904 (Mac OS X 10.14.6): Executed 2 of 3 SUCCESS (0 secs / 0.078 secs)
LOG: '>> clicked from NgZone', Object{}
Chrome 78.0.3904 (Mac OS X 10.14.6): Executed 2 of 3 SUCCESS (0 secs / 0.078 secs)
Chrome 78.0.3904 (Mac OS X 10.14.6): Executed 3 of 3 SUCCESS (0.112 secs / 0.1 secs)
TOTAL: 3 SUCCESS
TOTAL: 3 SUCCESS

Ivy output:

 鉁 顐 ~/tmp/latest-ng-ivy-boilerplate 顐 顐 master 鈼 顐 npm run test

> latest-ng-ivy-boilerplate@0.0.0 test /Users/vladislav.sharikov/tmp/latest-ng-ivy-boilerplate
> ng test

30% building 41/41 modules 0 active08 11 2019 15:36:17.616:WARN [karma]: No captured browser, open http://localhost:9877/
08 11 2019 15:36:18.430:INFO [karma-server]: Karma v4.1.0 server started at http://0.0.0.0:9877/
08 11 2019 15:36:18.430:INFO [launcher]: Launching browsers Chrome with concurrency unlimited
08 11 2019 15:36:18.445:INFO [launcher]: Starting browser Chrome
08 11 2019 15:36:22.601:WARN [karma]: No captured browser, open http://localhost:9877/
08 11 2019 15:36:22.650:INFO [Chrome 78.0.3904 (Mac OS X 10.14.6)]: Connected on socket l0d4_43bHLsUoo3vAAAA with id 48919445
LOG: '>> clicked from NgZone', Object{}
Chrome 78.0.3904 (Mac OS X 10.14.6): Executed 0 of 3 SUCCESS (0 secs / 0 secs)
Chrome 78.0.3904 (Mac OS X 10.14.6) AppComponent should listen click wrapped with runOutsideAngular FAILED
        Error: Expected spy handlerFromOutsideNgZone to have been called with:
          [ Object({  }) ]
        but it was never called.
            at <Jasmine>
            at UserContext.<anonymous> (http://localhost:9877/_karma_webpack_/src/app/app.component.spec.ts:51:48)
            at ZoneDelegate.invoke (http://localhost:9877/_karma_webpack_/node_modules/zone.js/dist/zone-evergreen.js:365:1)
            at ProxyZoneSpec.onInvoke (http://localhost:9877/_karma_webpack_/node_modules/zone.js/dist/zone-testing.js:305:1)
Chrome 78.0.3904 (Mac OS X 10.14.6): Executed 1 of 3 (1 FAILED) (0 secs / 0.108 secs)
Chrome 78.0.3904 (Mac OS X 10.14.6) AppComponent should listen click wrapped with runOutsideAngular FAILED
        Error: Expected spy handlerFromOutsideNgZone to have been called with:
          [ Object({  }) ]
        but it was never called.
            at <Jasmine>
            at UserContext.<anonymous> (http://localhost:9877/_karma_webpack_/src/app/app.component.spec.ts:51:48)
            at ZoneDelegate.invoke (http://localhost:9877/_karma_webpack_/node_modules/zone.js/dist/zone-evergreen.js:365:1)
LOG: '>> clicked from NgZone', Object{}
Chrome 78.0.3904 (Mac OS X 10.14.6): Executed 2 of 3 (1 FAILED) (0 secs / 0.114 secs)
Chrome 78.0.3904 (Mac OS X 10.14.6): Executed 3 of 3 (1 FAILED) (0.135 secs / 0.121 secs)
TOTAL: 1 FAILED, 2 SUCCESS
TOTAL: 1 FAILED, 2 SUCCESS

馃敩 Minimal Reproduction

Here is stackblitz repro *BUT IVY DOES NOT WORK THERE* (I don't know how to enabled it in stackblitz): https://stackblitz.com/github/sharikovvladislav/ivy-listen-in-run-outside-angular-do-not-work

So, here is github repo: https://github.com/sharikovvladislav/ivy-listen-in-run-outside-angular-do-not-work

  1. clone it
  2. go to the directory where you cloned the repo
  3. install dependencies using yarn
  4. execute unit tests: yarn run test

You will get the output above. I pushed version with Ivy enabled to the repo. You can disable it using change enableIvy option to false in tsconfig.spec.json.

馃敟 Exception or Error

No exceptions.

馃實 Your Environment

Angular Version:




Angular CLI: 9.0.0-rc.0
Node: 10.16.0
OS: darwin x64
Angular: 9.0.0-rc.1
... animations, common, compiler, compiler-cli, core, forms
... language-service, platform-browser, platform-browser-dynamic
... router

Package                           Version
-----------------------------------------------------------
@angular-devkit/architect         0.900.0-rc.0
@angular-devkit/build-angular     0.900.0-rc.0
@angular-devkit/build-optimizer   0.900.0-rc.0
@angular-devkit/build-webpack     0.900.0-rc.0
@angular-devkit/core              9.0.0-rc.0
@angular-devkit/schematics        9.0.0-rc.0
@angular/cli                      9.0.0-rc.0
@ngtools/webpack                  9.0.0-rc.0
@schematics/angular               9.0.0-rc.0
@schematics/update                0.900.0-rc.0
rxjs                              6.5.3
typescript                        3.6.4
webpack                           4.41.2
@mhevery

This comment has been minimized.

Copy link
Member

@mhevery mhevery commented Nov 9, 2019

The issue is here mhevery/ivy-listen-in-run-outside-angular-do-not-work@f7f8680#diff-5c26d2c8f8838f32ace87fdecd6344eaR18-R22. @JiaLiPassion Could you please have a look and let me know what is going on?

Update: I see the issue is that outside of Angular Zone https://github.com/angular/angular/blob/master/packages/platform-browser/src/dom/events/dom_events.ts#L142-L195 prevents the normal registration.

mhevery added a commit to mhevery/angular that referenced this issue Nov 9, 2019
Angular sometimes registers events using [fast path](https://github.com/angular/angular/blob/1d429b216556911edc4b0675ece4cb9081967155/packages/platform-browser/src/dom/events/dom_events.ts#L142-L195)
which than need to be located correctly by the `DebugNode` so that
`triggerEventHandler` works correctly.

Fixes angular#33687
@JiaLiPassion JiaLiPassion self-assigned this Nov 9, 2019
@JiaLiPassion

This comment has been minimized.

Copy link
Contributor

@JiaLiPassion JiaLiPassion commented Nov 9, 2019

@mhevery , it is a bug of eventListeners.

First why not work

The reason now it is not working, because in ivy, now when call triggerEventHandler, it will try to use eventListeners to find out the event listener list from target. And there is a bug in here,
The FALSE variable defined https://github.com/angular/angular/blob/master/packages/platform-browser/src/dom/events/dom_events.ts#L28 is FALSE, but false in https://github.com/angular/angular/blob/master/packages/zone.js/lib/common/utils.ts#L38. So when the event Listener is added outside of ngZone, it will not work.

Second, why not work in ivy only.

In ViewEngine, the DomRendererFactory2 is DebugRendererFactory2, so the Renderer2 is DebugRenderer2, so the renderer2.listene method will call here

debugEl.listeners.push(new DebugEventListener(eventName, callback));
, so the debug node can manage the listeners array by itself.

But in ivy, the DomRendererFactory2 is DomRendererFactory2 itself, so the Renderer2 becomes DefaultDomRenderer2, so the listeners array becomes empty, and finally it use eventTarget.eventListeners to get all listeners.

@sharikovvladislav

This comment has been minimized.

Copy link
Author

@sharikovvladislav sharikovvladislav commented Nov 11, 2019

Do you have any plans like when this PR will be merged and when next RC will be released?

Do we have workarounds at the moment?

@JiaLiPassion

This comment has been minimized.

Copy link
Contributor

@JiaLiPassion JiaLiPassion commented Nov 11, 2019

@sharikovvladislav, I believe this PR will be merged very soon if the solution is ok after review.
And unfortunately, there is no easy workarounds now (there are some, but none of them are easy to do...) so please wait for the next RC release. Or you may not using triggerEventHandler and use some other alternatives.

@mhevery

This comment has been minimized.

Copy link
Member

@mhevery mhevery commented Nov 11, 2019

fix in #33711

@kara kara closed this in d8be830 Nov 11, 2019
kara added a commit that referenced this issue Nov 11, 2019
@sharikovvladislav

This comment has been minimized.

Copy link
Author

@sharikovvladislav sharikovvladislav commented Nov 12, 2019

Thank you =) That was fast

@sharikovvladislav

This comment has been minimized.

Copy link
Author

@sharikovvladislav sharikovvladislav commented Nov 12, 2019

I patched local version of node_modules in my project. The problem is gone.

Thank you! =)

@sharikovvladislav

This comment has been minimized.

Copy link
Author

@sharikovvladislav sharikovvladislav commented Nov 13, 2019

@mhevery @JiaLiPassion any info when you will have next release candidate?

Looks like you have 1 rc / next version per week. Am I right that new one will be published something like tomorrow or in a next days?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can鈥檛 perform that action at this time.