[TSVB] Fixing memory leak

[majagrubic]

Summary

I was playing around with TSVB visualization to answer a Discuss question and noticed that sometimes Chrome freezes entirely when TSVB is open, which led me to believe there's a memory leak somewhere. I am not 100% sure it was caused by lodash set, but after removing it, I couldn't reproduce the freezing of Chrome and thought I'd open this for review, in case you think it's beneficial.

Checklist

Delete any items that are not applicable to this PR.

- [ ] Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
- [ ] Documentation was added for features that require explanation or tutorials

• Unit or functional tests were updated or added to match the most common scenarios
  - [ ] This was checked for keyboard-only and screenreader accessibility
  - [ ] This renders correctly on smaller devices using a responsive layout. (You can test this in your browser
  - [ ] This was checked for cross-browser compatibility, including a check against IE11

For maintainers

• This was checked for breaking API changes and was labeled appropriately

[legrego]

LGTM on green CI - tested different combinations of TSVB visualizations, and I'm not noticing any issues

[elasticmachine]

Pinging @elastic/kibana-app (Team:KibanaApp)

[flash1293]

I tested TSVB and did a sanity check of all the functions touched in this PR. I couldn't notice any breakage.

I reviewed the code, but it's hard to tell whether there's another place like https://github.com/elastic/kibana/pull/64918/files#diff-1accfc2b04681b275b52ae76e6ddd9b1R39 due to the lack of types. Also, I had a hard time parsing the logic - so many aggs :D

From my understanding the difference in behavior between lodash and set-value is this:

let a = { deep: { nested: { path: { b: '123' } } } }
set(a, 'deep.nested.path', { c: '456' }); // { deep: { nested: { path: { b: '123', c: '456' } } } }
_.set(a, 'deep.nested.path', { c: '456' }); // { deep: { nested: { path: { c: '456' } } } }

We could do something like this:

import set from 'set-value';

export overwrite(obj, path, val) {
  set(obj, path, undefined);
  set(obj, path, val);
}

and then use overwrite instead of set directly in all the places you touched. What do you think @majagrubic ? Seems like a small adjustment and then we can be safe(r) no logic is breaking.

[kibanamachine]

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request
• Commit: 53b3ca8

History

• 💚 Build #44548 succeeded 99b0fb8
• 💚 Build #44483 succeeded 3e11339

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

[legrego]

Tested updated implementation, still LGTM!