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

NgUpgrade Performance Issues #204

Open
tcrite opened this issue May 1, 2018 · 12 comments
Open

NgUpgrade Performance Issues #204

tcrite opened this issue May 1, 2018 · 12 comments

Comments

@tcrite
Copy link

tcrite commented May 1, 2018

Hi I'm working on a project to upgrade an AngularJs app to Angular 5 using ngupgrade. I had vs-repeat installed prior to the upgrade and it worked fined. However, since installing Angular 5 and bootstrapping Angular and AngularJs accordingly, I'm getting extremely poor performance from vs-repeat. Almost to the point where it's unusable. Was wondering if it could have something to do with AngularJs and Angular competing over change detection? Thanks and any help would be much appreciated.

@m3Lith
Copy link

m3Lith commented May 20, 2018

I'm having the same issue currently. Did you manage to do something by any chance?

@tcrite
Copy link
Author

tcrite commented May 25, 2018

@m3Lith if I comment this line out in my polyfills.ts file:

import 'zone.js/dist/zone'; // Included with Angular CLI.

it works ok. haven't had any ill effects from it yet, but I get the feeling it's not ideal

@m3Lith
Copy link

m3Lith commented May 25, 2018

@tcrite but above that it says "Zone JS is required by default for Angular itself." and it throws an error inside console: "Error: In this configuration Angular requires Zone.js".

Does it really work for you with that one change?

@tcrite
Copy link
Author

tcrite commented May 25, 2018

@m3Lith you are right. I just realized I added an import for zone.js in my main.ts file. Seems. to fix the configuration error thrown when you comment it out in the polyfills file

import 'zone.js';

@dfitoussi
Copy link

I comment the line out and added import 'zone.js'; but still, have poor performance.

@jziggas
Copy link

jziggas commented Jul 10, 2018

It has to do with this snippet:

$scope.$watch(() => {
          if (typeof window.requestAnimationFrame === 'function') {
            window.requestAnimationFrame(reinitOnClientHeightChange);
          } else {
            reinitOnClientHeightChange();
          }
});

If you profile your app and drill down you will probably see something like zone.js -> Animation Frame Fired -> Recalculate Style (angular-vs-repeat) due to this hook into window.requestAnimationFrame. Unfortunately I don't have a good solution, we were forced to remove this package because it brought our app to an unusable crawl.

@m3Lith
Copy link

m3Lith commented Jul 11, 2018

@jziggas: have you tried uncommenting/adding this line in polyfills.ts?

(window as any).__Zone_disable_requestAnimationFrame = true;

It didn't help me, but I wonder if you would see a difference.

@jziggas
Copy link

jziggas commented Jul 11, 2018

I don't have a polyfills.ts in my project so I'm not sure where that would go.

@m3Lith
Copy link

m3Lith commented Jul 11, 2018

You're including zone.js somewhere though? Try adding that line before the import of it.

@ghost
Copy link

ghost commented Jul 17, 2018

Just encountered this issue, I can confirm @m3Lith's suggestion works

@jkainth
Copy link

jkainth commented Jul 31, 2018

Had the same issue.
Disabling request animationframe on the zone didn't make sense - since it would mess with other places which use it.
Instead i just forked the vs-repeat and commented the line contents of the $watch except for the reinitOnClientHeightChange; A bit of perf hit on vs-repeat, better than a blow up.

@amalitsky
Copy link

As of now angular-vs-repeat virtually doesn't work in angularJS/angular hybrid mode due to constant $digest re-triggering.

@kamilkp, would this be considered as a reasonable fix in the form of PR? Thanks

function angularHybridModeIsOn() {
  return '$$UpgradeModule' in $element.injector().modules;
}

$scope.$watch(function () {
  if (typeof window.requestAnimationFrame === 'function' && !angularHybridModeIsOn()) { 
    window.requestAnimationFrame(reinitOnClientHeightChange);
  } else {
    reinitOnClientHeightChange();
  }
});

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants