[data view field editor] Composite runtime field editor

[mattkime]

Summary

Implement composite runtime field editor in data view field editor.

Closes: #126247

Screen.Recording.2022-08-13.at.11.34.57.PM.mov

[Dosant]

@Dosant I've attempted to reproduce the problem you're seeing but thus far I've been unable to. If you're able to reproduce it at least sometimes , log from just afterbufferCount. Based on what I see here, I suspect a ChangeSet failed to apply although I have no idea how that might have happened. I'd pay attention to the rerendering of the field_editor.

This is how I was able to reproduce different state inconsistencies
I believe looking at those could in general, help to stabilize state management

When field preview request is still in progress, change to runtime field name leads to stale name in the preview and lose of "updating" state until request finishes

bug1.mov

Switching between composite and non-composite types could lead to stale composite keys, probably related to "resetting" fields (see how key7 is there in generated fields list at the end)

bug2.mov

[Dosant]

Since last review change LGTM 👍

I found this bug I didn't notice before while testing again:

• Have a regular runtime field saved
• Edit it and change it to composite. Edit the template
• Try to save - you'll see the subfields reset and won't allow you to save:

[Dosant]

So maybe lastPreview$ is not needed in the end? not sure exactly why it is needed. I believe in resetTypes we could just use fieldPreview.getValue() first
Also, useMemo is probably no the best choice here because it isn't guaranteed to be a stable reference and should be used just for perf optimizations

[mattkime]

@Dosant fieldPreview$ is a Subject rather than a BehaviorSubject, hence the conversion

[mattkime]

@Dosant I believe I resolved the items you brought up, mind taking another look?

[Dosant]

@Dosant fieldPreview$ is a Subject rather than a BehaviorSubject, hence the conversion

Would it be difficult to make it BehaviourSubject? Or would this not work?
I think we would improve these places then:

https://github.com/mattkime/kibana/blob/0cad9fefd8e44e3416e72bac92069b904f698f6d/src/plugins/data_view_field_editor/public/components/field_editor/field_editor.tsx#L161-L173

https://github.com/mattkime/kibana/blob/0cad9fefd8e44e3416e72bac92069b904f698f6d/src/plugins/data_view_field_editor/public/components/field_editor/field_editor.tsx#L175-L182

But if not making a BehaviourSubject, can we reuse this lastPreview in reset functionality? I think current useCallback usage is not right: it should be useRef or useState, but looks like we already have lastPreview in useRef that seems could work in reset also https://github.com/mattkime/kibana/blob/0cad9fefd8e44e3416e72bac92069b904f698f6d/src/plugins/data_view_field_editor/public/components/field_editor/field_editor.tsx#L175-L182

[mattkime]

@Dosant I was largely able to implement your suggestions. My only complaint is that I couldn't find a way to get the filter function to properly express that it was returning FieldPreview[] instead of FieldPreview | undefined

[kibana-ci]

💚 Build Succeeded

• Buildkite Build
• Commit: ff82723

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
dataViewFieldEditor	136	141	+5

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
dataViewFieldEditor	29	30	+1
dataViews	206	208	+2
total			+3

Async chunks

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

id	before	after	diff
dataViewFieldEditor	148.1KB	154.4KB	+6.3KB
dataViewManagement	145.8KB	147.3KB	+1.5KB
securitySolution	6.4MB	6.4MB	+34.0B
total			+7.8KB

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id	before	after	diff
dataViewFieldEditor	3	0	-3

Page load bundle

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

id	before	after	diff
dataViewFieldEditor	21.9KB	22.8KB	+887.0B
dataViews	42.9KB	42.9KB	+27.0B
total			+914.0B

Unknown metric groups

API count

id	before	after	diff
dataViewFieldEditor	49	60	+11
dataViews	963	966	+3
total			+14

ESLint disabled line counts

id	before	after	diff
dataViewFieldEditor	8	9	+1

Total ESLint disabled count

id	before	after	diff
dataViewFieldEditor	8	9	+1

History

• 💚 Build #71073 succeeded e260d2e
• 💔 Build #71072 failed 83b3324
• 💚 Build #70687 succeeded 0cad9fe
• 💚 Build #69916 succeeded f518638
• 💔 Build #69911 failed 8f4f16a
• 💔 Build #69900 failed 4479598

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

[Dosant]

@Dosant I was largely able to implement your suggestions. My only complaint is that I couldn't find a way to get the filter function to properly express that it was returning FieldPreview[] instead of FieldPreview | undefined

@mattkime, I think you can use type guards like this:

subject.pipe(
    filter((preview): preview is FieldPreview[] => preview !== undefined), // <---- see here
    ...
)