[Discuss] Organising/grouping exports from plugins like kibana_utils

[Dosant]

Originally posted by @lukeelmers in #52280

We’ve been talking for awhile now about how static code is organized / exported from plugins, and two themes have frequently come up:

1. We should be doing named exports from our index files (instead of export *), otherwise it is too easy to accidentally leak internal apis as part of the public contract. [We've taken the initial steps toward fixing this in [Data Plugin]: Remove export * for common code from public/server index files #52821]

2. We should consider how we are “organizing” static exports. Core does this by prefixing most names with the name of the service they belong to. Another thing that has been tried recently is exporting the code under namespaces that match each service in a plugin.

So I think the motivation here is to make it more intuitive for folks consuming these utils to understand which ones are related.

That said, I have a few reservations with this PR that perhaps we should discuss further:

1. On the whole I support the idea of namespacing the static exports. But I think how we go about this is something we should agree to some conventions on, because if things aren’t consistent we will be no better off than we are today. (e.g. do we want to namespace multiple levels deep? Or just one? do we capitalize? And all those similar nit picky questions)

2. I’m not sure if we should create a designated state management grouping. I know this existed as a concept in the legacy world, but I don’t know if it is the right way to frame things in the new platform. The way I’ve been thinking about it is that syncState is basically an extension of the other store stuff, and the hash/unhash/encode/decode items are basic utils for manipulating urls. “State management” is now the job of apps.

TLDR - I guess what I am proposing is, instead of starting to change some of these imports now, perhaps we should re-export everything from legacy ui/state_management under their original names, until we can align on a design we think will work. (Otherwise we need to go back and change these again later). WDYT @Dosant @streamich ?

Maybe we should move this part of the discussion to the POC PR since it is ultimately design related? And perhaps cover the topic in our next app arch sync so we are aligned on a consistent approach? Since this effort isn’t super urgent, I’d rather take the time to make sure we sort these things out together.

Originally posted by @lukeelmers in #52280

[elasticmachine]

Pinging @elastic/kibana-app-arch (Team:AppArch)

[lukeelmers]

On the whole I support the idea of namespacing the static exports. But I think how we go about this is something we should agree to some conventions on, because if things aren’t consistent we will be no better off than we are today. (e.g. do we want to namespace multiple levels deep? Or just one? do we capitalize? And all those similar nit picky questions)

After a bunch of back and forth on this topic, it's time for us to choose a path forward for the time being. There are two approaches to consider here, both with pros and cons:

1. Prefixing

This involves prefixing every statically-exported item (types and static code) with the name of the corresponding service:

// src/plugins/data/public/index.ts
export {
  // static code / utils
  IndexPatternsFlattenHitWrapper,
  IndexPatternsValidate,
  // types
  IIndexPattern,
  IFieldList, // IIndexPatternFieldList? IndexPatternIFieldList?
} from './index_patterns';

// myPlugin.ts
import { IndexPatternsFlattenHitWrapper, IFieldList } from '/src/plugins/data/public';

Pros:

• This is consistent with the way core structures exports
• Very self-explanatory in terms of implementation; doesn't require anything special other than prefixing a name

Cons:

• More work to switch our current code over vs the namespacing approach
• Core doesn't really export static utils, but we do, so we are likely to have way more prefixed items which could result in some really awkward names (see example above)
• How to deal with types which already have an I* prefix as a convention? Feels weird to have two prefixes; where does the I go?

2. Namespacing

This involves creating a single static.ts file for each service, which exports all public types and static code for that service. The contents of this file are then exported under a namespace matching the corresponding service from the top level of the plugin:

// src/plugins/data/public/index_patterns/index.ts (public & internal stuff)
export * from './static';
export { someInternalThing } from './somewhere';
export { ISomeInternalType } from './types';

// src/plugins/data/public/index_patterns/types.ts (public & internal stuff)
export { IIndexPattern } from './somewhere';
export { IFieldList } from './somewhere';
export { ISomeInternalType } from './somewhere';

// src/plugins/data/public/index_patterns/static.ts (only public stuff)
export { IIndexPattern, IFieldList } from './types';
export { flattenHitWrapper } from './somewhere';
export { validate } from './somewhere';

// src/plugins/data/public/index.ts
export * as indexPatterns from './index_patterns/static';

// myPlugin.ts
import { indexPatterns } from '/src/plugins/data/public';
const fieldList: indexPatterns.IFieldList = [...];
indexPatterns.flattenHitWrapper(...);

Pros:

• Much easier to implement as it only involves grouping existing items
• Doesn't have the same naming challenges as the prefixing approach: instead of literally renaming everything that's public, you simply group it under the correct namespace.
• We've already tried this in a few places and it seems to be working okay so far, as long as we implement it consistently

Cons:

• A bit more plumbing to do across each of our services (need a static.ts and types.ts for each service).
• Source of truth for the public contract gets split out of public/index.ts, and instead is spread across static.ts files which live in each individual service.
• Doesn't answer the question about what to do with miscellaneous utils that don't belong to any particular service (I suppose those would just get exported without any namespacing). To be fair, this will be an issue for both approaches.

---

Personally, the more I think about this, I lean toward option 2. But if someone feels that option 1 is clearly the right approach, please do speak up 🙂

I'll plan on closing this issue with our decision early next week so we can move forward with the other migration-related items that need to happen. Please add any comments over the next couple of days.

[lizozom]

It seems like a major con of using namespaces is that it fails the documentation process at the moment.

Speaking to @rudolf, it seems like platform team ran into issues with this as well.

I'm going to try to find a workaround and will update.

[lukeelmers]

@lizozom Ah, yes this is a big problem. Having the generated documentation work is more important than the convention we choose here, so if there isn't a workaround it could be a dealbreaker for namespaces IMO.

[lizozom]

Following our team sync discussions and as demoed here, we decided to stick with prefixed exports in the majority of cases. It allows us to use ApiExtractor to generate docs and it also makes for a clearer and more consistent API.

We'll use namespaces in specific cases, where an area of service provides multiple utility functions \ classes (like filter helper functions).

And most important, until supported by both TS and ApiExtractor, we will avoid from exporting types\interfaces on namespaces.

[lukeelmers]

Closing with the resolution @lizozom summarizes above. Here's the issue where we are tracking implementation of the new static code grouping in the data plugin: #56881