grafana / grafana-plugin-examples

181 stars 52 forks source link

app-with-extension-point: use the hooks instead of the getter #308

Open leventebalogh opened 4 months ago

leventebalogh commented 4 months ago

What changed?

Updated the example to use the usePluginExtensions() hook instead of the getter - as this is what we are suggesting to use now and we are moving away (deprecating) the getter.

Note for the reviewer Unfortunately there is no public release yet where @grafana/runtime would contain the hooks (11.0.0 still doesn't have them), so I had to use a canary version of the package, and the main docker image tag, I am not sure how much this is a problem.

@sympatheticmoose for visibility.

sunker commented 4 months ago

Currently all integrations tests run against canary and latest, so we'd need to add some kind of exception to that to get the tests for this example plugin to pass.

Maybe we should use APIs that are only available in canary releases in the example repo? Can this change wait until G11.1 is out the door?

leventebalogh commented 4 months ago

@sunker

Maybe we should use APIs that are only available in canary releases in the example repo? Can this change wait until G11.1 is out the door?

You mean that we shouldn't use APIs that are only available in canary releases in this repo? If yes, then I think you are right, we probably should only use features that are part of a public release. 🤔

Can this change wait until G11.1 is out the door?

Yes, I think it can (and probably it should). On the other hand we could also have a brainstorming about this, as it kind of "clashes" with our continuous deployment strategy on cloud, and can result in situations where our internal app developers would already need some examples on a certain feature while there no new public release cut.

sunker commented 4 months ago

Maybe we should use APIs that are only available in canary releases in the example repo? Can this change wait until G11.1 is out the door?

You mean that we shouldn't use APIs that are only available in canary releases in this repo? If yes, then I think you are right, we probably should only use features that are part of a public release. 🤔

Oh - looks like a there's a not missing in my comment. :)

On the other hand we could also have a brainstorming about this, as it kind of "clashes" with our continuous deployment strategy on cloud, and can result in situations where our internal app developers would already need some examples on a certain feature while there no new public release cut.

Yeah makes sense. Maybe a feature branch could suffice?

leventebalogh commented 4 months ago

Yeah makes sense. Maybe a feature branch could suffice?

You mean to have a dedicated branch besides main (e.g. latest or canary or something similar), that we would link to from our README(s), and which would always hold versions of the examples that hold the latest available features? I think that would be a great idea, and then we could also merge this every-now-and-again to main - possibly whenever there is a new grafana release?

sunker commented 4 months ago

Yep exactly! I think historically i Grafana, we've called these kind of branches next.

This would be yet one more thing to maintain and keep track of, but I think it will be manageable.

mckn commented 4 months ago

LGTM!

Given another thought, it might not be ready for a merge.

sunker commented 3 months ago

Can't we toggle the rendering based on Grafana version?

//App.ts
if (semver.gte(config.buildInfo.version, '11.0.0')) {
  //get extensions using new api
} else {
  //get extensions using legacy api
}