[Embeddables Rebuild] Make Serialize Function Synchronous by ThomThomson · Pull Request #203662 · elastic/kibana

ThomThomson · 2024-12-10T18:18:18Z

Summary

This PR changes the signature of the serializeState function so that it no longer returns a MaybePromise. This change is made in preparation for an architectural shift which will align all Dashboard embeddable interactions on serialized state rather than runtime state

Additionally, the one example that used the async version of serializeState has been updated. Previously the book example embeddable would save changes to its reference at Dashboard save time. After this PR, the book example embeddable saves changes to its reference on save in the editor flyout. This matches our pattern for inline editable panels that can be saved by reference which is used in the Links panel.

Checklist

Check the PR satisfies following conditions.

Reviewers should verify this PR satisfies this list as well.

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

…synchronousSerialize' into embeddableRefactor/synchronousSerialize

ThomThomson · 2024-12-11T16:18:25Z

/ci

elasticmachine · 2024-12-11T18:15:17Z

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

nreese · 2024-12-11T18:43:28Z

examples/embeddable_examples/public/app/state_management_example/state_management_example.tsx

              setIsSaving(true);
-              const bookSerializedState = await bookApi.serializeState();
+              const bookSerializedState = bookApi.serializeState();
              if (!isMounted()) {


isMounted check can be removed since callback is now sync

Addressed and some other cleanups in 8b0809c

nreese · 2024-12-11T18:47:16Z

src/plugins/dashboard/public/dashboard_api/types.ts

    forceRefresh: () => void;
    getSettings: () => DashboardStateFromSettingsFlyout;
-    getDashboardPanelFromId: (id: string) => Promise<DashboardPanelState>;
+    getDashboardPanelFromId: (id: string) => DashboardPanelState;


src/plugins/dashboard/public/dashboard_actions/copy_to_dashboard_modal.tsx still awaits getDashboardPanelFromId.

removed in 8b0809c

nreese · 2024-12-11T18:50:58Z

src/plugins/dashboard/public/dashboard_api/panels_manager.ts

@@ -419,31 +412,18 @@ export function initializePanelsManager(
        references: Reference[];
      }> => {
        const references: Reference[] = [];


you can remove async from getState method

removed in 8b0809c

nreese · 2024-12-11T18:57:37Z

examples/embeddable_examples/public/react_embeddables/saved_book/saved_book_editor.tsx

            closeOverlay(overlay);
          }}
