[Enterprise Search] Move externalUrl helper out of React Context

[cee-chen]

Summary

Follow up to #78231 - This is the 3rd out of a set of 4 PRs focused on converting our current React Context (currently down to just KibanaContext!) into Kea/redux stores.

This PR focuses on the externalUrl helper (responsible for storing our public-facing Enterprise Search URLs, and helpers that form App Search vs Workplace Search URLs for example). However, it's a little unusual in that:

1. I decided not to move it into a Kea logic file (see 815b66e). I did so initially, but the logic just looked really odd/bare-bones, unlike the other ones which either updated its own state or at least had some kind of mount/unmount behavior.
   • Unlike the previous PRs, this helper does not ever need to be updated - it's simply a one-time only set obj that contains helper functions.
   • As such, I looked for different methods of accomplishing what I wanted and settled on using a plain JS obj (to store the public-facing enterpriseSearch URL) with a getter/setter that ensures we don't accidentally mutate said obj after initialization.
   • The benefit of this plain obj over a class is that it doesn't need to be instantiated or passed around - it can be imported flatly at any time (see 44bac94).
   • It's also possible we eventually (after the full Kibana migration) might not need this helper, so the simpler it is to delete the better.
2. It exposes a race condition issue with our Kibana header actions apps that will eventually occur with our Kea/Redux stores as well (3a8fccc). The issue is that the header app loads in first / before the main render app, and as such will crash because it's missing mounted props/data. The eventual solution (PR number 5) will be to move when the header component is rendered to after the main app.

QA/Regression testing

• App Search external links work exactly as before (side nav: Account Settings, Role Mappings, Credentials)
• Workplace Search external links work exactly as before (e.g., header action that takes you to /ws/search)

Checklist

• Unit or functional tests were updated or added to match the most common scenarios

[kibanamachine]

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request
• Commit: 9d993d8

Metrics

async chunks size

id	value	diff	baseline
enterpriseSearch	503.3KB	-327.0B	503.6KB

page load bundle size

id	value	diff	baseline
enterpriseSearch	21.6KB	-2.3KB	23.9KB

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

[scottybollinger]

This is fantastic. So much cleaner! Thanks for doing this!

[JasonStoltz]

Hey Constance, general question. Why aren't we just storing external Url in AppLogic?

[cee-chen]

Why aren't we just storing external Url in AppLogic?

AppLogic is product-specific (AS & WS contain their own separate AppLogic files), and the enterpriseSearchUrl / URL helpers are shared by multiple products.

I mean, we certainly could store it in AppLogic if we wanted to, but at this point it's also just as easy to do an import { getEnterpriseSearchUrl } from './shared/enterprise_search_url'; and skip calling useValues() or importing AppLogic, so there's not much extra benefit there.

Also, for why I decided against giving it its own top-level Kea logic file:

I [moved it into a Kea logic file] initially, but the logic just looked really odd/bare-bones, unlike the other ones which either updated its own state or at least had some kind of mount/unmount behavior.
Unlike the previous PRs, this helper does not ever need to be updated - it's simply a one-time only set obj that contains helper functions.

[JasonStoltz]

Just thinking out loud.

plugin.ts currently fetches all of the configuration data for Enterprise Search and passes it down via props to each one of the top level applications.

WorkplaceSearch and AppSearch both receive the same initial configuration data and store it in their respective AppLogic stores. The data is then accessed from those stores by individual components. The data flows down in this case.

With this new Singleton, data is no longer flowing down the tree. Since you're initializing it into a globally available store, you open up yourself to race conditions, like the one you're experiencing now. It's also creating yet another place to store configuration data.

I'm just wondering out loud if it wouldn't be better to fetch the data in plugin.ts, and then pass it down to the individual applications (including headers) and have them use it as needed as we do with other configuration data.

EDIT: So maybe even if the top level logic looked a little bare bones, it would at least be giving us some consistency, which might help with simplicity and understanding.

[cee-chen]

