Big FOUC as server side styles removed before the switch to client side

mgol commented 6 years ago

Note: for support questions, please use the Universal Slack Channcel or https://gitter.im/angular/universal

I'm submitting a ...
[x] bug report
[ ] feature request
Which parts of preboot are affected by this issue?
[ ] server side
[x] client side
[ ] inline
[ ] build process
[ ] docs
[ ] tests
Do you want to request a feature or report a bug?

A bug.

What is the current behavior?

Server-side styles are in some scenarios removed by Angular before Preboot switches the app to the client-side one, causing a potentially huge flash of unstyled content.

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem by creating a github repo.

Clone https://github.com/mgol/angular-server-bug and follow README instructions to run the app.
Load the page & observe it (it may take ~15 seconds).

What is the expected behavior?

There should be no visible app swap. Styles shouldn't disappear before the app is switched to the client side one.

What is the motivation / use case for changing the behavior?

Current behavior looks really broken to the end user.

Please tell us about your environment:
Browser: Chrome 64
Language: TypeScript 2.6.2
OS: macOS
Platform: Node.js
Other information (e.g. detailed explanation, stacktraces, related issues, suggestions how to fix, links for us to have context, eg. stackoverflow, gitter, etc)

This issue happens if a component is hidden behind *ngIf and the condition is set to a truthy one only after some time - in particular, when waiting for a specific HTTP request to finish. Angular thinks the client side app is ready as it bootstrapped correctly and it just waits for the HTTP request to finish to swap the flag. However, Preboot still waits for the app to get stable so it shows the server-side generated one. Unfortunately, its inline styles are already removed and the new ones are not loaded yet as Angular haven't created the component yet client side.

CaerusKaru commented 6 years ago

We're looking into this, but it looks like this may be an issue with Angular core. We'll update this as the issue develops.

xban1x commented 6 years ago

This is a huge issue for me. Where can we expect more info on this?

CaerusKaru commented 6 years ago

@xban1x The Angular core team is looking into this, but I don't have an ETA for you on a resolution.

xban1x commented 6 years ago

@CaerusKaru any news here?

CaerusKaru commented 6 years ago

None yet, still investigating.

xban1x commented 6 years ago

@CaerusKaru just pinging...

ItsMeDelanoDev commented 6 years ago

Ping!

snakenstein commented 6 years ago

I am experiencing this bug too. But, actually, not on the latest versions: Angular v.5.2.5, Preboot v.6.0.0-beta.3)

Willovent commented 6 years ago

Here is how I fixed this issue for my project : I removed the .withServerTransition from the import of the BrowserModule and use this instead

import { DOCUMENT, ɵgetDOM, ɵTRANSITION_ID } from '@angular/platform-browser';
import { APP_BOOTSTRAP_LISTENER, APP_ID, ModuleWithProviders, NgModule, PLATFORM_ID } from '@angular/core';
import { isPlatformBrowser } from '@angular/common';

export function removeStyleTags(document: HTMLDocument, platformId: string): Function {
  return () => {
    if (isPlatformBrowser(platformId)) {
      const dom = ɵgetDOM();

      const styles: HTMLElement[] = Array.prototype.slice.apply(dom.querySelectorAll(document, 'style[ng-transition]'));
      document.addEventListener('PrebootComplete', () => {
        setTimeout(() => styles.forEach(el => dom.remove(el)));
      });
    }
  };
}

@NgModule({})
export class ServerTransition {
  public static forRoot(param: { appId: string }): ModuleWithProviders {
    return {
      ngModule: ServerTransition,
      providers: [
        { provide: APP_ID, useValue: param.appId },
        { provide: ɵTRANSITION_ID, useValue: param.appId },
        {
          provide: APP_BOOTSTRAP_LISTENER,
          useFactory: removeStyleTags,
          deps: [DOCUMENT, PLATFORM_ID],
          multi: true
        }
      ]
    };
  }
}

As you can see, I remove the style tags when the event PrebootComplete is raised. No more flickering. Here is how to import it :

@NgModule({
  imports: [
    ...,
    BrowserModule,
    ServerTransition.forRoot({ appId: 'hg-customer' }),
    PrebootModule.withConfig({ appRoot: 'hg-root', replay: false, eventSelectors })
  ],
  declarations: [
    AppComponent,
    ...
  ]
})
export class AppModule {}

SwamyKottur commented 6 years ago

@Willovent On safari on page reload from browser refresh, the entire dom is being deleted. Did you come across this issue?

Willovent commented 6 years ago

I did actually, I've post an issue for that (https://github.com/angular/preboot/issues/91). However this has nothing to do with the transition, but more with the preboot buffer. To avoid it, I delayed my application bootstrap by 10ms after investigating this issue and evrything work's fine.

document.addEventListener('DOMContentLoaded', () => {
  setTimeout(
    () =>
      platformBrowserDynamic()
        .bootstrapModule(AppBrowserModule)
        .catch(err => console.log(err)),
    10
  );
});

However I had this issue on all browsers, so it may not be related to yours.

Edit: It is fix on beta-5. I removed the workaround on my application

Maqix commented 6 years ago

Hi @Willovent the fix you are talking about is in the beta-5 of what? I did see the flashing

Willovent commented 6 years ago

@Maqix I was talking about the entire DOM being deleted. This was a response to SwamyKottur question. I still have the code for the ServerTransition in my applications. FYI this will probably not corrected in this repository since the code that need to be modified is located in platforme_browser

malengrin commented 6 years ago

Thanks for the workaround @Willovent. It's solving this issue, but it breaks the Angular server transfer state. Because TRANSITION_ID is not properly provided, Angular generates a random one different from server and browser. It's generating a html that looks like:

<html>
  <head>
    [...]
    <style ng-transition="my-app">.foo[_ngcontent-sc0] {}</style>
    [...]
  </head>
  <body>
    [...]
    <script id="gmx-state" type="application/json">{&q;G.https://...}</script>
                ^-> gmx is randomly generated. It should be my-app-state
  </body>
</html>

I fixed it by keeping the BrowserModule.withServerTransition(...)import and adding this provider to the app module:

{
  provide: APP_INITIALIZER,
  useFactory: function(document: HTMLDocument, platformId: Object): Function {
    return () => {
      if (isPlatformBrowser(platformId)) {
        const dom = ɵgetDOM();
        const styles: any[] = Array.prototype.slice.apply(dom.querySelectorAll(document, `style[ng-transition]`));
        styles.forEach(el => {
          // Remove ng-transition attribute to prevent Angular appInitializerFactory
          // to remove server styles before preboot complete
          el.removeAttribute('ng-transition');
        });
        document.addEventListener('PrebootComplete', () => {
          // After preboot complete, remove the server scripts
          setTimeout(() => styles.forEach(el => dom.remove(el)));
        });
      }
    };
  },
  deps: [DOCUMENT, PLATFORM_ID],
  multi: true
}

amit-kumar27 commented 6 years ago

I am facing the same issue. @Willovent your workaround is not working with angular@5.2.1 and preboot@6.0.0.-beta.5. Can you please help? This is the configuration for root-module:-

import { BrowserModule } from "@angular/platform-browser";
import { PrebootModule } from 'preboot';
import { ServerTransition } from "./server-transition.module";
import { RootComponent } from "./root.component";
...

@NgModule({
  imports: [
    ...,
    BrowserModule,
    ServerTransition.forRoot({ appId: 'web-5' }),
    PrebootModule.withConfig({ appRoot: 'root', replay: false }),
  ],
  declarations: [
    RootComponent,
    ...
  ]
})
export class RootModule {}

rodneypantonial commented 5 years ago

I sidestepped this issue by adding a "loader" to cover the FOUC. Add this style in the <head>:

        #loader {
            width: calc(100vw);
            height: calc(100vh);
            display: flex;
            justify-content: center;
            position: fixed;
            left: 0;
            top: 0;
            z-index: 999;
        }

Add this at the start of the <body>:

<div id="loader" class="loader">
    <object type="image/svg+xml"
            data="./assets/images/logo-loading.svg"
            class="app-loading-img"
            height="30"
            style="align-self: center"
    >
        <p>Loading...</p>
    </object>
</div>

Add this code to the ngOnInit of the root module:

        this.router.events
            .pipe(
                filter(event => event instanceof NavigationEnd),
                first(),
            )
            .subscribe(() => {
                // pure JS, consider replacing with Renderer2
                document.getElementById('loader').classList.add('hidden');
            });

