Variables: Notify scene after each variable completion or value change

[torkelo]

Removes the batching of the notifications so they happen right away after a variable update is completed or value is changed. This requires a breaking chance to SceneVariableDependencyConfigLike but I think it's worth it.

Proper fix for #524

PR changes:

• Makes scene updates more efficient as parts of the scene now get notified right away when a variable is changed instead of having to wait for all variables. There is an increased cost as this requires more calls to sceneGraph.hasVariableDependencyInLoadingState to check if any other variable dependency is still loading but it's worth it I think.
• VariableDependencyConfig now handles the "waitingForVariables" state logic

So the main flow after this is this

SceneVariableSet will always notify the scene after each variable completes an update process and / or when the variable changes value. This is done in the same callback (and in case a loading variable changes value after loading is complete this is done in a single call)

VariableDependencyLike interface

 variableUpdateCompleted(variable: SceneVariable, hasChanged: boolean): void;

Scene objects care about two updates, when a variable that they depend on directly has changed or when a variable has completed an update process (no matter if it changed or not). This is because sceneGraph.hasVariableDependencyInLoadingState(X) returns true if a variable or its dependency is in a loading state. SceneQueryRunner will wait to issue the query until that function returns false. But to check again the SceneQueryRunner will need to know whenever a variable completes it's "loading/value validation" phase.

This is not different from how it works now, only difference is that we now check this after each variable completes.

VariableDependencyConfig (the default implementation) will take care of checking whether the changed variable is a dependency (so in it's callback the hasChanged boolean is now named dependencyChanged). In #526 I also expand the role of VariableDependencyConfig to handle the "isWaitingForVariables" state so that it is not duplicated and spread in multiple places.

• This causes VizPanel to re-render for each of its variable dependencies as they change, we could have an option in VariableDependencyConfig to batch notifications together after the last dependency in a loading state is completed. But I think we can leave this optimization for the future.

Release notes

VariableDependencyConfigLike interface has changed so that scene objects now get notified after each variable update is completed (or changed value). Before, the SceneVariableSet waited for all variables to complete before notifying scene objects.

The function variableUpdatesCompleted has changed name and signature:

variableUpdateCompleted(variable: SceneVariable, hasChanged: boolean): void;

VariableDependencyConfig has also some breaking changes. The function named onVariableUpdatesCompleted has changed name and signature to:

 onVariableUpdateCompleted?: () => void;

VariableDependencyConfig now handles the state logic for "waitingForVariables". If you call VariableDependencyConfig.hasDependencyInLoadingState and it returns true it will remember this waiting state and call onVariableUpdateCompleted as soon as the next variable update is completed, no matter if that variable is a dependency or if it changed or not.

[torkelo]

@dprokop added some more description to the flow of the change

[dprokop]

This feels wrong or hard to reason about. Won't this trigger queries for individual variable changes?

[dprokop]

All right, I get it now, we also check whethere or not there are any variables pending in the runWithTimeRange call. Think we need to document the Variables/SQR flow in some readme as it's already quite complex and certain assumptions are not documented anywhere but in the PRs. WDYT?

[torkelo]

@dprokop added some docs and release notes

[dprokop]

Would wait for @domasx2 review before landing this, ok?

[torkelo]

@domasx2 had a chance to look at this? this should fix your issue

[domasx2]

Hey! Unfortunately I refactored out this use case in app o11y, so no longer have something to test against

[grafanabot]

🚀 PR was released in v2.0.0 🚀