race conditions, like the one you're experiencing now

That's not quite correct. It's not a race condition within the main app. It's a race condition within completely separate React apps. Even if we moved externalUrl to a logic file, we'd run into the exact same issue trying to obtain the externalUrl/enterpriseSearchUrl value from <WorkplaceSearchHeaderActions />: the header app renders before the main app, which is in charge of mounting all our logic.

That is the race condition that needs to be fixed, and which I'm working on/have a solution for locally (I just need these series of PRs to merge in first :). I'm happy to hop on a screenshare to demo it (or link to a remote branch here in a second, once I clean up my commit history). The solution that I came up with (and is the most straightforward, IMO) is moving the header app to be rendered by/after the main app. This is also a paradigm other plugins in the Kibana ecosystem use - you can see in the first result of that search that other plugins are passing the setHeaderActionMenu into their main apps, not registering it from plugin.ts (which is fine if your header menu is super simple and doesn't need to share any state w/ your main app, but for most production apps that's not the case).

and then pass it down to the individual applications (including headers) and have them use it as needed as we do with other configuration data.

The problem with this is it doesn't scale. What happens when we need more than just enterpriseSearchUrl from our future header menus? For example, what happens if we need http from HttpLogic, or need to update state from a component-specific store in the main app?

We're now essentially forced to pass down basically every single param/kibana dep into renderHeaderActions as well as renderApp just to maintain parity. IMO, that's super unnecessary, and it makes far more sense to simply make renderApp our single point of dependency loading and ensure that all/any other separate apps that depend on its store are loaded/rendered afterwards.

[JasonStoltz]

The solution that I came up with (and is the most straightforward, IMO) is moving the header app to be rendered by/after the main app.

Great. That makes sense to me. Then the header would have access to all of our existing Logic stores, which sounds like an ideal solution.

What I'm leery of is storing externalUrl in external_url.ts, rather than in Kea, along with our other app configuration.

[JasonStoltz]

Maybe it will make more sense to me when you put up your next PR.

[cee-chen]

What I'm leery of is storing externalUrl in external_url.ts, rather than in Kea, along with our other app configuration.

Gotcha, that's definitely fair. I could potentially move it to (the new upcoming) KibanaLogic file, but it felt weird there for the following reasons:

1. This isn't anything being generated by Kibana or passed to us by Kibana - it's all our entirely data (a public-facing static URL from the Enterprise Search API endpoint) and our helpers.

2. It's never updated at any point after the point we fetch it on plugin init. There is never really any scenario in which we would ever manually update it or expect it to be updated dynamically rather than in a full server restart.
   That being said, you could make the same "it never updates" argument for the kibana config obj as well. I stored that in KibanaLogic because it felt like it "belonged" to Kibana, but that's definitely a hard-to-categorize one.

3. Another reason I kept it separate is that I am hoping we can eventually lose the helper completely once we're fully on Kibana and are never linking out to the standalone UI. The only exceptions I can see to that is our Reference/Search UI preview/downloader, and potentially the Workplace Search search experience (the one that goes to /ws/search - not sure what the plan is there).
   Once we remove the helper, my best guess is that we would then move to storing publicUrl in our AppLogic files for when we need them (e.g.: to replace API URL, to generate a link to Reference UI previews). If you think it's safer, I can start passing that to via initialAppData now so we have it (vs pulling it out).

For now though, this flat helper makes life a little bit nicer when generating our external URLs to the standalone UI - we just pass in route params to getAppSearchUrl(CREDENTIALS_PATH) for example, and don't have to think further about it or manually append /as every time. And when we eventually delete the helper, flatter imports also makes for fewer lines to delete :)

[JasonStoltz]

Sounds good. We'll see how things shakes out.

[cee-chen]

Awesome, thanks a ton Jason as always for the sanity checks. I should have definitely mentioned I had the same hesitations earlier and went back and forth as well myself. In the end I'm definitely looking forward to the follow-up PR to this where we can just delete the whole folder 😁