grafana / scenes

Build Grafana dashboards directly in your Grafana app plugins.
https://grafana.com/developers/scenes
Apache License 2.0
133 stars 20 forks source link

AdHocFiltersVariable: Fixes issue updating hide state causing variable to be deactivated and preventing it from being shown again #679

Closed torkelo closed 5 months ago

torkelo commented 5 months ago

While troubleshooting the problem of clearing adhoc filters and setting hide: VariableHide.hideVariable and then adding filters and setting variable hide back to Variable.dontHide I discovered some fundamental issues with useState and AdHocFiltersVariable

Problem with useState.

Solution:

Problem with AdHocFiltersVariable

Example.

  1. setState (update filters) (state update A)
  2. State change handler 1 triggers for update A (this is its own handler), it causes another setState with updated filterExpression (state update B)
  3. State change handler 1 triggers for update B (does nothing as filters has not changed only filterExpression)
  4. State change handler 2 triggers for update B (for some other component that subscribes to state)
  5. State change handler 2 triggers for update A (for some other component that subscribes to state)

As you can see the other component subscribing to the variable state will get the first state change (with old filterExpression) as the last value. Could be fixed with updating filterExpresion inside a setTimeout but opted to update filterExpression inside an overridden setState function as this eliminates two state updates when updating filters.

Release notes

<SceneObject>.useState will now cause the scene object to be activated (if it was not already). When the component that calls <sceneObject>.useState unmounts it will deactivate the scene object if it was the last component to call useState on this instance.

The same is not true for <SceneObject>.subscribeToState, this function still does not cause the source object to be activated.

📦 Published PR as canary version: 4.4.2--canary.679.8556229462.0
:sparkles: Test out this PR locally via: ```bash npm install @grafana/scenes@4.4.2--canary.679.8556229462.0 # or yarn add @grafana/scenes@4.4.2--canary.679.8556229462.0 ```
grafanabot commented 5 months ago

:rocket: PR was released in v4.5.5 :rocket: