Skip to content

Fix imports from restricted paths#130102

Merged
mbondyra merged 6 commits intoelastic:mainfrom
DianaDerevyankina:issues/129316
May 3, 2022
Merged

Fix imports from restricted paths#130102
mbondyra merged 6 commits intoelastic:mainfrom
DianaDerevyankina:issues/129316

Conversation

@DianaDerevyankina
Copy link
Copy Markdown
Contributor

@DianaDerevyankina DianaDerevyankina commented Apr 13, 2022

Closes #129316

Summary

  • Changed most of imports paths from public to common to get rid of restricted paths error;
  • EmbeddableRegistryDefinition type was moved from embeddable server to common and re-exported from server, which resolved some restricted paths in maps common and didn't affect other imports from server files.

For maintainers

@DianaDerevyankina DianaDerevyankina self-assigned this Apr 13, 2022
@DianaDerevyankina DianaDerevyankina added Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// release_note:skip Skip the PR/issue when compiling release notes v8.3.0 labels Apr 13, 2022
import type {
EmbeddablePersistableStateService,
EmbeddableRegistryDefinition,
} from '../../../embeddable/common';
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please let me clarify you some rules:

  • If your file placed in {your_plugin_id}/server/**/*.ts(x) normally we should import from "{plugin}/server".
  • If your file placed in {your_plugin_id}/public/**/*.ts(x) normally we should import from "{plugin}/public".
  • If your file placed in {your_plugin_id}/common/**/*.ts(x) normally we should import from "{plugin}/common".

About this one. The following import import { EmbeddablePersistableStateService } from '../../../embeddable/common'; was absolutely right and we don't needed to modify that.

Please do re-export needed types from the common folder instead of removing it

# Conflicts:
#	src/plugins/controls/server/control_group/control_group_container_factory.ts
#	src/plugins/controls/server/control_types/options_list/options_list_embeddable_factory.ts
#	src/plugins/controls/server/control_types/time_slider/time_slider_embeddable_factory.ts
#	src/plugins/dashboard/server/embeddable/dashboard_container_embeddable_factory.ts
#	src/plugins/embeddable/common/types.ts
#	src/plugins/embeddable/server/plugin.ts
#	src/plugins/visualizations/common/types.ts
#	src/plugins/visualizations/server/embeddable/make_visualize_embeddable_factory.test.ts
#	src/plugins/visualizations/server/embeddable/make_visualize_embeddable_factory.ts
#	x-pack/plugins/lens/common/embeddable_factory/index.ts
#	x-pack/plugins/lens/common/expressions/counter_rate/counter_rate.test.ts
#	x-pack/plugins/lens/common/expressions/format_column/format_column.test.ts
#	x-pack/plugins/lens/common/expressions/merge_tables/merge_tables.test.ts
#	x-pack/plugins/lens/common/expressions/time_scale/time_scale.test.ts
#	x-pack/plugins/lens/server/embeddable/make_lens_embeddable_factory.ts
#	x-pack/plugins/lens/server/migrations/saved_object_migrations.ts
#	x-pack/plugins/lens/server/migrations/types.ts
#	x-pack/plugins/maps/common/embeddable/extract.ts
#	x-pack/plugins/maps/common/embeddable/inject.ts
@DianaDerevyankina DianaDerevyankina marked this pull request as ready for review April 28, 2022 12:55
@DianaDerevyankina DianaDerevyankina requested review from a team as code owners April 28, 2022 12:55
@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/kibana-vis-editors @elastic/kibana-vis-editors-external (Team:VisEditors)

@DianaDerevyankina
Copy link
Copy Markdown
Contributor Author

@elasticmachine merge upstream

@botelastic botelastic bot added the Feature:Embedding Embedding content via iFrame label Apr 28, 2022
Copy link
Copy Markdown
Contributor

@nreese nreese left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maps changes LGTM
code review

Copy link
Copy Markdown
Contributor

@flash1293 flash1293 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@flash1293
Copy link
Copy Markdown
Contributor

@elasticmachine merge upstream

@kibana-ci
Copy link
Copy Markdown

💚 Build Succeeded

Metrics [docs]

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
embeddable 386 388 +2
Unknown metric groups

API count

id before after diff
embeddable 476 478 +2

ESLint disabled line counts

id before after diff
lens 37 29 -8
maps 41 39 -2
visualizations 19 16 -3
total -13

Total ESLint disabled count

id before after diff
lens 39 31 -8
maps 69 67 -2
visualizations 21 18 -3
total -13

History

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

cc @dziyanadzeraviankina

@mbondyra mbondyra merged commit 2355501 into elastic:main May 3, 2022
@kibanamachine kibanamachine added the backport:skip This PR does not require backporting label May 3, 2022
kertal pushed a commit to kertal/kibana that referenced this pull request May 24, 2022
* Fix imports from restricted paths

* Re-export EmbeddableRegistryDefinition from embedabble server and fix some imports

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:skip This PR does not require backporting Feature:Embedding Embedding content via iFrame release_note:skip Skip the PR/issue when compiling release notes Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// v8.3.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix imports from restricted paths

9 participants