[Portable Dashboards] Prep Redux Tools #136572

ThomThomson · 2022-07-18T20:59:42Z

Summary

This PR introduces a new Redux Embeddable Tools system to enhance embeddable DX for the Portable Dashboards initiative.

This PR:

Provides a simple, opt-in state management system for all kinds of state that an Embeddable needs.
- Explicit Input which is state that is passed in to build this embeddable - not including state inherited from the parent
- Output which is derived state that this embeddable outputs to communicate externally
- Component State which is ephemeral, derived state which should not be tracked or persisted.
Unifies all embeddable-based state interactions: Whether they are used from inside the embeddable class, or from inside react components underneath the embeddable class, it is possible to use reducers, and access state from the redux store.
Provides automatic two-way syncing between embeddable inputs and outputs and the redux store.

This PR also migrates the Controls plugin to use the new Redux Embeddable Tools, and removes all other state management workarounds from them.

In addition, this PR reduces the number of re-renders that the Controls are subjected to by using useSelector hooks individually for each piece of state that the component needs rather than using one selector for the whole state object.

Architecture

Checklist

Delete any items that are not applicable to this PR.

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

cla-checker-service · 2022-07-18T20:59:47Z

💚 CLA has been signed

…ing on Control Group

…e slider, and control group

…eDashboard/prepReduxWrapper

elasticmachine · 2022-07-22T16:00:56Z

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

ThomThomson · 2022-07-25T16:00:10Z

@elasticmachine merge upstream

…eDashboard/prepReduxWrapper

Heenawter

Wooo, so happy this is going in 🎉

Mostly a code review only... I did pull and run Kibana, added some controls to Dashboard and ensured everything still... worked? Honestly not sure what else to do, since there's no user-facing changes, heh :) But nothing exploded, so woohoo!

Left a few questions/comments but nothing major. I think it all makes sense to me, and I'm excited for when this can finally be used in Dashboard - seeing it used for Controls really shows the potential!

Heenawter · 2022-07-27T22:15:14Z

