[Embeddable Rebuild] [Controls] Clean up services + TODOs #193180

Heenawter · 2024-09-17T14:40:48Z

Part of #192005
Closes #167438

Summary

This PR represents the second major cleanup task for the control group embeddable refactor. The tasks included in this PR can be loosely summarized as follows:

It removes the old, cumbersome services implementation and replaces it with a much simpler system (which is the same one used in the links + presentation_panel plugins).
- This it the main reason for the decrease in lines - the old system required a huge amount of boilerplate code, which is no longer necessary with the new method for storing services.
It addresses and/or removes any remaining TODO comments in the controls plugin
- This includes renaming ControlStyle and DEFAULT_CONTROL_STYLE to ControlLabelPosition and DEFAULT_CONTROL_LABEL_POSITION respectively, which represents a bulk of the changes.
It moves all compatibility checks for all control actions to be async imported.
It removes the ability to register controls from the controls plugin setup contract.

Checklist

Unit or functional tests were updated or added to match the most common scenarios

For maintainers

This was checked for breaking API changes and was labeled appropriately

…-fix'

Heenawter · 2024-09-17T20:43:28Z

src/plugins/controls/common/options_list/types.ts

@@ -15,8 +15,6 @@ import { OptionsListSortingType } from './suggestions_sorting';
 import { DefaultDataControlState } from '../types';
 import { OptionsListSearchTechnique } from './suggestions_searching';

-export const OPTIONS_LIST_CONTROL = 'optionsListControl'; // TODO: Replace with OPTIONS_LIST_CONTROL_TYPE


After #192017, this was not actually used - so removing it now.

Heenawter · 2024-09-17T20:54:43Z

src/plugins/controls/public/react_controls/control_group/get_control_group_factory.tsx

                  allowExpensiveQueries: boolean;
