[Visualizations] Remove vis_default_editor - visualize plugins cyclic dependencies by alexwizp · Pull Request #85422 · elastic/kibana

alexwizp · 2020-12-09T15:12:44Z

Part of: #84750

Summary

Removes circular dependencies between vis_default_editor and visualize plugins.
The dependencies that were removed based on the node scripts/find_plugins_with_circular_deps --debug script.

What was changed in that PR:

setDefaultEditor function was added into visualize API (setup hook).
vis_default_editor still depends on visualize and execute setDefaultEditor hook on setup phase

These two changes allowed the dependency to be kept in only one direction: vis_default_editor -> visualize

… dependencies # Conflicts: # src/plugins/visualize/kibana.json

alexwizp · 2020-12-10T08:33:43Z

@elasticmachine merge upstream

alexwizp · 2020-12-11T14:43:12Z

@elasticmachine merge upstream

sulemanof · 2020-12-14T07:55:18Z

Hey! I'm fond of creating an editor setter in visualize plugin contract!
But, what if you create a registry instead ?
This would be possible to register other editors (since we also have the tsvb editor) only if visualize is enabled.
Then the editor would be picked up something like:

// vis.type.editor = 'tsvb'
const Editor = visualize.editorRegistry.find(vis.type.editor);

find would return the default one if editor is not specified

alexwizp · 2020-12-14T09:29:34Z

@elasticmachine merge upstream

alexwizp · 2020-12-14T09:58:45Z

@sulemanof Honestly I don't see any benefits of adding one more registry for that. Currently we can set a specific editor in visTypeDefinition object. It's a pretty flexible solution which works without any extra classes.

Probably we can implement your idea in future, but it should be definitely resolved in a separate PR, cause for me it looks like before using custom Editor it should be registered. It can be a blocker for some of ThirdParty plugins.

elasticmachine · 2020-12-14T13:41:52Z

Pinging @elastic/kibana-app (Team:KibanaApp)

alexwizp · 2020-12-17T08:49:39Z

@elasticmachine merge upstream

jbudz

operations - circular dependencies list LGTM

alexwizp · 2020-12-18T15:41:33Z

@elasticmachine merge upstream

alexwizp · 2020-12-20T19:19:35Z

@elasticmachine merge upstream

alexwizp · 2020-12-21T07:22:18Z

@elasticmachine merge upstream

alexwizp · 2020-12-22T09:24:22Z

@elasticmachine merge upstream

kibanamachine · 2020-12-22T11:35:16Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 5a9651f

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`visDefaultEditor`	125	126	+1

Async chunks

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

id	before	after	diff
`visDefaultEditor`	300.9KB	300.9KB	+1.0B
`visualize`	130.9KB	130.8KB	-52.0B
total			-51.0B

Distributable file count

id	before	after	diff
`default`	47144	47904	+760

Page load bundle

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

id	before	after	diff
`visDefaultEditor`	46.0KB	46.5KB	+491.0B
`visualize`	40.4KB	42.3KB	+1.9KB
total			+2.4KB

History

💚 Build #95667 succeeded db908ad
💚 Build #95654 succeeded eeca01c
💚 Build #95440 succeeded 5fe40a8
💚 Build #95083 succeeded 1bc8c1c
💚 Build #93909 succeeded fbc689b

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

stratoula

Dependency has been removed, checked it locally. Code LGTM. I tested it and it seems that works fine 😉

… dependencies (elastic#85422) * [Visualizations] Remove vis_default_editor - visualize plugins cyclic dependencies # Conflicts: # src/plugins/visualize/kibana.json * fix CI * fix CI Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

* master: (36 commits) update apm index pattern (elastic#86739) [Visualizations] Remove vis_default_editor - visualize plugins cyclic dependencies (elastic#85422) [ML] Fix alignment of values in data frame analytics results view badges (elastic#86621) [Visualizations] Remove charts - editor plugins cyclic dependencies (elastic#84887) fixing blank page (elastic#86640) Update dependency vega to ^5.17.1 (elastic#86715) [Monitoring] Convert Kibana-related server files that read from _source to typescript (elastic#86364) Uses @elastic/elasticsearch-canary (elastic#86398) [CI] Removes script previously used for Karma (elastic#86412) [build] Remove grunt checkPlugins task (elastic#85852) [build] Remove grunt docker:docs task (elastic#85848) [ML] Add doc link for classification AUC ROC evaluation (elastic#86660) [ML] Edits saved object synchronization message (elastic#86664) Uses the new es client in canvas usage collector's fetch methods (elastic#86668) [ML] Support legacy watcher URL (elastic#86661) [ML] Fix Single Metric Viewer y domain extending beyond the visible focus area (elastic#86655) Migrates search telemetry usage collector es client from legacy to new (elastic#86597) [Alerting] Encourage type safe usage of Alerting (elastic#86623) Migrates kql_telemetry usage collector es client (elastic#86585) [ML] Fix time range adjustment for the swim lane causing the infinite loop update (elastic#86461) ...

… dependencies (#85422) (#86753) * [Visualizations] Remove vis_default_editor - visualize plugins cyclic dependencies # Conflicts: # src/plugins/visualize/kibana.json * fix CI * fix CI Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

[Visualizations] Remove vis_default_editor - visualize plugins cyclic…

075d2ca

… dependencies # Conflicts: # src/plugins/visualize/kibana.json

alexwizp force-pushed the deps_1 branch from 05b852d to 075d2ca Compare December 9, 2020 15:15

alexwizp mentioned this pull request Dec 9, 2020

Remove circular deps between KibanaApp team plugins #84750

Closed

5 tasks

kibanamachine and others added 2 commits December 10, 2020 03:33

Merge branch 'master' into deps_1

bea1171

fix CI

982426c

kibanamachine and others added 2 commits December 11, 2020 09:43

Merge branch 'master' into deps_1

1492dc9

fix CI

6db8f60

alexwizp requested review from stratoula and sulemanof December 11, 2020 15:15

alexwizp self-assigned this Dec 11, 2020

alexwizp added v7.12.0 v8.0.0 Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// Feature:Vis Editor Visualization editor issues release_note:skip Skip the PR/issue when compiling release notes labels Dec 11, 2020

alexwizp requested a review from timroes December 11, 2020 15:22

Merge branch 'master' into deps_1

fbc689b

alexwizp marked this pull request as ready for review December 14, 2020 13:41

alexwizp requested a review from a team December 14, 2020 13:41

alexwizp requested a review from a team as a code owner December 14, 2020 13:41

Merge branch 'master' into deps_1

1bc8c1c

jbudz approved these changes Dec 17, 2020

View reviewed changes

Merge branch 'master' into deps_1

5fe40a8

Merge branch 'master' into deps_1

eeca01c

Merge branch 'master' into deps_1

db908ad

Merge branch 'master' into deps_1

5a9651f

stratoula approved these changes Dec 22, 2020

View reviewed changes

alexwizp merged commit e2c4356 into elastic:master Dec 22, 2020

alexwizp mentioned this pull request Dec 22, 2020

[7.x] [Visualizations] Remove vis_default_editor - visualize plugins cyclic dependencies (#85422) #86753

Merged

alexwizp deleted the deps_1 branch January 16, 2021 15:25

Conversation

alexwizp commented Dec 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Uh oh!

alexwizp commented Dec 10, 2020

Uh oh!

alexwizp commented Dec 11, 2020

Uh oh!

sulemanof commented Dec 14, 2020

Uh oh!

alexwizp commented Dec 14, 2020

Uh oh!

alexwizp commented Dec 14, 2020

Uh oh!

elasticmachine commented Dec 14, 2020

Uh oh!

alexwizp commented Dec 17, 2020

Uh oh!

jbudz left a comment

Choose a reason for hiding this comment

Uh oh!

alexwizp commented Dec 18, 2020

Uh oh!

alexwizp commented Dec 20, 2020

Uh oh!

alexwizp commented Dec 21, 2020

Uh oh!

alexwizp commented Dec 22, 2020

Uh oh!

kibanamachine commented Dec 22, 2020

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

Distributable file count

Page load bundle

History

Uh oh!

stratoula left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

alexwizp commented Dec 9, 2020 •

edited

Loading