-
Notifications
You must be signed in to change notification settings - Fork 114
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
Add TCCP htmx extensions to handle cache busting and filter tracking #8266
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You might sometimes see htmx:historyCacheError errors in your console.
Indeed I do see this. It looks like HTMX saves the full page content into the history. According to this browsers can store up to 5 MiB per origin. Some of our pages can be more than 1MB (is this due to the repeated large SVGs for the dollar sign icons when we fetch 100s of cards into the table? is there some way to optimize this?) and so this fills up pretty quickly.
Is it worth setting hx-history
to false
to prevent the page content from being stored in cache? The URLs should still be, it's just that each page will need to be re-requested from the server (although they'll be cached in Akamai at least).
Adds an htmx extension that adds an `htmx` query param to URLs immediately before htmx makes a request and then removes it before pushing it to the browser's history. This allows endpoints fetched by htmx via AJAX to have a URL that is different from their host page's URL, reducing CDN caching mistaken identity bugs. Also adds an htmx extension store's the filter page's pathname to web storage so that our breadcrumbs can later retrive it.
Co-authored-by: Andy Chosak <[email protected]>
Works for me (c716050). Akamai edge caching should take care of everything. Our page does have a lot of HTML when there are 400+ cards shown to the user but it's still smaller than other cf.gov pages (e.g. iRegs). If you're looking at the size of network requests, keep in mind that localhost isn't compressing anything so the same request on beta should be ten times smaller when gzip'ed. If page performance becomes an issue we can load ten results on page load and have the "show more" button dynamically fetch more. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you're looking at the size of network requests, keep in mind that localhost isn't compressing anything so the same request on beta should be ten times smaller when gzip'ed.
@contolini thanks; I was looking at the size of the data in local storage, which doesn't get stored compressed:
Disabling the history works, though!
Thanks for adding Cypress tests - let's follow up in a subsequent PR to add test data for the detail pages.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Working beautifully!
Adds two htmx extensions: One that adds an
htmx
query param to URLs immediately before htmx makes a request and then removes it before pushing it to the browser's history. This allows endpoints fetched via AJAX to have a URL that is different from their host page's URL, (hopefully) solving our Akamai issues. The second extension stores the filter page'spathname to web storage so that our breadcrumbs can later retrieve it.
I also added a few cypress tests to detect future regressions.
How to test this PR
./frontend.sh
htmx=true
query params but the page's URL does not.sessionStorage.getItem('tccp-filter-path')
into the browser console and you'll see the most recent page's pathname with query parameters (to be used for breadcrumbs).yarn cypress run --spec test/cypress/integration/consumer-tools/credit-cards/explore-cards.cy.js
Screenshots
Notes and todos
htmx:historyCacheError
errors in your console. This happens when htmx's caching logic exceeds the browser's local storage limit. We don't have any control over this error and it's unrelated to this PR.Checklist
/docs
folder) – for basic, close-to-the-code docs on working with this repoFront-end testing
Browser testing
Visually tested in the following supported browsers:
Accessibility
Other