As the FOUC just happens on the first "load", this solves the issue with a "cover" without fancy coding

drew-thompson commented 5 years ago

@CaerusKaru is there an update planned for this issue, either before or after Ivy releases?

intellix commented 5 years ago

Not sure if it's the same thing but getting some weird behaviour and am not sure whether to open a different issue or not.

AppServerModule.imports contains:

BrowserModule.withServerTransition({ appId: 'meh' }),
PrebootModule.withConfig({ appRoot: 'cw-root' }),

Server delivers index.html like this:

<style ng-transition="meh">.router-outlet-main[_ngcontent-sc1] { ... }</style>
<script class="preboot-inline-script">var prebootInitFn = (function() { ... })();</script>

...

<cw-root _nghost-sc0="" ng-version="7.2.14">
  <script class="preboot-inline-script">prebootInitFn();</script>
  ...
</cw-root>

...

<script id="meh-state" type="application/json">{}</script></body></html>

As soon as the JS is downloaded, the DOM contains something like this:

<cw-root _nghost-sc0="" ng-version="7.2.14" _nghost-meh-c0="" style="display: none;">
  ... contents not removed ...
</cw-root>
<cw-root _nghost-sc0="" ng-version="7.2.14" ng-non-bindable="">
  ...
</cw-root>

All of the styles are for _nghost-meh-c0 so the secondary added cw-root is unstyled. Not seeing any details like this here in this issue so I'm not sure if it's the same?

Not replicable locally when building browser/server but happens on the server when there's a long gap between universal content delivered and client JS download

bmarti44 commented 5 years ago

I had the same issue as @intellix and I was able to fix it by injecting the EventPlayer service from preboot into the main app component. I think ran the replayAll method on that service in an ngAfterViewInit, and everything worked well. The below is a slimmed down version of my component with some pseudo code as an example. Make sure to wrap the replayAll method in an isBrowser check.

import {AfterViewInit, Component, OnInit} from '@angular/core';
import {EventReplayer} from 'preboot';

/**
 * Main app boot component.
 */
@Component({
  selector: 'nba-app',
  templateUrl: './app.component.html',
  styleUrls: ['./app.component.scss'],
})
export class AppComponent implements AfterViewInit {
  /**
   * AppComponent constructor.
   */
  constructor(
    private replayer: EventReplayer
  ) {}

  /**
   * After the view has been rendered, rerun all browser events.
   */
  ngAfterViewInit() {
    this.replayer.replayAll();
  }
}

kblestarge commented 5 years ago

Update on this issue?

vdjurdjevic commented 5 years ago

Can we add @malegrin solution to Preboot module? We could just provide that app initializer in PrebootModule for angular?

Ismaestro commented 5 years ago

anyone there?

Splaktar commented 5 years ago

There are some comments about this being an Angular core issue, but there isn't a link to the issue in angular/angular. Does anyone have a link to such issue?

Was it https://github.com/angular/angular/issues/15716? That was fixed in Angular 4.2.0 via PR https://github.com/angular/angular/pull/16712 if you set initialNavigation: 'enabled' in your Router options.

However, there appear to be a lot of comments that using that option is not enough. Some have commented that they need this Preboot library to solve the issue in addition to that router config.

Looking at https://github.com/mgol/angular-server-bug, it appears to be based on Angular 5.2.5 and it uses initialNavigation: 'enabled' along with Preboot.

There appear to be some related issues in angular/universal:

So it seems like this issue is not resolved, but I can't find any open Angular core issue to track the problem or any details of how Angular core is involved.

vdjurdjevic commented 5 years ago

I had this problem with 8.0.1, and router config is not enough. The solution below works like a charm, so I figured out we could just make this initializer part of preboot.

Thanks for the workaround @Willovent. It's solving this issue, but it breaks the Angular server transfer state. Because TRANSITION_ID is not properly provided, Angular generates a random one different from server and browser. It's generating a html that looks like:

