[Canvas] Cleanup services #194634

Heenawter · 2024-10-01T19:14:44Z

Closes #194050

Summary

This PR refactors the Canvas services to no longer use the PluginServiceProvider from the PresentationUtil plugin. Note that the Canvas storybooks are broken on main (and they have been for who knows how long) and so, while I did make some changes to the storybooks to make them compile, I didn't bother to get them fully functional.

Note that the Ecommerce workpad is broken - this is not due to this PR, it is a bug that is present on main.

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

x-pack/plugins/canvas/public/components/app/index.tsx

Heenawter · 2024-10-07T22:03:07Z

x-pack/plugins/canvas/public/state/actions/filters.js

We had to seperate this code out to avoid circular dependencies 🫠

elasticmachine · 2024-10-07T22:03:26Z

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

nreese

LGTM - just a couple of nits that can be ignored
code review only

x-pack/plugins/canvas/public/components/app/index.tsx

x-pack/plugins/canvas/public/components/embeddable_flyout/flyout.tsx

nreese · 2024-10-08T17:39:19Z

x-pack/plugins/canvas/public/lib/create_handlers.ts

@@ -94,6 +96,7 @@ export const createDispatchedHandlerFactory = (
            break;
          case 'applyFilterAction':
            filters.updateFilter(element.id, event.data);
+            dispatch(fetchAllRenderables());


Was this a fix for something else? Why was dispatch added?

This was part of the circular dependencies fix, since we had to move fetchAllRenderables from updateFilter

x-pack/plugins/canvas/public/routes/workpad/hooks/use_fullscreen_presentation_helper.ts

kibanamachine · 2024-10-08T20:34:19Z

Starting backport for target branches: 8.x

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

elasticmachine · 2024-10-08T20:34:34Z

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`canvas`	1291	1285	-6

Async chunks

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

id	before	after	diff
`canvas`	1.1MB	1.1MB	-13.6KB

Page load bundle

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

id	before	after	diff
`canvas`	13.9KB	14.7KB	+814.0B

Unknown metric groups

async chunk count

id	before	after	diff
`canvas`	18	14	-4

ESLint disabled line counts

id	before	after	diff
`canvas`	31	29	-2

References to deprecated APIs

id	before	after	diff
`canvas`	96	90	-6

Total ESLint disabled count

id	before	after	diff
`canvas`	35	33	-2

History

💔 Build #240398 failed 78ee22b
💔 Build #240330 failed 71b386d
💛 Build #240061 was flaky f6d892d
💚 Build #239995 succeeded 452bd89

kibanamachine · 2024-10-08T20:38:45Z

💔 All backports failed

Status	Branch	Result
❌	8.x	Backport failed because of merge conflicts You might need to backport the following PRs to 8.x: - [Presentation Util] Cleanup services (#194201)

Manual backport

To create the backport manually run:

node scripts/backport --pr 194634

Questions ?

Please refer to the Backport tool documentation

Heenawter · 2024-10-09T16:04:20Z

💚 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

Closes elastic#194050 ## Summary This PR refactors the Canvas services to no longer use the `PluginServiceProvider` from the `PresentationUtil` plugin. Note that the Canvas storybooks are broken on main (and they have been for who knows how long) and so, while I did make some changes to the storybooks to make them **compile**, I didn't bother to get them fully functional. Note that the Ecommerce workpad is broken - this is not due to this PR, it is a [bug](elastic#195297) that is present on main. ### 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: Catherine Liu <[email protected]> (cherry picked from commit 91c045d)

# Backport This will backport the following commits from `main` to `8.x`: - [[Canvas] Cleanup services (#194634)](#194634)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) \r\n\r\n---------\r\n\r\nCo-authored-by: Catherine Liu <[email protected]>","sha":"91c045d698b2e68afd13f5d4bef9229d8a231abe","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Team:Presentation","loe:medium","technical debt","release_note:skip","impact:high","v9.0.0","backport:prev-minor"],"number":194634,"url":"https://github.com/elastic/kibana/pull/194634","mergeCommit":{"message":"[Canvas] Cleanup services (#194634)\n\nCloses https://github.com/elastic/kibana/issues/194050\r\n\r\n## Summary\r\n\r\nThis PR refactors the Canvas services to no longer use the\r\n`PluginServiceProvider` from the `PresentationUtil` plugin. Note that\r\nthe Canvas storybooks are broken on main (and they have been for who\r\nknows how long) and so, while I did make some changes to the storybooks\r\nto make them **compile**, I didn't bother to get them fully functional.\r\n\r\nNote that the Ecommerce workpad is broken - this is not due to this PR,\r\nit is a [bug](#195297) that is\r\npresent on main.\r\n\r\n### Checklist\r\n\r\n- [x] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios\r\n\r\n### For maintainers\r\n\r\n- [ ] This was checked for breaking API changes and was [labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\n\r\n\r\n\r\n---------\r\n\r\nCo-authored-by: Catherine Liu <[email protected]>","sha":"91c045d698b2e68afd13f5d4bef9229d8a231abe"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","labelRegex":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/194634","number":194634,"mergeCommit":{"message":"[Canvas] Cleanup services (#194634)\n\nCloses https://github.com/elastic/kibana/issues/194050\r\n\r\n## Summary\r\n\r\nThis PR refactors the Canvas services to no longer use the\r\n`PluginServiceProvider` from the `PresentationUtil` plugin. Note that\r\nthe Canvas storybooks are broken on main (and they have been for who\r\nknows how long) and so, while I did make some changes to the storybooks\r\nto make them **compile**, I didn't bother to get them fully functional.\r\n\r\nNote that the Ecommerce workpad is broken - this is not due to this PR,\r\nit is a [bug](#195297) that is\r\npresent on main.\r\n\r\n### Checklist\r\n\r\n- [x] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios\r\n\r\n### For maintainers\r\n\r\n- [ ] This was checked for breaking API changes and was [labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\n\r\n\r\n\r\n---------\r\n\r\nCo-authored-by: Catherine Liu <[email protected]>","sha":"91c045d698b2e68afd13f5d4bef9229d8a231abe"}}]}] BACKPORT-->

Heenawter added 7 commits September 26, 2024 14:24

Starting work on refactor

85de1cc

Remove platform service

9f10903

Refactor notify + expressions services

20270be

Start cleaning up hooks

bc18acd

Refactor reporting service

bc02982

Refactor custom elements service

d0681b0

Remove last of plugin services

d10cb91

Heenawter and others added 17 commits October 1, 2024 14:47

Fix data views service hook

49c8a61

Make workpad service a class

d0d28c2

Fix workpad service

258fc97

Trying to fix storybooks

c5ada39

Move hooks

4a9d90f

Fix hooks

f8253aa

Move things out of hooks and fix (?) public export

e93c62a

Remove unnecessary hooks

419afbd

Update tsconfig

8d8ab83

Fix bundle size

2d33d99

Re-add useNotifyService

81a28ae

Remove data views service

aa88d85

Fix mock services

8ebd8ce

Add filters actions

435d5c2

Fix circular deps

e451ebb

Cleanup + fix undefined expression service

0988e32

Add fetch all renderables

6bccb22

Heenawter and others added 3 commits October 7, 2024 10:18

Merge branch 'main' into cleanup-canvas-services_2024-09-26

bb45fd5

Fix reporting service

9ccfb0c

Fix undefined reporting

452bd89

Heenawter commented Oct 7, 2024

View reviewed changes

x-pack/plugins/canvas/public/components/app/index.tsx Outdated Show resolved Hide resolved

Small cleanups

f6d892d

elastic deleted a comment from kibana-ci Oct 7, 2024

Heenawter commented Oct 7, 2024

View reviewed changes

Heenawter marked this pull request as ready for review October 7, 2024 22:03

Heenawter requested a review from a team as a code owner October 7, 2024 22:03

Heenawter and others added 4 commits October 8, 2024 09:17

Merge branch 'main' into cleanup-canvas-services_2024-09-26

10849ac

Fix merge conflicts

7b2eae7

Remove space from HTML

71b386d

Remove core from presentation util

ee5edad

nreese self-requested a review October 8, 2024 17:27

nreese approved these changes Oct 8, 2024

View reviewed changes

Review feedback

78ee22b

Heenawter enabled auto-merge (squash) October 8, 2024 18:01

Remove unused import

ad95d9c

Heenawter merged commit 91c045d into elastic:main Oct 8, 2024
22 checks passed

kibanamachine added the v9.0.0 label Oct 8, 2024

Heenawter deleted the cleanup-canvas-services_2024-09-26 branch October 8, 2024 20:37

Heenawter mentioned this pull request Oct 9, 2024

[8.x] [Canvas] Cleanup services (#194634) #195639

Merged

kibanamachine added the v8.16.0 label Oct 9, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Canvas] Cleanup services #194634

[Canvas] Cleanup services #194634

Heenawter commented Oct 1, 2024 •

edited by kibanamachine

Loading

Heenawter Oct 7, 2024

elasticmachine commented Oct 7, 2024

nreese left a comment

nreese Oct 8, 2024

Heenawter Oct 8, 2024

kibanamachine commented Oct 8, 2024

elasticmachine commented Oct 8, 2024

async chunk count

ESLint disabled line counts

References to deprecated APIs

Total ESLint disabled count

kibanamachine commented Oct 8, 2024

Heenawter commented Oct 9, 2024

[Canvas] Cleanup services #194634

[Canvas] Cleanup services #194634

Conversation

Heenawter commented Oct 1, 2024 • edited by kibanamachine Loading

Summary

Checklist

For maintainers

Heenawter Oct 7, 2024

Choose a reason for hiding this comment

elasticmachine commented Oct 7, 2024

nreese left a comment

Choose a reason for hiding this comment

nreese Oct 8, 2024

Choose a reason for hiding this comment

Heenawter Oct 8, 2024

Choose a reason for hiding this comment

kibanamachine commented Oct 8, 2024

elasticmachine commented Oct 8, 2024

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

Page load bundle

async chunk count

ESLint disabled line counts

References to deprecated APIs

Total ESLint disabled count

History

kibanamachine commented Oct 8, 2024

💔 All backports failed

Manual backport

Questions ?

Heenawter commented Oct 9, 2024

💚 All backports created successfully

Questions ?

Heenawter commented Oct 1, 2024 •

edited by kibanamachine

Loading