angular-async-local-storage icon indicating copy to clipboard operation
angular-async-local-storage copied to clipboard

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

Open naveedahmed1 opened this issue 2 years ago • 5 comments

Checks before posting an issue

  • [x] I have read all the library's documentation:
  • [x] I have checked in already:
  • [x] I understand I am writing to a human being, doing this open source project unpaid
  • [x] I will be involved in my own issue, by answering questions and informing if the issue is resolved

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.

naveedahmed1 avatar Apr 13 '23 17:04 naveedahmed1

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 avatar Apr 13 '23 20:04 cyrilletuzi

Same is the case with Edge, in bfcache issue on lighthouse repository https://github.com/GoogleChrome/lighthouse/issues/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 https://github.com/firebase/firebase-js-sdk/issues/6167

*Adding links for reference.

naveedahmed1 avatar Apr 13 '23 21:04 naveedahmed1

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 avatar Apr 14 '23 08:04 cyrilletuzi

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 },
    )

naveedahmed1 avatar Apr 15 '23 21:04 naveedahmed1

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.

cyrilletuzi avatar Apr 16 '23 09:04 cyrilletuzi