elastic / kibana

Your window into the Elastic Stack
https://www.elastic.co/products/kibana
Other
19.35k stars 7.98k forks source link

[Reporting] Double-down on Screenshotting plugin being an optional dependency #182987

Open tsullivan opened 1 week ago

tsullivan commented 1 week ago

Screenshotting is an optional dependency for Reporting, but we may consider wanting to remove the Screenshotting plugin from serverless builds entirely. To support that, we should not import static code from Screenshotting in Reporting and move those imports to a package, instead. Optionally, we can move the Screenshotting types to a package, however that code gets compiled away and will not impact the build. We should move the Screenshotting types to a package, to sanitize Reporting's tsconfig.json file from any direct references to the Screenshotting plugin.🧴

More imports exist in test code, which I haven't listed here.

elasticmachine commented 1 week ago

Pinging @elastic/appex-sharedux (Team:SharedUX)

tsullivan commented 1 week ago

We should move the Screenshotting types to a package, to sanitize Reporting's tsconfig.json file from any direct references to the Screenshotting plugin.

@jbudz can you weigh in on this? Would this be helpful for slimming down the background task nodes in Kibana?

jbudz commented 1 week ago

I'm not sure - with the plugin already disabled I think it's more a matter of being able to clear out unused code and dependencies for a smaller distribution size. @lukeelmers @pgayvallet do you have any thoughts?

lukeelmers commented 1 week ago

Hah, I sent Tim to ask you about this because I wasn't sure @jbudz 🙂

Yes, overall I think Tim had opened this issue because I was asking if we could further reduce the distribution size by stripping out the whole screenshotting plugin, instead of needing to pinpoint specific files. But it turns out there are a few type & static code dependencies between reporting > screenshotting.

So I was trying to remember 2 things:

  1. Am I correct in my recollection that we are still allowing static code imports across plugins even if a plugin is disabled? If that's the case I would imagine that by removing any code imports from screenshotting and relocating them to a shared package, we could possibly reduce the size of the distribution further (because we could entirely strip out the screenshotting plugin while only adding a [theoretically] small amount of package code).
  2. Am I correct in assuming we are stripping these assets out after the build? (In other words, cross-plugin TypeScript dependencies don't matter after all because they will have been compiled away?)