<html>
  <head>
    [...]
    <style ng-transition="my-app">.foo[_ngcontent-sc0] {}</style>
    [...]
  </head>
  <body>
    [...]
    <script id="gmx-state" type="application/json">{&q;G.https://...}</script>
                ^-> gmx is randomly generated. It should be my-app-state
  </body>
</html>

I fixed it by keeping the BrowserModule.withServerTransition(...)import and adding this provider to the app module:

{
  provide: APP_INITIALIZER,
  useFactory: function(document: HTMLDocument, platformId: Object): Function {
    return () => {
      if (isPlatformBrowser(platformId)) {
        const dom = ɵgetDOM();
        const styles: any[] = Array.prototype.slice.apply(dom.querySelectorAll(document, `style[ng-transition]`));
        styles.forEach(el => {
          // Remove ng-transition attribute to prevent Angular appInitializerFactory
          // to remove server styles before preboot complete
          el.removeAttribute('ng-transition');
        });
        document.addEventListener('PrebootComplete', () => {
          // After preboot complete, remove the server scripts
          setTimeout(() => styles.forEach(el => dom.remove(el)));
        });
      }
    };
  },
  deps: [DOCUMENT, PLATFORM_ID],
  multi: true
}

CWSpear commented 5 years ago

@malengrin's solution mostly worked... background-images still flicker (load in and out, but since it's background-images, it doesn't affect overall layout). It's still 1000x better than it was!

hgwat commented 4 years ago

Any updates?

stemaDev commented 4 years ago

dom.querySelectorAll doesn't work, dom doesn't contain such a property, could this be an angular 9 thing?

CaerusKaru commented 4 years ago

dom.querySelectorAll doesn't work, dom doesn't contain such a property, could this be an angular 9 thing?

No

stemaDev commented 4 years ago

Then I rephrase my question: how come I get this error for this fix? "Property 'querySelectorAll' does not exist on type 'ɵDomAdapter'.ts(2339)"

edit: though I found this and "querySelectorAll" is there in version 8 but not in 9 https://unpkg.com/browse/@angular/platform-browser@8.2.14/platform-browser.d.ts https://unpkg.com/browse/@angular/platform-browser@9.0.7/platform-browser.d.ts

edit2: using document.querySelectorAll('style[ng-transition]') works

CarlosTorrecillas commented 4 years ago

Hello guys, just for our own interest? Do you think this will get addressed anytime soon? To me it seems a very important issue and wouldmake the actual SSR experience much better overall.

Many thanks!

Splaktar commented 4 years ago

@CarlosTorrecillas I haven't seen any indication that this will be resolved "soon". I agree that it's important for most SSR/Universal apps.

adrian-marcelo-gallardo commented 3 years ago

I had the same issue as @intellix and I was able to fix it by injecting the EventPlayer service from preboot into the main app component. I think ran the replayAll method on that service in an ngAfterViewInit, and everything worked well. The below is a slimmed down version of my component with some pseudo code as an example. Make sure to wrap the replayAll method in an isBrowser check.
import {AfterViewInit, Component, OnInit} from '@angular/core';
import {EventReplayer} from 'preboot';

/**
 * Main app boot component.
 */
@Component({
  selector: 'nba-app',
  templateUrl: './app.component.html',
  styleUrls: ['./app.component.scss'],
})
export class AppComponent implements AfterViewInit {
  /**
   * AppComponent constructor.
   */
  constructor(
    private replayer: EventReplayer
  ) {}

  /**
   * After the view has been rendered, rerun all browser events.
   */
  ngAfterViewInit() {
    this.replayer.replayAll();
  }
}

In My case I also needed to wrap the replay in a setTimeout. E.g.:

import { isPlatformBrowser } from '@angular/common';
import { Component, Inject, PLATFORM_ID } from '@angular/core';
import { EventReplayer } from 'preboot';

@Component({
  selector: 'app-root',
  templateUrl: './app.component.html',
  styleUrls: ['./app.component.scss']
})
export class AppComponent {

  constructor(
    @Inject(PLATFORM_ID) private platformId,
    private replayer: EventReplayer,
  ) {}

  ngAfterViewInit() {
    if (isPlatformBrowser(this.platformId)) {
      setTimeout(() => {
        this.replayer.replayAll();
      }, 0);
    }
  }
}

intellix commented 3 years ago

It's the 3 year anniversary that Preboot doesn't support Angular and seems it has been abandoned. I think the only solution is to just display an old-school loader or fade-out overlay on top of the content to prevent users clicking links and opening new tabs etc. Is this supported in other libraries like Scully?

I've always been on-board with how Angular provide everything out of the box: Animations, Forms, Universal rendering but it just means you come to rely on official components that get abandoned (like forms)

CaerusKaru commented 3 years ago

This project has not been abandoned and fully supports Angular v11 (latest). This issue is the only major issue, and doesn't affect all users. PRs are welcome to fix this issue, unfortunately it's low on our current list of priorities.

laurentgoudet commented 3 years ago

FYI I was profiling our app's performance & looking into ways to optimize it, and I've realized @malengrin's fix is causing an unnecessary style recalculation of the whole #document, due to the fact that the server styles removal is scheduled on the next tick of the event loop (setTimeout()).

The app is initially rendered on a hidden div (1st style recalculation), then when NgZone becomes stable the server-side view is swapped with the client-side one (2nd style recalculation - and the events are replayed & PrebootComplete is fired), and then the server styles are removed (3rd style recalculation).

In my case removing the server styles synchronously does not create a FOUC (since the element has been swapped anyway), and removes the 3rd unnecessary style recalculation, improving the TBT & TTI (as this happened to be the task long task).

In short the updated fix is:

{
  provide: APP_INITIALIZER,
  useFactory: function(document: HTMLDocument, platformId: Object): Function {
    return () => {
      if (isPlatformBrowser(platformId)) {
        const dom = ɵgetDOM();
        const styles: any[] = Array.prototype.slice.apply(dom.querySelectorAll(document, `style[ng-transition]`));
        styles.forEach(el => {
          // Remove ng-transition attribute to prevent Angular appInitializerFactory
          // to remove server styles before preboot complete
          el.removeAttribute('ng-transition');
        });
        document.addEventListener('PrebootComplete', () => {
          // After preboot complete, remove the server scripts
          styles.forEach(el => dom.remove(el));
        });
      }
    };
  },
  deps: [DOCUMENT, PLATFORM_ID],
  multi: true
}

dominikfryc commented 3 years ago

Not sure if it's the same thing but getting some weird behaviour and am not sure whether to open a different issue or not.

AppServerModule.imports contains:
BrowserModule.withServerTransition({ appId: 'meh' }),
PrebootModule.withConfig({ appRoot: 'cw-root' }),
Server delivers index.html like this:
<style ng-transition="meh">.router-outlet-main[_ngcontent-sc1] { ... }</style>
<script class="preboot-inline-script">var prebootInitFn = (function() { ... })();</script>

...

<cw-root _nghost-sc0="" ng-version="7.2.14">
  <script class="preboot-inline-script">prebootInitFn();</script>
  ...
</cw-root>

...

<script id="meh-state" type="application/json">{}</script></body></html>
As soon as the JS is downloaded, the DOM contains something like this:
<cw-root _nghost-sc0="" ng-version="7.2.14" _nghost-meh-c0="" style="display: none;">
  ... contents not removed ...
</cw-root>
<cw-root _nghost-sc0="" ng-version="7.2.14" ng-non-bindable="">
  ...
</cw-root>
All of the styles are for _nghost-meh-c0 so the secondary added cw-root is unstyled. Not seeing any details like this here in this issue so I'm not sure if it's the same?

Not replicable locally when building browser/server but happens on the server when there's a long gap between universal content delivered and client JS download

I finally resolved this issue after almost a year. So here is my solution for anyone having the same problem.

It was only happening when I was using SSR for pages behind authentication in my case. Client-side hydration most likely happens after the application is stable (ApplicationRef.isStable in Angular docs). In usage notes, you can see this important note:

the application will never be stable if you start any kind of recurrent asynchronous task when the application starts (for example for a polling process, started with a setInterval, a setTimeout or using RxJS operators like interval)

Which was exactly the reason why my app did not load in SSR because I set an interval for polling user notifications in a service. There are some solutions for this: start interval after the app is stable, start interval in ngOnInit instead of a constructor, or @bmarti44 solution by manually replaying events in ngAfterViewInit.

angular / preboot

Big FOUC as server side styles removed before the switch to client side #75