Add caching and cache-busting to dapp icons #2697
Conversation
|
Will need to change the frontend checks because the logos are now in a different place. |
|
changed the checks in here but they're also in separate PRs, we should approve and merge the separate PRs before this one |
|
Added smooth transitions so the lazy-loaded images aren't jarring and everything looks slick. (The 'blinking' is me hitting cmd-r to reload to display the effect) Screen.Recording.2024-11-21.at.13.58.50.mov |
|
Ran the update dapps action with the new configuration, all runs well except the PR to main because of merge conflicts with main - this should be temporary though. 
https://github.com/dfinity/internet-identity/actions/runs/11954189450/job/33323727838 |
scripts/update-dapps
Outdated
| @@ -10,7 +10,7 @@ cd "$SCRIPTS_DIR/.." | |||
| # The path where we save the list of dapps in II | |||
| DAPPS_JSON="$PWD/src/frontend/src/flows/dappsExplorer/dapps.json" | |||
| # The path where we save logos in II | |||
| II_LOGO_PREFIX="src/frontend/assets/icons" | |||
| II_LOGO_PREFIX="src/frontend/assets/src/assets/icons" | |||
There was a problem hiding this comment.
Is this correct? Shouldn't it be frontend/src/assets/icons?
Is this also in another PR?
There was a problem hiding this comment.
jfc i thought i had corrected that
There was a problem hiding this comment.
oh wait it says outdated
There was a problem hiding this comment.
Yes I already corrected it. It's also in another PR, here: #2701
| @@ -5,7 +5,7 @@ | |||
| <meta http-equiv="X-UA-Compatible" content="IE=edge" /> | |||
| <meta name="viewport" content="width=device-width, initial-scale=1.0" /> | |||
| <title>Internet Identity</title> | |||
| <link rel="shortcut icon" href="/favicon.ico" /> | |||
| <link rel="shortcut icon" href="/src/assets/favicon.ico" /> | |||
There was a problem hiding this comment.
So we can set the cache-control header of the favicon and add a filehash to it so it too is cached. It's another 15kb and in my tests it could take quite a bit of time to load on some refreshes.
src/frontend/src/assets/favicon.ico
Outdated
There was a problem hiding this comment.
Do we need to move this?
There was a problem hiding this comment.
It's not 100% vital but it's another 15kb that was previously static and is now cached with a hash.
| import { features } from "$src/features"; | ||
| const iconFiles = import.meta.glob("../../assets/icons/*", { |
There was a problem hiding this comment.
Could you add a comment with the absolute path from the root directory? In case we ever move this file, to know where this relative path should be pointing to.
Corrected some errors in the comments.
Updated the location and then imports/loading of the dapp icons to use file hashes and the cacheable filename tag, so their cache-control headers are also set so 1 year but changing the file will reload them immediately.
Added a test that checks whether the header is set correctly (had to add a gzip decoder in the dev dependencies for that)
🟡 Some screens were changed