Skip to content

Further optimize initial page load by leveraging preboot#1760

Closed
ybnd wants to merge 9 commits intoDSpace:mainfrom
atmire:w2p-93492_Further-optimize-initial-page-load-by-leveraging-preboot_PR
Closed

Further optimize initial page load by leveraging preboot#1760
ybnd wants to merge 9 commits intoDSpace:mainfrom
atmire:w2p-93492_Further-optimize-initial-page-load-by-leveraging-preboot_PR

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
Closed
6 tasks
@tdonohue tdonohue self-requested a review August 18, 2022 15:02
@github-actions
Copy link

github-actions bot commented Feb 15, 2026

This pull request has been automatically marked as stale because it has not had
activity in one year. It will be closed in 14 days if no further activity occurs.

Allowing pull requests to close as stale helps us filter out old work that is no longer
relevant and helps developers focus on reviewing current work.

All pull requests closed by this bot act like normal pull requests;
they can be searched for, commented on or reopened at any point.

If these changes are still relevant then please comment and/or rebase your PR based on the
latest DSpace code. This will remove the stale status and notify us to assign a reviewer
for your PR.

@github-actions github-actions bot added the stale This issue or PR is stale because it has not been updated in a long time. label Feb 15, 2026
@tinsch
Copy link
Contributor

tinsch commented Feb 17, 2026

We are currently looking to solve some performance issues around page loading times and got notified because the PR was marked as stale. @ybnd I know this would probably need a lot of work because of the changes and changed dependencies in the meantime, but would you be willing to look at it again?

@ybnd
Copy link
Member Author

ybnd commented Feb 23, 2026

@tinsch I expect we'd have to more or less start from scratch at this point...

The @angular/preboot library has been deprecated, Angular Universal has been absorbed into Angular itself and many things have shifted around.

We are currently looking to solve some performance issues around page loading times and got notified because the PR was marked as stale

For what it's worth, this PR and #1761 were mostly looking at smoothness and perceived performance.
As far as I know SSR performance has improved in the meantime, especially with the more recent versions of Angular, so I don't know if this exact approach is as relevant anymore.

@tdonohue
Copy link
Member

tdonohue commented Feb 23, 2026

@ybnd : If this approach is no longer relevant, should we close this PR (and possibly also #1761 and #1733)? While I very much appreciate the work you did here to try out some optimizations, if this work isn't relevant anymore, I'd lean towards closing the work. Even if closed, we can always revisit these PRs at a time in the future if there is useful information to be had.

@ybnd
Copy link
Member Author

ybnd commented Feb 24, 2026

@tdonohue

Even if closed, we can always revisit these PRs at a time in the future if there is useful information to be had

That was more or less why I had kept them around as drafts I think, but looking back it would be more clear if they're closed.

@ybnd ybnd closed this Feb 24, 2026
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

Assignees

@ybnd ybnd

Labels

low priority merge conflict stale This issue or PR is stale because it has not been updated in a long time.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Further optimize initial page load by leveraging preboot

3 participants

@ybnd @tinsch @tdonohue