[Stack Monitoring] Ensure GlobalState class has it's destroy() method called on unmount

[Kerry350]

Summary

This fixes #135136 (and other related problems, e.g. #139231 (comment)).

Whilst the GlobalState class (which syncs to the URL) had a destroy() method, it wasn't being called. This is now called on unmount of the top-level <GlobalStateProvider />.

Testing

I was able to consistently replicate the bug by doing the following:

1. Navigate to Discover
2. Navigate to Stack Monitoring
3. Navigate back to Discover
4. Interact with the "Refresh every" toggle of the timepicker (either turn it off or on, it's the change of the refresh that matters)

This should throw a routing error without this PR.

[elasticmachine]

Pinging @elastic/infra-monitoring-ui (Team:Infra Monitoring UI)

[Dosant]

Thanks for fixing this!

While investigating this here #139231 (comment), I also noticed that this component creates GlobalState as a local variable during rendering instead of creating it once during mount:

kibana/x-pack/plugins/monitoring/public/application/contexts/global_state_context.tsx

Line 46 in 5bf5258

const state = new GlobalState(query, toasts, localState as { [key: string]: unknown });

I think this also could be a problem as this can be called multiple times, and we clean up only the last state when the component unmounts.

Should we put this state into useRef? Or into the useState?

[Kerry350]

@Dosant

I think this also could be a problem as this can be called multiple times, and we clean up only the last state when the component unmounts.

Yeah, that's a good point. Technically this component should only mount and render once given the Provider hierarchy. But it is susceptible to a subtle change somewhere upstream causing re-renders.

I've moved the instantiation into a useState call to keep the integrity between renders, if they were to happen.

I did also look at cleaning up localState too (the Angular > React conversion resulted in some code that wasn't idiomatic), but the object gets manipulated with raw property updates all throughout the codebase, so it's not a trivial change. Whilst not ideal if there were to be a re-render this one just leads to a small performance hit to recreate the objects rather than subscription leakage. Given the Platform Observability project now has priority over SM I think we can leave this bit as is.

[kibana-ci]

💚 Build Succeeded

• Buildkite Build
• Commit: f4dd576

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
monitoring	504	506	+2

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
monitoring	478.0KB	478.5KB	+449.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
monitoring	23.8KB	23.8KB	+49.0B

History

• 💚 Build #69430 succeeded bae9d2c
• 💚 Build #69013 succeeded 5bf5258

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @Kerry350

[Dosant]

Thanks!

[kibanamachine]

💚 All backports created successfully

Status	Branch	Result
✅	8.4

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation