Skip to content
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.

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

Performance: back/forward cache disabled with an open IndexedDB connection in Chromium #996

Open
4 tasks done
naveedahmed1 opened this issue Apr 13, 2023 · 5 comments
Open
4 tasks done

Comments

@naveedahmed1
Copy link

naveedahmed1 commented Apr 13, 2023

Checks before posting an issue

Configuration

  • @ngx-pwa/local-storage version:
  • Angular version (ng version):
"@ngx-pwa/local-storage": "15.0.0",

"@angular/core": "16.0.0-next.7",

Description of the issue

As described in https://web.dev/bfcache, the browsers' back/forward cache doesn't work when IndexedDB has an open connection. It seems that in current implementation the connections are opened but they're never closed. Here when running a Lighthouse audit for an Angular app, there's an error

Page prevented back/forward cache restoration 1 failure reason
Many navigations are performed by going back to a previous page, or forwards again. The back/forward cache (bfcache) can speed up these return navigations. [Learn more about the bfcache](https://developer.chrome.com/docs/lighthouse/performance/bf-cache/?utm_source=lighthouse&utm_medium=devtools)

The documentation of back/forward cache: https://developer.chrome.com/docs/lighthouse/performance/bf-cache/

How to reproduce the issue

Running a Lighthouse audit for an Angular app using latest version (I am testing it on Version 112.0.5615.87 (Official Build) (64-bit)) of Chrome, should throw this error.

@cyrilletuzi cyrilletuzi changed the title Performance: Pages with an open IndexedDB connection does not work with back/forward cache Performance: back/forward cache with an open IndexedDB connection does not work Apr 13, 2023
@cyrilletuzi cyrilletuzi changed the title Performance: back/forward cache with an open IndexedDB connection does not work Performance: back/forward cache with an open IndexedDB connection does not work in Chrome Apr 13, 2023
@cyrilletuzi
Copy link
Owner

cyrilletuzi commented Apr 13, 2023

Hi,

I have been able to reproduce the Lighthouse diagnostic in Chrome 112 (it was not here in previous versions).

Could you investigate if this is a Chrome-specific behavior? While disabling back/forward cache if an IndexedDB transaction is still opened makes sense, it is quite weird to disable it if a connection is still open, which is a very common and intended behavior.

The Chrome documentation itself is ambiguous, as it first tells there can be issues if "in the middle of an IndexedDB transaction", and then later in the article it tells there can be issues "with an open IndexedDB connection".

Firefox documentation says cache is disabled if "the page has running IndexedDB transactions".

Found nothing about indexedDb in Safari documentation.

Could you find a way to check the behavior of Firefox and Safari?

@cyrilletuzi cyrilletuzi changed the title Performance: back/forward cache with an open IndexedDB connection does not work in Chrome Performance: back/forward cache disabled with an open IndexedDB connection in Chrome Apr 13, 2023
@naveedahmed1
Copy link
Author

naveedahmed1 commented Apr 13, 2023

Same is the case with Edge, in bfcache issue on lighthouse repository GoogleChrome/lighthouse#13960
addyosmani mentioned about BFCache Not Restored Reasons

As per the list:

Pages that have an open IndexedDB connection are not currently eligible for back/forward cache.
Close the open connection in the pagehide event to make this page eligible.

I am not sure if this list is about all browsers.

There's some discussion on the topic in Firebase repo as well firebase/firebase-js-sdk#6167

*Adding links for reference.

@cyrilletuzi
Copy link
Owner

Same is the case with Edge

Edge is based on Chromium so that was expected.

I've done some testing, and like the discussion in the Firebase thread you mentioned suggested, it is a big mess.

First, without any IndexedDB (or anything else that could disabled the back/forward cache):

  • only Firefox is working correctly:
    • pageshow
    • pagehide
  • Safari:
    • pageshow
    • pagehide: never shown, either in event.persisted mode or not
  • Chromium behavior is the most crazy one:
    • pageshow
    • pagehide: not shown when the page is leaved, but is shown afterwards when we come back and pageshow is fired, adding back the console.log higher in the console history stack, where it was not appearing before...

Second, with IndexedDB opened:

  • ✅ Firefox: back/foward cache working
  • ✅ Safari: back/foward cache working
  • ❌ Chromium: back/foward cache disabled

So it is a non-standard Chromium behavior, and globally the implementation is buggy, so I am not sure the library will be able to fix this.

If anyone wants to investigate, be sure to launch your Angular project with ng serve --live-reload false, otherwise there is a WebSocket for live reload, and WebSockets also disable back/forward cache.

@cyrilletuzi cyrilletuzi changed the title Performance: back/forward cache disabled with an open IndexedDB connection in Chrome Performance: back/forward cache disabled with an open IndexedDB connection in Chromium Apr 14, 2023
@naveedahmed1
Copy link
Author

Thank you so much @cyrilletuzi for your time and looking into this and sharing your findings. Yes I agree, but I am surprised that if its inconsistent across browsers, why have the Lighthouse team included it in latest version?

I have one question, can you please share what impact does the below workaround have on Angular app, when using this plugin:

    const open = window.indexedDB.open.bind(window.indexedDB)
    const dbs = []

    window.indexedDB.open = (...args) => {
      const request = open(...args)

      request.addEventListener('success', event => {
        const db = event.target.result
        dbs.push(db)
      })

      return request
    }

    window.addEventListener(
      'pagehide',
      () => {
        for (const db of dbs) {
          db.close()
        }
      },
      { capture: true },
    )

@cyrilletuzi
Copy link
Owner

why have the Lighthouse team included it in latest version?

Lighthouse is a part of Google Chrome, and its tests are opinionated. It could be an idea to report an issue in the Lighthouse repo to ask their reasoning, and what would be a cross-browser compatible solution.

can you please share what impact does the below workaround have on Angular app, when using this plugin

A console.log inside the loop of the pagehide event should be enough to see if it works or not. But be sure to test the behavior in all browsers. The last part your code could be implemented in the lib, but one other reason I said it may not be a good idea is because someone in the Firebase thread tell this:

On Safari for iOS v15.5, bfcache will sort of work regardless of closing IndexedDB connections or running deleteApp(), but there's a big problem: if you call deleteApp() on pagehide, when you come back to that page and it's restored from bfcache, a newly initialized Firestore instance will just not work anymore (e.g. if you add a document listener, onNext callback won't be called, neither with data from cache nor from network). Probably due to an incomplete cleanup. Now, if I let the page be hidden by Safari without calling deleteApp() and then call it on pageshow right before reinitializing Firebase, then it works. This is a serious bug and I think it should elevate the priority of this issue.

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

No branches or pull requests

2 participants