Skip to content
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

Merged
merged 3 commits into from
Feb 2, 2024

Conversation

samjewell
Copy link
Contributor

@samjewell samjewell commented Jan 25, 2024

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

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
@samjewell samjewell requested a review from a team as a code owner January 25, 2024 11:42
@samjewell samjewell requested review from jackw and removed request for a team January 25, 2024 11:42
@samjewell samjewell self-assigned this Jan 25, 2024
Copy link

github-actions bot commented Jan 25, 2024

Hello! 👋 This repository uses Auto for releasing packages using PR labels.

✨ This PR can be merged and will trigger a new patch release.
NOTE: When merging a PR with the release label please avoid merging another PR. For further information see here.

@samjewell samjewell added the patch Increment the patch version when merged label Jan 25, 2024
@samjewell
Copy link
Contributor Author

I guessed at the patch label on this PR. Please change if you'd like.

Copy link
Collaborator

@jackw jackw left a 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.

@samjewell samjewell requested a review from jackw January 25, 2024 15:52
@samjewell samjewell changed the title Fix broken livereload injection when using Grafana 10.2.3 (and bump Grafana version) Fix broken livereload injection when using Grafana 10.2.3 Jan 26, 2024
@jackw jackw added the release Create a release when this pr is merged label Jan 31, 2024
@samjewell
Copy link
Contributor Author

@jackw final review, so we can merge this? 🙏

@jackw
Copy link
Collaborator

jackw commented Feb 1, 2024

@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.

Copy link
Collaborator

@jackw jackw left a 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

@jackw jackw merged commit a465018 into main Feb 2, 2024
17 checks passed
@jackw jackw deleted the sj/update-grafana-version-and-fix-livereload-injection branch February 2, 2024 10:49
@grafana-plugins-platform-bot
Copy link

🚀 PR was released in @grafana/create-plugin@3.4.0 🚀

@grafana-plugins-platform-bot grafana-plugins-platform-bot bot added the released This issue/pull request has been released. label Feb 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
patch Increment the patch version when merged release Create a release when this pr is merged released This issue/pull request has been released.
Projects
Development

Successfully merging this pull request may close these issues.

2 participants