-
Notifications
You must be signed in to change notification settings - Fork 32
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
Fix broken livereload injection when using Grafana 10.2.3 #696
Fix broken livereload injection when using Grafana 10.2.3 #696
Conversation
I needed this version of Grafana to use the latest features in my scenesapp plugin. But bumping to this version of Grafana broke the livereload injection. I haven't tracked down exactly when/how/why the old injection command no longer works, but I have found this fix. If you're happy with it, maybe we can just merge this change, without bothering to find the cause of the issue? Or maybe you can help by tracking down the cause (or help me to do so)? Thanks
I guessed at the |
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.
@samjewell thanks so much for taking the time to put a PR together to fix this. 🏅
I haven't tracked down exactly when/how/why the old injection command
no longer works, but I have found this fix.
Great spot! The reason is due to changes in the Grafana codebase which now doesn't minify the html template in the same way which prevents the regex in the sed command from matching.
@jackw final review, so we can merge this? 🙏 |
@samjewell apologies, I'll get to it when I can. I need to test it across Grafana versions to make sure it doesn't cause it to break in earlier versions of Grafana. |
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.
LGTM! 🚀
Tested across 10.0.x, 10.1.x, and 10.2.x
🚀 PR was released in |
I needed this version of Grafana to use the latest features in my
scenesapp plugin. But bumping to this version of Grafana broke the
livereload injection.
I haven't tracked down exactly when/how/why the old injection command
no longer works, but I have found this fix. If you're happy with it,
maybe we can just merge this change, without bothering to find the
cause of the issue? Or maybe you can help by tracking down the cause
(or help me to do so)?
Thanks
Also bumping the Grafana version here, so you can see this in action. But happy to revert the Grafana version and just keep the livereload change, if we need a smaller scope to get this merged.📦 Published PR as canary version:
Canary Versions
✨ Test out this PR locally via:
npm install @grafana/create-plugin@3.1.2-canary.696.20ca811.0 # or yarn add @grafana/create-plugin@3.1.2-canary.696.20ca811.0