Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[chore] Correct dependency inconsistencies in Presentation team plugins #173778

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

clintandrewhall
Copy link
Contributor

@clintandrewhall clintandrewhall commented Dec 20, 2023

Changes identified using the script from #171483

Summary

The script from #171483 can identify inconsistencies and untyped dependencies in Kibana plugins. This PR fixes all of the plugins owned by @elastic/kibana-presentation.

Caveats

  • These changes have had only rudimentary testing in the application. There may be side-effects, where a dependency is not explicitly used, but adds some necessary, implicit effect. These dependencies should at the very least be avoided, if not outright eliminated.

Follow-up

  • expression_* plugins should be consolidated and moved to a package.

controls

  • Dependencies for the server layer were not explicitly defined on the plugin class.
  • kibanaReact and presentationUtil were used statically, and there should be required bundles.
  • savedObjects and expressions were not used at all-- check for side-effects!
Screenshot 2023-12-19 at 5 38 08 PM

dashboard

  • controls, kibanaReact, kibanaUtils, presentationUtil, controls, and savedObjectsFinder are all used statically; they should be required bundles instead.
  • share is used optionally as a dependency and statically to import components. It was likely made required to avoid the build error, (optional plugins cannot be used statically). share was moved to both collections.
  • taskManager and usageCollection are optional plugins, but required in the TS types. The types were refactored to reflect the manifest.
  • dataViews was not used at all.
  • customBranding was listed in the Typescript type, but missing from the manifest, and unused. This was likely a leftover from the move to core, type removed
Screenshot 2023-12-19 at 5 42 36 PM

embeddable

  • Dependencies for both the public and server layer were not explicitly defined on the plugin class.
  • data and savedObjectsFinder are both used statically; they should be required bundles instead.
  • usageCollection was listed in the manifest as optional; updated type.
Screenshot 2023-12-20 at 11 49 30 AM

expression_*

  • The start dependencies relied on expression rather than expressionS. They were also entirely unused.
  • presentationUtil is used statically; it should be a required bundle instead.

inputControlVis

  • Dependencies for the public layer were not explicitly defined on the plugin class.
  • None of the public start dependencies are used.
  • visDefaultEditor is used statically; it should be a required bundle instead.
Screenshot 2023-12-20 at 11 54 24 AM

inspector

  • Dependencies for the public layer were not explicitly defined on the plugin class.
Screenshot 2023-12-20 at 11 56 00 AM

links

  • Dependencies for the public layer were not explicitly nor correctly defined on the plugin class.
  • uiActionsEnhanced and savedObjects are both used statically; they should be required bundles instead.
  • kibanaReact and kibanaUtils were unused.
  • usageCollection should be optional in the manifest.
Screenshot 2023-12-20 at 11 58 10 AM

presentationUtil

  • Dependencies for both the public and server layer were neither explicitly nor correctly defined on the plugin class.
  • savedObjects, kibanaReact, expressions and embeddable are all used statically; they should be required bundles instead.
Screenshot 2023-12-20 at 12 00 20 PM

canvas

  • Dependencies for the server layer were not explicitly nor correctly defined on the plugin class.
  • fieldFormats, savedObjectFinder and expression* are all used statically; they should be required bundles instead.
  • usageCollection should be optional in the manifest.
  • home should be optional in the Typescript type and in logic.
Screenshot 2023-12-20 at 12 03 07 PM

dashboardEnhanced

  • unifiedSearch is unused. check for side effects!
  • uiActionsEnhanced is used statically; it should be a required bundle instead.
Screenshot 2023-12-20 at 12 04 03 PM

embeddableEnhanced

  • kibanaReact and uiActions are unused.
Screenshot 2023-12-20 at 12 06 16 PM

@clintandrewhall clintandrewhall added review chore Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas loe:medium Medium Level of Effort release_note:skip Skip the PR/issue when compiling release notes impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. v8.13.0 labels Dec 20, 2023
@clintandrewhall clintandrewhall requested review from a team as code owners December 20, 2023 18:15
@botelastic botelastic bot added the Feature:Embedding Embedding content via iFrame label Dec 20, 2023
Copy link
Contributor

@ThomThomson ThomThomson left a comment

Choose a reason for hiding this comment

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

Changes LGTM! Ran some smoke tests locally and looked through the code changes. Great cleanup!

@clintandrewhall clintandrewhall enabled auto-merge (squash) January 5, 2024 03:16
@clintandrewhall clintandrewhall force-pushed the chore/presentation_plugins branch from 1dd70f1 to 5c00157 Compare January 9, 2024 20:20
@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

✅ unchanged

History

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Feature:Embedding Embedding content via iFrame impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. loe:medium Medium Level of Effort release_note:skip Skip the PR/issue when compiling release notes review Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas v8.13.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants