Move ExpressionRenderer to setup instead of start

[chrisdavies]

When working on Lens, we ran into an issue where we were unable to access ExpressionRenderer in our plugin's setup method. Our plugin's start method was never called (e.g. when embedding in dashboards). This moves the ExpressionRenderer to setup so that plugins can use it in their setup.

Dev Docs

Plugins may now access the expression renderer in their setup phase as well as their start phase.

[elasticmachine]

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request

[wylieconlon]

This change is fine, but you've definitely removed ExpressionRenderer from the start phase- I think Lens is the only consumer for now, but please update the dev docs which say that it's available in both setup and start

[flash1293]

I'm not sure whether that's the right route.

Our plugin's start method was never called (e.g. when embedding in dashboards). This moves the ExpressionRenderer to setup so that plugins can use it in their setup.

That was a small error on my part, sorry for the confusion. I forgot to call our shimmed start method in the embeddable entrypoint, that's why it didn't show up. @wylieconlon fixed this in #45171 and I independently added the same fix in a commit to the functional tests PR (https://github.com/elastic/kibana/pull/44814/files#diff-928c42b094a3ca517fe9859b94d75aab)

That approach in this PR works with the current code, but it's also dirty because it's moving the lifecycle problem to another place - with this setup the expression loader is exposed through the renderer before the inspector is registered.

The code of the loader is operating under the assumption that the inspector dependency is available which isn't the case anymore if you can access it in the setup phase (for example in src/legacy/core_plugins/data/public/expressions/lib/loader.ts line 84 - if the inspector isn't registered, this will throw a null pointer). It makes sense to set it up like this, because the inspector depends on the flyout service, and you shouldn't open flyouts in the setup phase.

[chrisdavies]

Closing since @wylieconlon said that my info was wrong. Setup is the method that is being discouraged, and start is the one that is going to be most common. If that's true, then we should keep the code as is.

[ppisljar]

ExpressionRenderer (or ExpressionDataLoder) should not be exposed in the setup lifecycle method.
in the setup method addFunction and addRenderer are exposed. Plugins can register their functions in the setup method. If you would to use ExpressionRenderer, there is no guarantee which plugins executed before yours and were able to register their functions and which weren't. This way you could get in weird errors when some functions would not exist when you would try to execute the expression.

For this reason ExpressionRenderer (and all the other functions allowing you to execute expressions) are only available in the start lifecycle method, when we can be sure all the plugins had a chance to register their functions. In the start lifecycle method adding to the registries is no longer possible.