Skip to content

Further optimize initial page load by leveraging preboot #1760

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.

Sign up for GitHub

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

Jump to bottom
Draft
wants to merge 9 commits into
base: main
Choose a base branch
from
Draft

Further optimize initial page load by leveraging preboot #1760

wants to merge 9 commits into from

Conversation

@ybnd
Copy link
Member

@ybnd ybnd commented Aug 11, 2022 •
edited
Loading

References

Description

This PR introduces Angular preboot as a next step in improving initial page load performance in production mode.

The main improvement in this PR concerns the transition from server-side to client-side rendered HTML on lazily-loaded pages (e.g. /search, /items/... etc.)

  • Instead of blanking out the page while it is being replaced, preboot buffers the CSR page in the background while the SSR page is still visible and only replaces it once the app is completely done loading

    This results in significantly smoother page loads

Changes:

  • Added preboot

  • Fixed some minor compatibility issues between DSpace and preboot

    • Made ApplicationRef.isStable more reliable (preboot depends on it)
    • Workaround for loss of CSS during CSR

  • Fixed some minor bugs with the navbar search box & admin sidebar

  • Added an animation to make it more clear when the application is still loading. IMO this can help a lot with understanding that e.g. events may not come through

    More of a proof-of-concept for now, feedback is greatly appreciated!

    • There's a hiccup in the animation around the time the CSR buffering starts, not sure how to get rid of it
    • For this to work, the SSR HTML must start out with the animation playing -- this also means that it stays on for users that disable JS

Caveats / further work:

  • We've observed very minor flashing on some DSO pages with thumbnails even after this PR
  • On some systems we've seen fonts flash back and forth between the ones downloanded ~ SSR and the fallbacks during the transition. At this point it's not exactly cear why this happens (or why it doesn't in the other cases)
  • preboot currently doesn't support Angular 13 without a workaround that hasn't been merged yet
    • There is a fork on NPM that includes it -- confirmed that we can get it working properly again if we use this.
    • However, we don't recommend merging this PR until this problem is addressed in preboot itself
  • Another feature of preboot is to capture interaction events on the SSR page and replay them once the CSR page is loaded. However, fully integrating this would require more time. See Enable preboot event replay for shorter apparent TTI #1761 for more information.

Instructions for Reviewers

Compare initial load perfomance in production mode between this PR and current main in different browsers

  1. Start from a blank page

  2. Open any lazily-loaded DSpace page (try this for multiple pages, while logged in and not)

    • /search?q=...
    • /communities/...
    • ...

  3. Do the same on https://demo7.dspace.org/home and compare for "smoothness"

    • In the current build, the page will blank out for a moment while rendering.

      This PR should significantly improve this

    • Currently there is a glitch when loading or refreshing pages while already logged in; the SSR page accounts for the admin sidebar as if it was pinned, while it can't be.

      This PR should eliminate this bug.

  4. Hard refresh the page and compare in the same way. Note that the "feel" can be quite different between these two cases -- when loading from a blank page the flashing tends to be more noticeable

Checklist

This checklist provides a reminder of what we are going to look for when reviewing your PR. You need not complete this checklist prior to creating your PR (draft PRs are always welcome). If you are unsure about an item in the checklist, don't hesitate to ask. We're here to help!

  • My PR is small in size (e.g. less than 1,000 lines of code, not including comments & specs/tests), or I have provided reasons as to why that's not possible.
  • My PR passes TSLint validation using yarn run lint
  • My PR doesn't introduce circular dependencies
  • My PR includes TypeDoc comments for all new (or modified) public methods and classes. It also includes TypeDoc for large or complex private methods.
  • My PR passes all specs/tests and includes new/updated specs or tests based on the Code Testing Guide.
  • If my PR includes new, third-party dependencies (in package.json), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation.

ybnd added 9 commits August 4, 2022 18:53
@ybnd
93492: Use preboot for SSR/CSR transition
79c4d09 
https://github.com/angular/preboot
@ybnd
93492: Fix unresolved ApplicationRef.isStable
aa5ce3c 
preboot is triggered when the app first becomes stable, which caused the app to hang once CSR started
Tracked the issue down to initialization
  - GoogleAnalyticsService.addTrackingIdToPage()
  - AuthService.trackTokenExpiration()
@ybnd
93492: Fix flash of unstyled content during CSR
d692f10 
SSR HTML inlines CSS into style.ng-transition elements. By default, Angular removes these as soon as CSR starts.
Because preboot buffers CSR off-screen, the SSR HTML remains visible during the transition - without CSS.

This workaround strips the ng-transition attribute from SSR styles so they aren't removed automatically
and subsequently removes these same styles once preboot is finished.
@ybnd
93492: Fix ds-search-navbar submit button expand on click
754c1b7
@ybnd
93492: Fix SSR dodging pinned admin sidebar
d759e51 
On the server, @slideSidebarPadding always resolved to 'expanded' because slideSidebarOver did not emit true
@ybnd
93492: Turn off preboot event replay for now
53068c4
@ybnd
93492: Add loading animation to mark when CSR is finished
92975ce
@ybnd
Merge remote-tracking branch 'origin/main' into w2p-93492_Further-opt…
57b1903 
…imize-initial-page-load-by-leveraging-preboot
@ybnd
93492: Use preboot fork with workaround for Angular 12+ issue
4dd3715
@ybnd ybnd mentioned this pull request Aug 11, 2022
Draft
6 tasks
@tdonohue tdonohue self-requested a review August 18, 2022 15:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tdonohue tdonohue Awaiting requested review from tdonohue

At least 1 approving review is required to merge this pull request.

Assignees

@ybnd ybnd

Labels

low priority merge conflict

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Further optimize initial page load by leveraging preboot

2 participants

@ybnd @tdonohue