Skip to content
This repository was archived by the owner on Mar 20, 2024. It is now read-only.

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

Open
2 of 8 tasks
mgol opened this issue Feb 15, 2018 · 36 comments
Open
2 of 8 tasks

Comments

@mgol
Copy link
Member

mgol commented Feb 15, 2018

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

  • I'm submitting a ...
  • bug report
  • feature request
  • Which parts of preboot are affected by this issue?
  • server side
  • 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.
  1. Clone https://github.com/mgol/angular-server-bug and follow README instructions to run the app.
  2. 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
Copy link
Member

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
Copy link

xban1x commented Mar 4, 2018

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

@CaerusKaru
Copy link
Member

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

@xban1x
Copy link

xban1x commented Mar 24, 2018

@CaerusKaru any news here?

@CaerusKaru
Copy link
Member

None yet, still investigating.

@xban1x
Copy link

xban1x commented Apr 12, 2018

@CaerusKaru just pinging...

@ItsMeDelanoDev
Copy link
Contributor

Ping!

@snakenstein
Copy link

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
Copy link

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
Copy link

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

@Willovent
Copy link

Willovent commented Aug 18, 2018

I did actually, I've post an issue for that (#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
Copy link

Maqix commented Aug 31, 2018

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

@Willovent
Copy link

@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
Copy link

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
Copy link

amit-kumar27 commented Sep 21, 2018

I am facing the same issue. @Willovent your workaround is not working with [email protected] and [email protected]. 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
Copy link

rodneypantonial commented Jan 5, 2019

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
Copy link

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

@intellix
Copy link

intellix commented Apr 25, 2019

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
Copy link

bmarti44 commented May 2, 2019

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
Copy link

Update on this issue?

@vdjurdjevic
Copy link

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

@Ismaestro
Copy link

anyone there?

@Splaktar
Copy link
Contributor

Splaktar commented Sep 23, 2019

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 angular/angular#15716?
That was fixed in Angular 4.2.0 via PR angular/angular#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
Copy link

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
Copy link

CWSpear commented Oct 6, 2019

@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
Copy link

hgwat commented Jan 21, 2020

Any updates?

@stemaDev
Copy link

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

@CaerusKaru
Copy link
Member

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

No

@stemaDev
Copy link

stemaDev commented Mar 24, 2020

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/[email protected]/platform-browser.d.ts
https://unpkg.com/browse/@angular/[email protected]/platform-browser.d.ts

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

@CarlosTorrecillas
Copy link

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
Copy link
Contributor

@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
Copy link

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
Copy link

intellix commented Feb 17, 2021

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
Copy link
Member

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
Copy link

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).

Screen Shot 2021-07-08 at 10 41 12

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).

Screen Shot 2021-07-08 at 10 45 03

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
Copy link

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.

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

No branches or pull requests