Cache-bust dynamic configuration files and theme CSS - #4936
Conversation
DSpace Angular's production server can modify the configuration it serves to CSR-only clients through YAML or environment variables. However, these files can remain cached in the browser and leave users with out-of-date configuration until the TTL runs out or the user does a "hard refresh". On build time this sort of problem is solved by saving the content hash as part of the path of each file (JS, CSS, ...) We introduce HashedFileMapping to bridge the same gap for dynamic content generated _after_ the server is built: - Files added to this mapping on the server are copied to a hashed path - A copy is injected into index.html, where clients can read it out and resolve the hashed paths - If a given path is not found in the mapping, the client will fall back to the original version (to prevent errors in development mode) With this mechanism we can ensure updates to config.json and similar files take immediate effect without losing the performance benefit of client-side caching.
The JSON mapping needs to be declared as a data block, otherwise browers may complain (nothing seems to really break though, except for Cypress) See https://developer.mozilla.org/en-US/docs/Web/HTML/Element/script#any_other_value
Mainly for 8.x and beyond, where ThemeService is injected more often ~ standalone components
Both the configuration and theme CSS are known to be needed for every render. We can save some time by retrieving them as soon as the HTML is read.
This didn't work well in one specific case: requests without a path (e.g. https://demo.dspace.org) Such requests would result in a 304 redirect directly to index.html, losing the hashed file mapping and causing it to get cached. Then it could remain in the cache across rebuilds. Solutions: - Don't try to hash index.html, but modify it in place - Introduce configuration to disable caching for specific static files and apply this to index.html to prevent similar problems - Don't let browsers cache index.html when it's served for CSR under another path
- Track head links in a Map, because Set doesn't deduplicate them properly - Make ServerHashedFileMapping independent of ThemeConfig - Make theme pre-fetching configurable (e.g. only prefetch the main theme CSS, but leave route-based theme CSS lazy)
f994389 to
ebb4c0e
Compare
alexandrevryghem
added
bug
performance / caching
|
The only thing which I cannot reproduce to work are the CSS changes due to wrongly resolved link to stylesheet. I hope somebody could double check it. This might have something to do with ThemeService. Reproduction steps: I use docker for development, Dockerfile.dist builds for production and run with PM2 supervisor(I run 1 instance like previous PR suggests).
$ docker compose -f docker/docker-compose-dist.yml build --no-cache
Environment:
Checklist: Tests: Shared:
It took me a bit to understand mechanism. Case 1. Swap from theme X to theme Y This works just fine. Case 2. Modify CSS for the same theme In my case I changed in <link rel="prefetch" as="style" href="custom-theme.c96c2175297734122eafc46bb0ffbcf3.css" class="hfm">
<link rel="stylesheet" type="text/css" class="theme-css" href="custom-theme.css">The same behaviour is with default theme. SSR:
I have checked with network tab and differences between index.html builds. The index.html and config.json changes occurs just fine. CSR:
I have checked with network tab and differences between index.html builds. The index.html and config.json changes occurs just fine. |
tdonohue
moved this from 🙋 Needs Reviewers Assigned
to 👀 Under Review
in DSpace 10.0 Release
…bust-dynamic-configuration_contribute-main
ebb4c0e to
c74daef
Compare
…nfiguration_contribute-main
There was a problem hiding this comment.
I did test the functionality but not review the code:
config.<hash>.jsonfile is correctly generated ✅config.<hash>.jsonis correctly referenced ✅<xxx>-theme.<hash>.cssfiles are correctly generated ✅<xxx>-theme.<hash>.cssfiles are correctly referenced ✅- I tried with one and two custom themes, works both ✅
index.htmlcontains the correct hashed-file-mappings and an additional<script id="hashed-file-mapping”section. ✅- New versions of the hashed files are generated when I make changes ✅
- Everything is correctly served to the browser, and that the app runs/looks as before the PR ✅
- I am not sure how to test “when SSR is bypassed” - what exactly does that mean? How do I make sure SSR is bypassed? ❓
|
Hi @AbhinavS96, |
|
@AbhinavS96 or @ybnd : Could one of you rebase this on latest |
…nfiguration_contribute-main
|
@tdonohue that was a quick one, just some dependencies shifting in the lock file |
tdonohue
left a comment
tdonohue
left a comment
There was a problem hiding this comment.
👍 Thanks @AbhinavS96 and @ybnd ! I tested and reviewed this today.
I've verified that hashed files are being created & loaded properly (so #1961 is fixed). I couldn't find any differences in UI behavior.
I also verified that #3992 is fixed by running the app in Production, stopping it, changing a config in my config.prod.yml, restarting it, and clicking the reload button on the page. In that scenario the updates to my config.prod.yml resulted in a newly hashed config.json and the new settings loaded immediately. (Previously I'd have to do a hard reload for settings to refresh properly in the browser, so this is fixed!)
One final note is that I'm adding the "needs documentation" label because the two new configs in this PR (noCacheFiles and prefetch) need to be added to the documentation at https://wiki.lyrasis.org/display/DSDOC10x/User+Interface+Configuration Please let me know once you've made those minor updates and we'll remove that label.
tdonohue
added
needs documentation
github-project-automation
Bot
moved this from 👍 Reviewer Approved
to ✅ Done
in DSpace 10.0 Release
|
Backport failed for Please cherry-pick the changes locally and resolve any conflicts. git fetch origin dspace-9_x
git worktree add -d .worktree/backport-4936-to-dspace-9_x origin/dspace-9_x
cd .worktree/backport-4936-to-dspace-9_x
git switch --create backport-4936-to-dspace-9_x
git cherry-pick -x d336d8336178dcd4a5d822df188f2d47aca73599 fdc6e8aaec25ab11f9cee3bcb93e65ce1415e907 2f235991057154531f6d244e4b5df59b17078dd6 ce4dd3aa3254be96a8e28b1d5090b2cdbce001b5 928e932b06eb1eb78364ebd940ae7d36430ac3aa c6bf2b7355c3c7ba493c3b2688709a9125740517 93da6ce34cded5b29c4e7dcb0aa1f50fbb2eb87f d2458e6f95daec73e0c617799c7c6f761ebd983e |
|
This should be ported to all active branches. In progress ports include: |
tdonohue
added
the
port to dspace-8_x
tdonohue
changed the title
|
@AbhinavS96 and @ybnd : Reminder, this PR still requires documentation.
|
tdonohue
removed
port to dspace-8_x
|
Thanks @ybnd ! Docs look good |
tdonohue
removed
the
needs documentation
7 participants
References
Fixes #1961
Fixes #3992
DSpace 10 port of #3993
Description
See original PR
Instructions for Reviewers
To test:
Run DSpace Angular locally, in production mode
Confirm that you see the following files:
[dist]/assets/config..json → hashed configuration file
Confirm that Angular runs with stale configuration when SSR is bypassed #3992 is resolved
Checklist
mainbranch of code (unless it is a backport or is fixing an issue specific to an older branch).npm run lintnpm run check-circ-deps)package.json), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation.