-
-
Notifications
You must be signed in to change notification settings - Fork 3.9k
Add must-revalidate to Cache-Control header and use WEB_BUILD_TIME for ETag to fix stale UI after firmware updates #5120
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
Conversation
|
Important Review skippedBot user detected. To trigger a single review, invoke the You can disable this status message by setting the Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. Comment |
…tion Co-authored-by: DedeHai <[email protected]>
…tion Co-authored-by: DedeHai <[email protected]>
Co-authored-by: DedeHai <[email protected]>
|
@copilot: since VERSION is static in wled.h but should actually be the build date: can this be updated at compile time in cdata.js? |
The VERSION is currently a manual release identifier in An alternative approach: Should I explore implementing this in a follow-up PR, or would you prefer a different approach? |
|
@copilot since html_ui.h is already being dynamically generated: add WEB_BUILD_TIME to that header and use WEB_BUILD_TIME instead of VERSION in the ETag mechanism. |
No, it shouldn't. If I were to build WLED 0.12 today, it should still have the original version number from 2021. It is more of a "codebase recency date" than a build date. Also, multiple people building different branches/versions on the same day would have the same version, which is very bad.
This is an awesome idea and I think it is fine to use that to automate the UI caching on new builds. |
Co-authored-by: DedeHai <[email protected]>
Implemented in 54f650a. |
Available in my fork for quite some time now. Not using cdata.js but set_version.py |
|
@copilot 4-hour Build slots make no sense. This does not have to use the same format as version. Please revise to using |
Co-authored-by: Aircoookie <[email protected]>
|
@copilot since this is for browser validation only, it is not required ot be human readable. To save on flash, make the #define a integer timestamp in UNIX time, we do not care about the 2038 rollover for validation. Then just printf that integer in in the ETag mechanism |
Co-authored-by: DedeHai <[email protected]>
Updated in 714a7f8. |
|
@copilot Does our webserver actually implement status 304 or are we just created etags that are then not referenced? |
Yes, the webserver implements 304. The |
willmmiles
left a comment
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.
After cleanup, this should get squashed down to one commit to reduce the history spam.
|
@copilot please fix the requested review changes. |
Co-authored-by: DedeHai <[email protected]>
Add WEB_BUILD_TIME to html_ui.h and use it for ETag generation Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: DedeHai <[email protected]> Co-authored-by: Aircoookie <[email protected]>
Understand the issue: UI loads from browser cache after firmware update
Analyze current caching implementation in
wled_server.cppBuild and test current codebase to establish baseline
Add
WEB_BUILD_TIMEgeneration tocdata.jsInclude
WEB_BUILD_TIMEin generatedhtml_ui.hUpdate ETag generation to use
WEB_BUILD_TIMEinstead ofVERSIONChange WEB_BUILD_TIME to UNIX timestamp integer for flash savings
Remove unnecessary
must-revalidatechange per reviewer feedbackRemove unnecessary
_codeql*gitignore entry per reviewer feedbackBuild and test changes (npm test passes - 16/16 tests)
Run hardware build to validate firmware compiles (esp32dev SUCCESS)
Fixes UI loads from browser cache even after an update #5119
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.