src/plugins/controls/public/control_group/embeddable/control_group_container.tsx

    super(
      initialInput,
-      { embeddableLoaded: {} },
+      { dataViewIds: [], loading: false, embeddableLoaded: {}, filters: [] },


Hmm... Why is loading false here? Are you just letting the parent constructor take care of it? If so, why set it explicitly?

I'm not actually sure why I added that - I think at one point I might have modified the output type more significantly, but the Control group doesn't even use loading. It actually has it's own untilReady method that does the same thing. Will remove this.

Heenawter · 2022-07-27T22:40:40Z

src/plugins/presentation_util/public/redux_embeddables/sync_redux_embeddable.ts

+  const { isInputEqual: inputEqualityCheck, isOutputEqual: outputEqualityCheck } = settings ?? {};
+  const inputEqual = (
+    inputA: Partial<ReduxEmbeddableStateType['explicitInput']>,
+    inputB: Partial<ReduxEmbeddableStateType['explicitInput']>
+  ) => (inputEqualityCheck ? inputEqualityCheck(inputA, inputB) : deepEqual(inputA, inputB));
+  const outputEqual = (
+    outputA: ReduxEmbeddableStateType['output'],
+    outputB: ReduxEmbeddableStateType['output']
+  ) => (outputEqualityCheck ? outputEqualityCheck(outputA, outputB) : deepEqual(outputA, outputB));


Ohhhh man, this logic is so much nicer for when we switch Dashboard over to use this for state management 🔥

Heenawter · 2022-07-27T22:43:28Z

src/plugins/presentation_util/public/redux_embeddables/sync_redux_embeddable.ts

+    const { explicitInput: reduxExplicitInput } = store.getState();
+
+    // store only explicit input in the store
+    const embeddableExplictiInput = embeddable.getExplicitInput() as Writeable<


Typo:

Suggested change

const embeddableExplictiInput = embeddable.getExplicitInput() as Writeable<

const embeddableExplicitInput = embeddable.getExplicitInput() as Writeable<

Whoops, will fix! Nice catch

Heenawter · 2022-07-28T15:04:28Z

src/plugins/controls/public/types.ts

 import { UnifiedSearchPublicPluginStart } from '@kbn/unified-search-plugin/public';
 import { ControlInput } from '../common/types';
 import { ControlsService } from './services/controls';

 export interface CommonControlOutput {
  filters?: Filter[];
-  dataViews?: DataView[];
+  dataViewId?: string;


Why the switch to dataViewId rather than just dataView directly? Just for... initial speed perhaps, since it's only needed when changes are actually made to the control's selections?

As far as why it's no longer an array - I assume that's because a control can only have a single data view? Just wanted to clarify :)

This is actually a little bit of an awkward change introduced by this PR. Basically embeddables have this pattern where they communicate the data views they use to the Dashboard so that the dashboard can pick appropriate data views when creating a filter. The problem with this is that the data view they were communicating was a whole data view object instance which is non-serializable and cannot be stored in Redux. For this reason I made the switch for Controls only to communicate the data view Id instead.

See this issue for a description of how we could better extend this pattern to other embeddables.

Ahhh that makes perfect sense - thank you! I definitely think reducing the amount of times we have to fetch the data view objects would be a huge win for performance, so communicating IDs and only fetching when absolutely necessary is a great pattern 💪

Heenawter · 2022-07-28T15:13:38Z

src/plugins/controls/public/control_types/options_list/options_list_component.tsx

-          invalidSelections={invalidSelections}
-          updateSearchString={updateSearchString}
-        />
+        <OptionsListPopover width={dimensions.width} updateSearchString={updateSearchString} />


So much cleaner, wow 🎉

Heenawter · 2022-07-28T15:22:12Z

src/plugins/controls/public/control_types/range_slider/range_slider.component.tsx

+export const RangeSliderComponent = () => {
+  return <RangeSliderPopover />;


Hmm... Is it worth even keeping RangeSliderComponent at this point now that a bunch of logic has been removed? Could we not just use RangeSliderPopover component directly for the embeddable render?

I was actually going to work through this with @cqliu1 yeah! The it seems like the whole Component was just a wrapper for passing some props down (which is no longer needed!). Other than that, I'd really like to be able to split the Popover up into two components - one with the popover contents only, and then the component can handle everything else, kinda like how Options List does it.

Created this issue

kibana-ci · 2022-07-28T18:25:24Z

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`controls`	173	196	+23
`presentationUtil`	181	183	+2
total			+25

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`	198	199	+1
`presentationUtil`	180	187	+7
total			+8

Async chunks

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

id	before	after	diff
`controls`	428.3KB	443.4KB	+15.1KB
`dashboard`	409.7KB	409.8KB	+3.0B
`presentationUtil`	127.6KB	127.5KB	-21.0B
total			+15.0KB

Canvas Sharable Runtime

The Canvas "shareable runtime" is an bundle produced to enable running Canvas workpads outside of Kibana. This bundle is included in third-party webpages that embed canvas and therefor should be as slim as possible.

id	before	after	diff
`module count`	4939	4876	-63
`total size`	7.7MB	7.6MB	-49.5KB

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
`presentationUtil`	11	12	+1

Page load bundle

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

id	before	after	diff
`controls`	29.2KB	29.3KB	+183.0B
`presentationUtil`	43.3KB	43.2KB	-141.0B
total			+42.0B

Unknown metric groups

API count

id	before	after	diff
`controls`	206	207	+1
`presentationUtil`	231	243	+12
total			+13

async chunk count

id	before	after	diff
`controls`	6	7	+1

ESLint disabled line counts

id	before	after	diff
`presentationUtil`	10	9	-1

Total ESLint disabled count

id	before	after	diff
`presentationUtil`	12	11	-1

History

💚 Build #61210 succeeded 950268d
💚 Build #60450 succeeded 3b0c402
💚 Build #59917 succeeded 05eebc0
💔 Build #59886 failed 84a6953
💚 Build #59687 succeeded f1424df
💔 Build #59653 failed 6364cb9

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

* Created new Redux system for embeddables in Presentation Util. Migrated all Controls to use it. (cherry picked from commit 01fc584)

kibanamachine · 2022-07-29T14:44:38Z

💚 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

ThomThomson added 3 commits July 18, 2022 17:18

new redux system in progress

450d2f9

migrated options list and range slider to use new redux system. Start…

0e00f73

…ing on Control Group

Track Input Output and componentState in redux for options list, rang…

ffa8253

…e slider, and control group

ThomThomson force-pushed the portableDashboard/prepReduxWrapper branch from 2f5e805 to ffa8253 Compare July 18, 2022 21:22

ThomThomson added 7 commits July 19, 2022 16:51

update mocks for jest, fix type errors

7a35777

Merge branch 'main' of https://github.com/elastic/kibana into portabl…

41ab705

…eDashboard/prepReduxWrapper

fix case-insensitive search for options list

6c13d21

Merge branch 'main' of https://github.com/elastic/kibana into portabl…

4781489

…eDashboard/prepReduxWrapper

lazy load redux embeddable tools to reduce bundle size

bcbcc00

add basic mock for redux embeddable package

6364cb9

mock for dashboard control group test

f1424df

ThomThomson changed the title ~~Portable dashboard/prep redux wrapper~~ [Portable Dashboards] Prep Redux Tools Jul 21, 2022

ThomThomson added 2 commits July 22, 2022 09:51

Merge branch 'main' of https://github.com/elastic/kibana into portabl…

84a6953

…eDashboard/prepReduxWrapper

reset search string on field change using Redux

05eebc0

ThomThomson marked this pull request as ready for review July 22, 2022 16:00

ThomThomson requested review from a team as code owners July 22, 2022 16:00

Merge branch 'main' into portableDashboard/prepReduxWrapper

3b0c402

botelastic bot added the Feature:Embedding Embedding content via iFrame label Jul 25, 2022

Heenawter self-requested a review July 26, 2022 16:54

ThomThomson added 2 commits July 27, 2022 12:31

undo change to embeddable.tsx

438d354

Merge branch 'main' of https://github.com/elastic/kibana into portabl…

950268d

…eDashboard/prepReduxWrapper

ThomThomson removed the request for review from a team July 27, 2022 16:32

Heenawter approved these changes Jul 28, 2022

View reviewed changes

review feedback

06592e9

ThomThomson merged commit 01fc584 into elastic:main Jul 29, 2022

ThomThomson added v8.5.0 and removed v8.4.0 labels Jul 29, 2022

kibanamachine added the backport:skip This commit does not require backporting label Jul 29, 2022

kibanamachine mentioned this pull request Jul 29, 2022

[8.4] [Portable Dashboards] Prep Redux Tools (#136572) #137595

Closed

ThomThomson mentioned this pull request Aug 2, 2022

[Controls] Refactor Range Slider Popover #137888

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Portable Dashboards] Prep Redux Tools #136572

[Portable Dashboards] Prep Redux Tools #136572

ThomThomson commented Jul 18, 2022 •

edited by kibanamachine

Loading

cla-checker-service bot commented Jul 18, 2022 •

edited

Loading

elasticmachine commented Jul 22, 2022

ThomThomson commented Jul 25, 2022

Heenawter left a comment

Heenawter Jul 27, 2022

ThomThomson Jul 28, 2022

Heenawter Jul 27, 2022

Heenawter Jul 27, 2022

ThomThomson Jul 28, 2022

Heenawter Jul 28, 2022

ThomThomson Jul 28, 2022

Heenawter Jul 28, 2022

Heenawter Jul 28, 2022

Heenawter Jul 28, 2022

ThomThomson Jul 28, 2022

ThomThomson Aug 2, 2022

kibana-ci commented Jul 28, 2022

API count

async chunk count

ESLint disabled line counts

Total ESLint disabled count

kibanamachine commented Jul 29, 2022

	const embeddableExplictiInput = embeddable.getExplicitInput() as Writeable<
	const embeddableExplicitInput = embeddable.getExplicitInput() as Writeable<

		export const RangeSliderComponent = () => {
		return <RangeSliderPopover />;

[Portable Dashboards] Prep Redux Tools #136572

[Portable Dashboards] Prep Redux Tools #136572

Conversation

ThomThomson commented Jul 18, 2022 • edited by kibanamachine Loading

Summary

Architecture

Checklist

cla-checker-service bot commented Jul 18, 2022 • edited Loading

elasticmachine commented Jul 22, 2022

ThomThomson commented Jul 25, 2022

Heenawter left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kibana-ci commented Jul 28, 2022

💚 Build Succeeded

Metrics [docs]

Module Count

Public APIs missing comments

Async chunks

Canvas Sharable Runtime

Public APIs missing exports

Page load bundle

API count

async chunk count

ESLint disabled line counts

Total ESLint disabled count

History

kibanamachine commented Jul 29, 2022

💚 All backports created successfully

Questions ?

ThomThomson commented Jul 18, 2022 •

edited by kibanamachine

Loading

cla-checker-service bot commented Jul 18, 2022 •

edited

Loading