-                  // TODO: Rename this route as part of https://github.com/elastic/kibana/issues/174961
-                }>('/internal/controls/optionsList/getExpensiveQueriesSetting', {
+                }>('/internal/controls/getExpensiveQueriesSetting', {


I renamed this route since it's no longer specific to the options list - the control group now does the fetching.

Heenawter · 2024-09-17T20:57:41Z

src/plugins/controls/public/react_controls/controls/types.ts

-    // HasSerializableState types serializeState as function returning 'MaybePromise'
-    // Controls serializeState is sync
-    serializeState: () => SerializedPanelState<DefaultControlState>;
-    /** TODO: Make these non-public as part of https://github.com/elastic/kibana/issues/174961 */


I decided there is no reason to lock down the API at this point in time, especially since we are no longer allowing external plugins to register their own control types. It honestly felt like overcomplicating to split this into "public" and "private" APIs 😅 So I just removed this TODO

Heenawter · 2024-09-17T20:58:25Z

src/plugins/controls/public/react_controls/controls/types.ts

@@ -62,7 +62,6 @@ export type ControlApiInitialization<ControlApi extends DefaultControlApi = Defa
    'serializeState' | 'getTypeDisplayName' | 'clearSelections'
  >;

-// TODO: Move this to the Control plugin's setup contract


We decided to not allow registering controls via the Control plugin's setup contract, so I removed this TODO

Heenawter · 2024-09-17T21:00:27Z

...blic/react_controls/controls/data_controls/range_slider/get_range_slider_control_factory.tsx

@@ -113,14 +111,15 @@ export const getRangesliderControlFactory = (

      const dataLoadingSubscription = combineLatest([loadingMinMax$, loadingHasNoResults$])
        .pipe(
+          debounceTime(100),


I added a small debounce here to prevent the loading state from "flashing" as often as it did before.

elasticmachine · 2024-09-17T21:07:26Z

Pinging @elastic/kibana-presentation (Team:Presentation)

src/plugins/controls/public/plugin.ts

...blic/react_controls/controls/data_controls/range_slider/get_range_slider_control_factory.tsx

nreese

LGTM - thanks for cleaning up the services
code review only

…-services_2024-09-12

kibana-ci · 2024-09-18T20:13:13Z

💚 Build Succeeded

Buildkite Build
Commit: d4c4620

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`controls`	362	353	-9

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
`controls`	134	130	-4

Async chunks

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

id	before	after	diff
`controls`	459.4KB	455.7KB	-3.7KB

Page load bundle

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

id	before	after	diff
`controls`	12.9KB	13.3KB	+401.0B

Unknown metric groups

API count

id	before	after	diff
`controls`	138	134	-4

async chunk count

id	before	after	diff
`controls`	11	10	-1

References to deprecated APIs

id	before	after	diff
`controls`	17	13	-4

History

💛 Build #235303 was flaky ded4bef
💛 Build #234983 was flaky 7b609b8
💔 Build #234966 failed c71bfeb
💔 Build #234937 failed b87037e
💔 Build #234901 failed 7d4406c

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

cc @Heenawter

…3180) Part of elastic#192005 Closes elastic#167438 ## Summary ## Summary This PR represents the second major cleanup task for the control group embeddable refactor. The tasks included in this PR can be loosely summarized as follows: 1. It removes the old, cumbersome services implementation and replaces it with a much simpler system (which is the same one used in the `links` + `presentation_panel` plugins). - This it the main reason for the decrease in lines - the old system required a **huge** amount of boilerplate code, which is no longer necessary with the new method for storing services. 2. It addresses and/or removes any remaining TODO comments in the `controls` plugin - This includes renaming `ControlStyle` and `DEFAULT_CONTROL_STYLE` to `ControlLabelPosition` and `DEFAULT_CONTROL_LABEL_POSITION` respectively, which represents a bulk of the changes. 3. It moves all compatibility checks for all control actions to be async imported. 4. It removes the ability to register controls from the `controls` plugin setup contract. ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios ### For maintainers - [ ] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> (cherry picked from commit db55574)

kibanamachine · 2024-09-19T14:07:27Z

💚 All backports created successfully

Status	Branch	Result
✅	8.x

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

Questions ?

Please refer to the Backport tool documentation

) (#193429) # Backport This will backport the following commits from `main` to `8.x`: - [[Embeddable Rebuild] [Controls] Clean up services + TODOs (#193180)](#193180)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Hannah Mudge <Heenawter@users.noreply.github.com>

Heenawter added 7 commits September 17, 2024 08:39

Refactor the services make use consistent

3961b87

Address some of the TODOs

40c3c19

Delete useAsync TODOs

1b2a3e4

Set services ready

5f6f754

Debounce range slider loading state

69cb7a2

Remove final TODO

d9b294a

Make compatibility checks async

7d4406c

Heenawter self-assigned this Sep 17, 2024

kibanamachine and others added 3 commits September 17, 2024 15:04

[CI] Auto-commit changed files from 'node scripts/yarn_deduplicate'

b87037e

[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…

5241d8d

…-fix'

Fix types + tests that use services

c71bfeb

Heenawter force-pushed the embeddableRebuild_controls_cleanup-todos-and-services_2024-09-12 branch from 6269f45 to c71bfeb Compare September 17, 2024 17:23

Fix timeslider registration to await new services

7b609b8

Heenawter mentioned this pull request Sep 17, 2024

[Embeddables Rebuild] [Controls] Final cleanup #192005

Closed

Heenawter commented Sep 17, 2024

View reviewed changes

Heenawter marked this pull request as ready for review September 17, 2024 21:07

Heenawter requested review from a team as code owners September 17, 2024 21:07

nreese self-requested a review September 18, 2024 13:23

nreese reviewed Sep 18, 2024

View reviewed changes

src/plugins/controls/public/plugin.ts Outdated Show resolved Hide resolved

src/plugins/controls/public/plugin.ts Outdated Show resolved Hide resolved

...blic/react_controls/controls/data_controls/range_slider/get_range_slider_control_factory.tsx Outdated Show resolved Hide resolved

Heenawter added 2 commits September 18, 2024 09:30

Code review

3cc39f7

Re-add untilPluginStartServicesReady

ded4bef

nreese approved these changes Sep 18, 2024

View reviewed changes

Heenawter enabled auto-merge (squash) September 18, 2024 18:17

Merge branch 'main' into embeddableRebuild_controls_cleanup-todos-and…

d4c4620

…-services_2024-09-12

rudolf approved these changes Sep 19, 2024

View reviewed changes

Heenawter merged commit db55574 into elastic:main Sep 19, 2024
19 checks passed

kibanamachine added the v9.0.0 label Sep 19, 2024

kibanamachine mentioned this pull request Sep 19, 2024

[8.x] [Embeddable Rebuild] [Controls] Clean up services + TODOs (#193180) #193429

Merged

Heenawter deleted the embeddableRebuild_controls_cleanup-todos-and-services_2024-09-12 branch September 19, 2024 14:10

Heenawter added the project:embeddableRebuild label Sep 19, 2024

kibanamachine added the v8.16.0 label Sep 19, 2024

nreese mentioned this pull request Sep 23, 2024

[Dashboard] Async chunks of control-plugin are loaded on listing page #189679

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Embeddable Rebuild] [Controls] Clean up services + TODOs #193180

[Embeddable Rebuild] [Controls] Clean up services + TODOs #193180

Heenawter commented Sep 17, 2024 •

edited by kibanamachine

Loading

Heenawter Sep 17, 2024

Heenawter Sep 17, 2024

Heenawter Sep 17, 2024

Heenawter Sep 17, 2024

Heenawter Sep 17, 2024

elasticmachine commented Sep 17, 2024

nreese left a comment

kibana-ci commented Sep 18, 2024

API count

async chunk count

References to deprecated APIs

kibanamachine commented Sep 19, 2024

[Embeddable Rebuild] [Controls] Clean up services + TODOs #193180

[Embeddable Rebuild] [Controls] Clean up services + TODOs #193180

Conversation

Heenawter commented Sep 17, 2024 • edited by kibanamachine Loading

Summary

Summary

Checklist

For maintainers

Heenawter Sep 17, 2024

Choose a reason for hiding this comment

Heenawter Sep 17, 2024

Choose a reason for hiding this comment

Heenawter Sep 17, 2024

Choose a reason for hiding this comment

Heenawter Sep 17, 2024

Choose a reason for hiding this comment

Heenawter Sep 17, 2024

Choose a reason for hiding this comment

elasticmachine commented Sep 17, 2024

nreese left a comment

Choose a reason for hiding this comment

kibana-ci commented Sep 18, 2024

💚 Build Succeeded

Metrics [docs]

Module Count

Public APIs missing comments

Async chunks

Page load bundle

API count

async chunk count

References to deprecated APIs

History

kibanamachine commented Sep 19, 2024

💚 All backports created successfully

Questions ?

Heenawter commented Sep 17, 2024 •

edited by kibanamachine

Loading