-          onSubmit={(addToLibrary: boolean) => {
+          onSubmit={async (addToLibrary: boolean) => {


now that onSubmit is aysnc, SavedBookEditor should show loading state and disable submit button until onSubmit returns to avoid allowing users to click multiple times

Good call. Updated this in 8b0809c

ThomThomson · 2024-12-11T19:27:27Z

/ci

nreese

kibana-presentation changes LGTM
code review only

src/plugins/dashboard/public/dashboard_api/panels_manager.ts

ThomThomson · 2024-12-12T15:41:57Z

/ci

ThomThomson · 2024-12-12T17:34:09Z

@elasticmachine merge upstream

ThomThomson · 2024-12-12T17:34:24Z

/ci

elasticmachine · 2024-12-12T19:28:31Z

💚 Build Succeeded

Buildkite Build
Commit: a842469

Metrics [docs]

Async chunks

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

id	before	after	diff
`dashboard`	668.3KB	668.1KB	-206.0B
`discover`	785.9KB	786.0KB	+109.0B
`links`	48.6KB	48.6KB	-5.0B
total			-102.0B

Page load bundle

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

id	before	after	diff
`dashboard`	52.3KB	52.3KB	-15.0B
`discover`	44.0KB	44.0KB	+56.0B
`embeddable`	65.4KB	65.3KB	-140.0B
total			-99.0B

History

💔 Build #259912 failed 567ffc3
💔 Build #259499 failed 8b0809c
💚 Build #259404 succeeded 36e8bc8
💔 Build #259087 failed 338d8a3
💔 Build #259075 failed 312a111

davismcphee

Data Discovery changes LGTM! Tested the saved search embeddable locally and saving panel state seems to still work as expected 👍

davismcphee · 2024-12-12T19:20:22Z

src/plugins/discover/public/embeddable/get_search_embeddable_factory.tsx

          ...titleComparators,
          ...timeRange.comparators,
          ...searchEmbeddable.comparators,
+          rawSavedObjectAttributes: getUnchangingComparator(),


Oh we have other unchanging state like nonPersistedDisplayOptions where maybe this is useful too (not for this PR, just thinking in general). Is getUnchangingComparator intended to be the standard way to handle this?

Yes! This is the intended workaround.

kibanamachine · 2024-12-16T18:45:30Z

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/12359199165

kibanamachine · 2024-12-16T18:45:31Z

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/12359199142

kibanamachine · 2024-12-16T18:50:26Z

💚 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

…3662) changes the signature of the `serializeState` function so that it no longer returns MaybePromise (cherry picked from commit abfd590)

kibanamachine · 2024-12-16T18:50:33Z

💚 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

kibanamachine · 2024-12-17T18:48:51Z

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

) (#204470) # Backport This will backport the following commits from `main` to `8.x`: - [[Embeddables Rebuild] Make Serialize Function Synchronous (#203662)](#203662)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Devon Thomson <devon.thomson@elastic.co>

ThomThomson and others added 7 commits December 10, 2024 13:17

Make serializeState synchronous

f84e622

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

ce98326

fix type

f24ad9b

Merge remote-tracking branch 'refs/remotes/origin/embeddableRefactor/…

a7fca39

…synchronousSerialize' into embeddableRefactor/synchronousSerialize

remove discover SO fetch on save.

312a111

remove unnecessary async

338d8a3

fix discover serialization tests.

36e8bc8

ThomThomson marked this pull request as ready for review December 11, 2024 18:15

ThomThomson requested review from a team as code owners December 11, 2024 18:15

ThomThomson added backport:skip This PR does not require backporting release_note:skip Skip the PR/issue when compiling release notes labels Dec 11, 2024

nreese self-requested a review December 11, 2024 18:41

nreese reviewed Dec 11, 2024

View reviewed changes

botelastic bot added the Feature:Embedding Embedding content via iFrame label Dec 11, 2024

review feedback

8b0809c

nreese approved these changes Dec 11, 2024

View reviewed changes

src/plugins/dashboard/public/dashboard_api/panels_manager.ts Outdated Show resolved Hide resolved

remove leftover await

567ffc3

Merge branch 'main' into embeddableRefactor/synchronousSerialize

a842469

davismcphee approved these changes Dec 13, 2024

View reviewed changes

ThomThomson merged commit abfd590 into elastic:main Dec 13, 2024

kibanamachine added the v9.0.0 label Dec 13, 2024

nreese mentioned this pull request Dec 13, 2024

[Dashboards] Add getSerializedState method to Dashboard API #204140

Merged

9 tasks

ThomThomson added backport:prev-minor and removed backport:skip This PR does not require backporting labels Dec 16, 2024

kibanamachine mentioned this pull request Dec 16, 2024

[8.x] [Embeddables Rebuild] Make Serialize Function Synchronous (#203662) #204470

Merged

kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Dec 17, 2024

kibanamachine added v8.18.0 and removed backport missing Added to PRs automatically when the are determined to be missing a backport. labels Dec 17, 2024

ThomThomson mentioned this pull request Dec 19, 2024

[Discover][Embeddable] Search embeddable to keep all SO fields #204651

Closed

ThomThomson mentioned this pull request Jan 3, 2025

[Embeddable][Dashboard] Use Serialized State Only #205531

Closed

5 tasks

Conversation

ThomThomson commented Dec 10, 2024 • edited by kibanamachine Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

Uh oh!

ThomThomson commented Dec 11, 2024

Uh oh!

elasticmachine commented Dec 11, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ThomThomson commented Dec 11, 2024

Uh oh!

nreese left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ThomThomson commented Dec 12, 2024

Uh oh!

ThomThomson commented Dec 12, 2024

Uh oh!

ThomThomson commented Dec 12, 2024

Uh oh!

elasticmachine commented Dec 12, 2024

💚 Build Succeeded

Metrics [docs]

Async chunks

Page load bundle

History

Uh oh!

davismcphee left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kibanamachine commented Dec 16, 2024

Uh oh!

kibanamachine commented Dec 16, 2024

Uh oh!

kibanamachine commented Dec 16, 2024

💚 All backports created successfully

Questions ?

Uh oh!

kibanamachine commented Dec 16, 2024

💚 All backports created successfully

Questions ?

Uh oh!

kibanamachine commented Dec 17, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ThomThomson commented Dec 10, 2024 •

edited by kibanamachine

Loading