visualizations plugin np_ready #57983

Dosant · 2020-02-19T09:48:53Z

Summary

Partially addresses #50897

updates visualizations plugin

makes visualizations/saved_visualizations np_ready
makes visualizations/embeddable np_ready
remove DefaultVisEditor dependency from visualizations
import AggConfigs directly instead of ui/public
Clean up imports from root. use relative imports
Remove bind directive import

Checklist

Delete any items that are not applicable to this PR.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
This was checked for keyboard-only and screenreader accessibility
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser
This was checked for cross-browser compatibility, including a check against IE11

For maintainers

This was checked for breaking API changes and was labeled appropriately

…/vis-emeddable

…/vis-emeddable # Conflicts: # src/legacy/core_plugins/visualizations/public/np_ready/public/embeddable/visualize_embeddable.ts # src/legacy/core_plugins/visualizations/public/np_ready/public/legacy/build_pipeline.ts

…/vis-emeddable # Conflicts: # src/legacy/core_plugins/visualizations/public/np_ready/public/embeddable/visualize_embeddable.ts # src/legacy/core_plugins/visualizations/public/saved_visualizations/saved_visualizations.ts

elasticmachine · 2020-02-24T12:42:25Z

Pinging @elastic/kibana-app-arch (Team:AppArch)

…/vis-emeddable # Conflicts: # src/legacy/core_plugins/visualizations/public/np_ready/public/embeddable/get_index_pattern.ts # src/legacy/core_plugins/visualizations/public/np_ready/public/embeddable/visualize_embeddable.ts

flash1293

Kibana app changes LGTM, didn't test

Dosant · 2020-02-24T15:21:59Z

@elasticmachine merge upstream

cchaos

SASS moves lgtm

…/vis-emeddable

Agg configs imports

ppisljar

LGTM

kibanamachine · 2020-02-25T13:11:28Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 7e3f83a

History

💚 Build #28709 succeeded b74b076
💚 Build #28480 succeeded 6acf997
💔 Build #28436 failed 6e82f63
💚 Build #28099 succeeded a04e2a2
💚 Build #28089 succeeded 13c9fb1

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

lizozom

Overall looks great.
Added couple questions for my own understanding. Lets address F2F.

src/legacy/core_plugins/kibana/public/visualize/kibana_services.ts

lizozom · 2020-02-25T11:33:04Z

src/legacy/core_plugins/visualizations/public/np_ready/kibana.json

-    "data",
-    "search"
-  ]
+  "requiredPlugins": ["data", "search", "expressions", "uiActions"]


Is there a search plugin at all?

Not sure :D
This will be fixed during cutover anyway. Bunch of plugins are missing here.

Can't we remove this one?

...legacy/core_plugins/visualizations/public/np_ready/public/saved_visualizations/_saved_vis.ts

lizozom · 2020-02-25T13:37:45Z

src/plugins/saved_objects/public/save_modal/saved_object_save_modal.tsx

+// TODO: can't import from '../../../../legacy/core_plugins/visualizations/public/' directly,
+// because yarn build:types fails after trying to emit type declarations for whole visualizations plugin
+// Bunch of errors like this: 'Return type of exported function has or is using private name 'SavedVis''
+import { VISUALIZE_EMBEDDABLE_TYPE } from '../../../../legacy/core_plugins/visualizations/public/np_ready/public/embeddable/constants';


// brain pick
What exactly happened here?

As I understood, there is yarn build:types which emits d.ts files for core code.
In compilation included only:

"include": [ "src/core/server/index.ts", "src/core/public/index.ts", "typings" ]

This file here - src/plugins/saved_objects/public/save_modal/saved_object_save_modal.tsx ends up as a deep dependency of "src/core/public/index.ts".

If to import whole visualisation plugin, the type declaration emitting breaks, because of incompatible for declarations code as a dependency of visualisation plugin.
The code which causing this error:
'Return type of exported function has or is using private name 'SavedVis''
is code like this:

