-
Notifications
You must be signed in to change notification settings - Fork 569
Make DebugKit toolbar robust with JS frameworks like HTMX, Hotwire, ... #1045
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
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.
Pull Request Overview
This PR enhances the DebugKit toolbar to work reliably with JavaScript frameworks like HTMX and Hotwire that perform partial page updates or DOM swaps. The changes address an issue where the toolbar iframe would be lost after navigation, causing "iframe.contentWindow is null" errors.
- Implements MutationObserver to automatically re-inject the toolbar iframe when it's removed from the DOM
- Updates the toolbar iframe's src with the latest debug kit ID from AJAX responses
- Prevents duplicate event listeners and XMLHttpRequest proxying to avoid memory leaks
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
This is now ready for testing / comments :-) |
- Persist toolbar iframe by placing it in document.documentElement instead of document.body. - Handle potential error by logging it to the console - Cleanups
|
I changed some things, realizing I overcomplicated it by adding the iframe to the shadow root, which I then moved outside document.body. The iframe alone outside of document.body is enough here. |
Is that a valid location for iframes? I tried reading through the HTML spec and iframes are part of 'embedded content' and I'm not sure that iframes can be under the Another solution could be to have debugkit render for these requests. We could expose it as a setting. I don't think we can start appending debugkit to all HTML responses, without an opt-in. |
Browsers are pretty forgiving about this but after some extra research it looks like this is indeed not really valid if you follow the html spec stricly. You can therefore currently ignore this MR.
Yeah i think a more dynamic approach would be better anyway. I will close this MR and do some thinking for a cleaner solution. |
This PR makes the toolbar work when used with libraries (such as HTMX, Hotwire, etc.) that swap or
replace the
<body>element or perform partial page updates.Improvements
The main error i experienced was
This error occured after transitioning to another page (e.g., with htmx:boost or similar techniques) because the toolbar iframe was lost or not properly re-injected.