valor-software / ngx-bootstrap

Fast and reliable Bootstrap widgets in Angular (supports Ivy engine)
https://valor-software.com/ngx-bootstrap
MIT License
5.53k stars 1.69k forks source link

Carousel from ng2-bootstrap is causing Angular Universal to get stuck in a loop #1353

Closed smsoo closed 6 years ago

smsoo commented 7 years ago

I'm having a problem with ng2-bootsrap carousel. The page (/home2) loads fine when the carousel code in the page is commented out. But when the carousel code is running, the browser will keep waiting for the server to serve the page(which the server never will), causing the page to be empty. FYI, I'm using the Angular Universal Starter project.

Here is the carousel code with myInterval set to 5000 in the home2 component:

<carousel [interval]="myInterval" [noWrap]="noWrapSlides">
            <slide *ngFor="let slidez of slides;let index=index" [active]="slidez.active">
               <!-- <img [src]="slidez.image">-->
                <div class="carousel-caption">
                    <h4>Slide {{index}}</h4>
                    <p>{{slidez.text}}</p>
                </div>
            </slide>
            <i class="fa fa-chevron-left left carousel-control"></i>
            <i class="fa fa-chevron-right right carousel-control"></i>
        </carousel>

Here is the output on the server side (express):

GET /home2 - - ms - -
inside ngApp
/home2
GET /home2 - - ms - -
nginside ngApp
/home2
GET /home2 - - ms - -
inside ngApp
/home2
GET /home2 - - ms - -

Here is the console log statements on the server:

function ngApp(req, res) {
  console.log("inside ngApp");
  console.log(req.originalUrl);
  res.render('index', {
    req,
    res,
    // time: true, // use this to determine what part of your app is slow only in development
    preboot: false,
    baseUrl: '/',
    requestUrl: req.originalUrl,
    originUrl: `http://localhost:${ app.get('port') }`
  });
}

It appears to me Angular Universal is stuck in a loop. Here is the implementation: interior design carousel with the interval disabled in order to avoid the infinite loop. I would appreciate it if anyone could tell me how to fix this. Any suggestions will be appreciated.

Thanks

valorkin commented 7 years ago

As for now universal is lagging behind angular core, as soon as angular-cli will have support of it or someone can help with adding universal starter here :( Or I will finish with docs, and will try to add it

linusbrolin commented 7 years ago

@valorkin It's due to the use of setInterval: https://github.com/angular/universal-starter/issues/76

So you need to either replace it with for example the timer from RxJS: https://github.com/Reactive-Extensions/RxJS/blob/master/doc/api/core/operators/timer.md

Or, you could add some checks to make sure the page is in a browser. Either import from Angular Universal:

import { isBrowser } from 'angular2-universal';

if (isBrowser) {
    setInterval(...)
}

Or maybe this will work instead: if (typeof (window) !== 'undefined') {}

linusbrolin commented 7 years ago

@valorkin I've just tested in my project and this works:

  private restartTimer(): any {
    this.resetTimer();
    let interval = +this.interval;
    if (typeof (window) !== 'undefined' && !isNaN(interval) && interval > 0) {
      this.currentInterval = setInterval(
        () => {
          let nInterval = +this.interval;
          if (this.isPlaying && !isNaN(this.interval) && nInterval > 0 && this.slides.length) {
            this.nextSlide();
          } else {
            this.pause();
          }
        },
        interval);
    }
  }
aecz commented 7 years ago

Another option with no browser check and i think better performance would be to use NgZone to tell Angular not to wait for this interval and validate the zone as stable. Something like this:

this.ngZone.runOutsideAngular(()=>{
    setInterval(() => {
        this.ngZone.run( () => {
            this.nextSlide();
        });
    },5000);
});

For now i am using the isPlatformServer class to set the interval to false in this case and have Universal work. Im using Angular / Universal / platform-server 4.3.

@valorkin Would you accept a PR for this ?

Flood commented 7 years ago

I am also experiencing this error and it would be nice if you could accept the PR from @aecz. But the time (5000) should not be hard coded.

valorkin commented 7 years ago

Sure, I will gladly fix PR :) There are 2 big components with a lot of issues, datepicker and typeahead, so I am planning to release new versions of them in August

aecz commented 7 years ago

PR #2388 created.

Flood commented 7 years ago

When can we expect this to be merged?

IlyaSurmay commented 6 years ago

Fixed in #2388