export function createSavedVisLoader(services: SavedObjectKibanaServicesWithVisualizations) { const { savedObjectsClient, visualizationTypes } = services; class SavedObjectLoaderVisualize extends SavedObjectLoader { .... } return SavedObjectLoaderVisualize

See that SavedObjectLoaderVisualize is private, so it can't be emitted in declaration.

This root cause issue should be addressed by: #58243

P.S. spend like half of a day figuring out what is going wrong :D

🎩
😄 👌

Wow, thanks for digging into it @Dosant ! This import should definitely not be there, this was an oversight during migration. Saved object save modal shouldn't know anything about the visualize embeddable, this logic should be moved into the specific calls (e.g. by passing in a prop like hideDescription).

I created an issue for this here: #58479

@joshdover I was under the impression the linter would prevent us from referencing legacy from within the new platform. A quick code search reveals a number of offenders, did something break here?

crob611

Canvas changes look good here 👍

Partially addresses elastic#50897 updates visualizations plugin makes visualizations/saved_visualizations np_ready makes visualizations/embeddable np_ready remove DefaultVisEditor dependency from visualizations import AggConfigs directly instead of ui/public Clean up imports from root. use relative imports Remove bind directive import

Partially addresses #50897 updates visualizations plugin makes visualizations/saved_visualizations np_ready makes visualizations/embeddable np_ready remove DefaultVisEditor dependency from visualizations import AggConfigs directly instead of ui/public Clean up imports from root. use relative imports Remove bind directive import Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

Dosant added 3 commits February 18, 2020 13:42

wip

d6fe71a

Merge branch 'master' of github.com:elastic/kibana into dev/vis-to-np…

a9f1eb4

…/vis-emeddable

wip

2094021

Dosant changed the title ~~[wip] vis np ready~~ [wip] vis embeddable np ready Feb 19, 2020

Dosant added 3 commits February 19, 2020 11:08

wip

b3edd03

wip

5ebafef

wip

bebab07

Dosant force-pushed the dev/vis-to-np/vis-emeddable branch from 76d1053 to bebab07 Compare February 19, 2020 14:54

Dosant added 11 commits February 19, 2020 17:13

wip

70ee042

wip

56909af

wip

d6b18e2

wip

e8efbf6

ts vis/find_list_items

1011e41

wip

24d3675

fix jest tests

110085e

wip

13c9fb1

wip

a04e2a2

Dosant changed the title ~~[wip] vis embeddable np ready~~ visualizations plugin np_ready Feb 21, 2020

Dosant requested a review from ppisljar February 21, 2020 13:12

Dosant added v7.7.0 v8.0.0 Feature:NP Migration Feature:Visualizations Generic visualization features (in case no more specific feature label is available) labels Feb 24, 2020

Dosant added the Team:AppArch label Feb 24, 2020

Dosant added release_note:skip Skip the PR/issue when compiling release notes review labels Feb 24, 2020

Dosant marked this pull request as ready for review February 24, 2020 12:45

Dosant requested a review from a team February 24, 2020 12:45

Dosant requested review from a team as code owners February 24, 2020 12:45

Dosant added 3 commits February 24, 2020 13:58

wip

7ab0754

fix ts

6e82f63

flash1293 approved these changes Feb 24, 2020

View reviewed changes

Merge branch 'master' into dev/vis-to-np/vis-emeddable

6acf997

cchaos approved these changes Feb 24, 2020

View reviewed changes

Dosant added 6 commits February 25, 2020 08:49

relative imports

5b58217

remove bind

ba4fc87

remove default

c5fdd17

Merge branch 'master' of github.com:elastic/kibana into dev/vis-to-np…

b74b076

…/vis-emeddable

wip

c880a9c

Agg configs imports

relative import

7e3f83a

ppisljar approved these changes Feb 25, 2020

View reviewed changes

lizozom approved these changes Feb 25, 2020

View reviewed changes

crob611 approved these changes Feb 25, 2020

View reviewed changes

Dosant merged commit 93556a9 into elastic:master Feb 25, 2020

Dosant mentioned this pull request Feb 25, 2020

[7.x] Visualizations plugin np_ready (#57983) #58508

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

visualizations plugin np_ready #57983

visualizations plugin np_ready #57983

Dosant commented Feb 19, 2020 •

edited

Loading

elasticmachine commented Feb 24, 2020

flash1293 left a comment

Dosant commented Feb 24, 2020

cchaos left a comment

ppisljar left a comment

kibanamachine commented Feb 25, 2020

lizozom left a comment

lizozom Feb 25, 2020 •

edited

Loading

Dosant Feb 25, 2020 •

edited

Loading

lizozom Feb 25, 2020

lizozom Feb 25, 2020

Dosant Feb 25, 2020

lizozom Feb 25, 2020

flash1293 Feb 25, 2020

crob611 left a comment

visualizations plugin np_ready #57983

visualizations plugin np_ready #57983

Conversation

Dosant commented Feb 19, 2020 • edited Loading

Summary

Checklist

For maintainers

elasticmachine commented Feb 24, 2020

flash1293 left a comment

Choose a reason for hiding this comment

Dosant commented Feb 24, 2020

cchaos left a comment

Choose a reason for hiding this comment

ppisljar left a comment

Choose a reason for hiding this comment

kibanamachine commented Feb 25, 2020

💚 Build Succeeded

History

lizozom left a comment

Choose a reason for hiding this comment

lizozom Feb 25, 2020 • edited Loading

Choose a reason for hiding this comment

Dosant Feb 25, 2020 • edited Loading

Choose a reason for hiding this comment

lizozom Feb 25, 2020

Choose a reason for hiding this comment

lizozom Feb 25, 2020

Choose a reason for hiding this comment

Dosant Feb 25, 2020

Choose a reason for hiding this comment

lizozom Feb 25, 2020

Choose a reason for hiding this comment

flash1293 Feb 25, 2020

Choose a reason for hiding this comment

crob611 left a comment

Choose a reason for hiding this comment

Dosant commented Feb 19, 2020 •

edited

Loading

lizozom Feb 25, 2020 •

edited

Loading

Dosant Feb 25, 2020 •

edited

Loading