Sets variable query refId manually#47
Conversation
This ensures that variable queries have their `refId` set, which is required for Grafana to associate the response with the request. I can't seem to find documentation about this, but it's reflected in other custom variable support implementations: https://github.com/grafana/grafana/blob/15293a2ceb083108c0f15490933db523aa915c7d/public/app/plugins/datasource/loki/components/VariableQueryEditor.tsx#L71
|
Levitate is-compatible report: 🔍 Resolving @grafana/data@latest... 🔬 Checking compatibility between ./src/module.ts and @grafana/data@12.0.2... 🔬 Checking compatibility between ./src/module.ts and @grafana/ui@12.0.2... 🔬 Checking compatibility between ./src/module.ts and @grafana/runtime@12.0.2... 🔬 Checking compatibility between ./src/module.ts and @grafana/schema@12.0.2... 🔬 Checking compatibility between ./src/module.ts and @grafana/e2e-selectors@12.0.2... 🔬 Checking compatibility between ./src/module.ts and @grafana/experimental@2.1.6... ✔️ ./src/module.ts appears to be compatible with @grafana/data,@grafana/ui,@grafana/runtime,@grafana/schema,@grafana/e2e-selectors,@grafana/experimental |
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a missing refId on variable queries so that variable options populate correctly and refactors the variable support into a dedicated class.
- Sets a constant
refIdon all variable query edits and on blur events to associate responses. - Moves
HaystackVariableSupportout ofdatasource.tsinto its own file with a cleaner implementation. - Updates the DataSource constructor to wire up the new
HaystackVariableSupport.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/datasource.ts | Replaced inline variable support with new class import and constructor wiring |
| src/components/VariableQueryEditor.tsx | Added refId handling on change and blur; renamed exported component |
| src/HaystackVariableSupport.ts | New file: encapsulates variable query logic from datasource.ts |
Comments suppressed due to low confidence (4)
src/components/VariableQueryEditor.tsx:13
- [nitpick] The file name
VariableQueryEditor.tsxdoes not match the exported component nameHaystackVariableQueryEditor. Consider renaming the file toHaystackVariableQueryEditor.tsxor aligning the component name with the file name for consistency.
export const HaystackVariableQueryEditor = ({ onChange, query }: Props) => {
src/HaystackVariableSupport.ts:26
- The new
querylogic inHaystackVariableSupportprocesses frame data and handlesisRefconversions—consider adding unit tests to cover both string and ref scenarios to ensure correct behavior.
query(request: DataQueryRequest<HaystackVariableQuery>): Observable<DataQueryResponse> {
src/HaystackVariableSupport.ts:19
- Check the generic parameters passed to
CustomVariableSupport. The previous implementation included more type arguments for query and options—ensure this signature correctly enforces type safety for variable queries.
export class HaystackVariableSupport extends CustomVariableSupport<DataSource, HaystackVariableQuery> {
src/components/VariableQueryEditor.tsx:11
- [nitpick] Using a constant refId for all variable queries may lead to collisions when multiple variables are evaluated. Consider generating a unique refId per instance or parameterizing it to avoid duplicates.
const refId = 'HaystackVariableQueryEditor-VariableQuery';
This reworks the HaystackVariableSupport to align with the Loki plugin's use of CustomVariableSupport: https://github.com/grafana/grafana/blob/15293a2ceb083108c0f15490933db523aa915c7d/public/app/plugins/datasource/loki/LokiVariableSupport.ts#L10 Since I didn't really use any source example when implementing it initially, it ended up being kinda messy.
3218f3d to
03ad6cc
Compare
This fixes an issue where the Variable page was not populating the variable options. Although a correct response was received, since the request was missing the
refId, the response was not associated with the request.It also refactors the variable support to a cleaner implementation.
This resolves #46