[data view editor] Form state refactor - create service, replace useEffect with observables

[mattkime]

Summary

• Created service which is passed into flyout like a plugin dependency, sidestepping react state concerns
• Code is rewritten to use observables instead of react state / useEffect
  • There's just one useEffect function
  • All useState has been replaced by useRef wrapped Subject and BehaviorSubject objects
• Fixes UI updates on 'Show hidden indices' toggle
• Fixes index name highlighting when index pattern has multiple (comma delineated) segments
• Fixes data view edit use case
• All network requests are debounced

Note: I attempted further refactoring but ran into problems. These changes are sufficient to provide a number of bug fixes.

Follow up to and in support of #142362

[mattkime]

@Dosant I've modified the code to eliminate the possibility of stale data between flyout invocations. I saw the timestamp field being populated without an index pattern - this has been fixed.

I've reduced the number of indices requests but I don't think I'll address the fields_for_wildcard as there's some unfortunate code thats tied to validation - that I hope to clean up in a subsequent PR.

[ppisljar]

I've reduced the number of indices requests but I don't think I'll address the fields_for_wildcard as there's some unfortunate code thats tied to validation - that I hope to clean up in a subsequent PR.

If the consequence of this PR are additional (not needed) requests and thus reduced performance then we can either address the issue in this PR or merge this PR into a feature branch and resolve the issue in the followup and once its resolved we can merge the feature branch into main.

[Dosant]

I think I don't understand why we wrap this into debounce. since this function would be called only once - I guess there is no need to debounce it.

Was the intention to make the cache live for 60 seconds? Then I think it doesn't work this way and instead cache is alive for as long as the instance of the service is alive. Because with every getIndicesMemory call we get the cached promise from memory instead of calling the debounced function again

[mattkime]

ah, good catch! No, debounce isn't needed. I've removed it. I think its okay for us to cache the response for the life of the flyout.

[Dosant]

Ah, Sounds good then 👍

Should we clean up the cache in case of an error?

[mattkime]

Should we clean up the cache in case of an error?

Definitely.

[Dosant]

This probably won't be visible for the end user, but because of this await, we'd have a delay between putting the promise into the cache and accessing the cache.

This code could be read like this:

if (!this.getIndicesMemory[key]) {
const isRollup = await this.getIsRollupIndex(); // <---- breaking the sync execution. gives a chance for other code to run "in parallel" 
this.getIndicesMemory[key] = this.dataViews.getIndices({ ...props, isRollupIndex: isRollup });
}

this means that there is a moment when getIndicesMemory can be called again and there is no promise yet in the cahche and then we do a redundant request.

this looks uglier, but probably can be fixed like this:

if (!this.getIndicesMemory[key]) {
this.getIndicesMemory[key] = this.getIsRollupIndex().then(isRollup => this.dataViews.getIndices({ ...props, isRollupIndex: isRollup }))
}

[Dosant]

we can also leave it as is, if we think this isn't a problem at all

[mattkime]

Practically speaking I think the code is fine but I like the formally correct solution you've proposed.

[Dosant]

Recent code changes lgtm.

I tried to smoke check and noticed that I can't edit -> save data view anymore. I couldn't reproduce in main:

can.t.save.mov

[mattkime]

@Dosant Addressed and added test

[kibana-ci]

💚 Build Succeeded

• Buildkite Build
• Commit: c1d4964

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
dataViewEditor	60	63	+3

Async chunks

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

id	before	after	diff
dataViewEditor	31.2KB	34.7KB	+3.5KB

Page load bundle

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

id	before	after	diff
dataViewEditor	10.4KB	10.5KB	+97.0B

Unknown metric groups

ESLint disabled in files

id	before	after	diff
dataViewEditor	3	1	-2

ESLint disabled line counts

id	before	after	diff
dataViewEditor	4	2	-2

Total ESLint disabled count

id	before	after	diff
dataViewEditor	7	3	-4

History

• 💚 Build #80870 succeeded c00ef54
• 💛 Build #80853 was flaky f109e9e
• 💔 Build #80795 failed 6f1ef8c
• 💚 Build #80596 succeeded 0625a98
• 💚 Build #80317 succeeded 245559d
• 💚 Build #79925 succeeded 1a44ee1

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

[Dosant]

Tested, seemed to be working well :)