[ESUI Shared] Exporting modules from public

[jloleysens]

Summary

The ESUI team would like to introduce namespaces for our cross plugin modules.

The problem

The ESUI team has a special plugin named es_ui_shared which houses, currently, modules we consume for public. The directory structure is:

es_ui_shared
  public
    <kludge of modules>

Code that uses this directory imports the modules like so (which satisfies our eslint enforced pattern):

import { /* many things indeed */ } from '<relative_path_to_es_ui_shared>/es_ui_shared/public'

We would like to migrate to the following structure:

import { /* a lot fewer things */ } from '<relative_path_to_es_ui_shared>/es_ui_shared/public/my_module'

Or something similar - essentially a way to namespace our modules we export. The alternative at the moment is to create namespaces at import site or to // eslint-disable (which would not be preferable).

CC @sebelga

[elasticmachine]

Pinging @elastic/es-ui (Team:Elasticsearch UI)

[elasticmachine]

Pinging @elastic/kibana-operations (Team:Operations)

[spalger]

We are actually going to be further limiting the imports that plugins can have to each other and only supporting imports of the root public directory in the build system, not in eslint where it can just be ignored. This way bundles from different plugins can be shared when they're served and loaded. The effort I'm describing is going to conflict with the effort in this issue, and while everything is code and anything is possible, I'm wondering if you could help me understand the motivation behind this and why it might justify adding an exclusion to the mechanism we're building to share bundles amongst all plugins.

[sebelga]

The es_ui_shared plugin is our shared place to export reusable static code across all of our apps. For that reason, it contains "packages" of code totally unrelated one with the other (forms utils, auth utils, console utils...). As we continue to add packages to this folder it will soon become difficult to export everything from one single index.ts and we will run into name conflicts.

This is the current state of the public/index.ts

// public/index.ts

export { JsonEditor, OnJsonEditorUpdateHandler } from './components/json_editor';

export { SectionLoading } from './components/section_loading';

export { CronEditor, MINUTE, HOUR, DAY, WEEK, MONTH, YEAR } from './components/cron_editor';

export {
  SendRequestConfig,
  SendRequestResponse,
  UseRequestConfig,
  UseRequestResponse,
  sendRequest,
  useRequest,
} from './request/np_ready_request';

export { indices } from './indices';

export { useUIAceKeyboardMode } from './use_ui_ace_keyboard_mode';

export {
  installXJsonMode,
  XJsonMode,
  ElasticsearchSqlHighlightRules,
  addXJsonToRules,
  ScriptHighlightRules,
  XJsonHighlightRules,
  collapseLiteralStrings,
  expandLiteralStrings,
} from './console_lang';

export {
  AuthorizationContext,
  AuthorizationProvider,
  NotAuthorizedSection,
  WithPrivileges,
  Privileges,
  MissingPrivileges,
  SectionError,
  Error,
  useAuthorizationContext,
} from './authorization';

We would much prefer to be able to namespace and export from top-level "domain" folders

import { AuthorizationContext, Error } from '../plugings/es_ui_shared/authorization';  

In the above public/index.ts you can see that we already namespace indices, and exported an object for it

// indices/index.ts
import { INDEX_ILLEGAL_CHARACTERS_VISIBLE } from './constants';

import {
  indexNameBeginsWithPeriod,
  findIllegalCharactersInIndexName,
  indexNameContainsSpaces,
} from './validate';

export const indices = {
  INDEX_ILLEGAL_CHARACTERS_VISIBLE,
  indexNameBeginsWithPeriod,
  findIllegalCharactersInIndexName,
  indexNameContainsSpaces,
};

But for Typescript types and interfaces we can't do that.

[cjcenizal]

for Typescript types and interfaces we can't do that.

@sebelga Do we have any examples of colliding types/interfaces? It seems like it would be a bug in our type system if we publish types that collide (e.g. types that are too generic or at least too generically named). I'm just trying to find out if a reasonable path forward is to export everything from the root using the object-namespacing pattern you shared in your last example, which is feasible as long as our types don't collide.

[spalger]

But for Typescript types and interfaces we can't do that.

What if we imported and then re-exported the whole namespace?

// public/indices/index.ts

export { INDEX_ILLEGAL_CHARACTERS_VISIBLE } from './constants';

export {
  indexNameBeginsWithPeriod,
  findIllegalCharactersInIndexName,
  indexNameContainsSpaces,
} from './validate';

// public/index.ts

import * as Indices from './indices'
export { Indices };

Use it as:

import { Indices } from '../plugins/es_ui_shared/public'
type X = Incices.SomeType

This supports all exported types from the inner index.ts file and makes it responsible for what is exported and what isn't.

[sebelga]

It seems like it would be a bug in our type system if we publish types that collide (e.g. types that are too generic or at least too generically named).

@cjcenizal no, it is perfectly normal. Take for example npm modules. Different modules might export a Request interface. Then the consuming code decides which interface it expects, be it from module A or B.

import { Request } from 'axios'; // this one?
import { Request } from 'super-http-client'; // or this one?

export const myHandler = (req: Request) => { ... }

What if we imported and then re-exported the whole namespace?

@spalger Yeah that'd be the only solution if we can't have the top-level folder to namespace. Not as nice as being able to do this

import { SomeType } from '../plugins/es_ui_shared/public/indices'

but if this is the only solution to move forward and allow "bundles from different plugins can be shared when they're served and loaded.", we can do that.
The only thing is... this plugin will never be served and loaded 😊 (it does not contain server or UI plugin.ts file)

[jloleysens]

Yeah I think that is a reasonable solution since our main concern is namespacing, thanks for the suggestion @spalger ! I had a feeling just ignoring those lint rules would be dangerous 😉.

Happy to move forward with the pattern proposed by @spalger too. Closing this issue for now.

[spalger]

The only thing is... this plugin will never be served and loaded 😊 (it does not contain server or UI plugin.ts file)

We actually had to add one a couple weeks ago: https://github.com/elastic/kibana/blob/master/src/plugins/es_ui_shared/public/index.ts#L62-L68

[cjcenizal]

Take for example npm modules. Different modules might export a Request interface. Then the consuming code decides which interface it expects, be it from module A or B.

Spencer's suggestion renders my point moot but if we have modules exporting different Request interfaces from ES UI shared, then I would request (ha! pun) that we give them names to differentiate them. That's a good example of what I would consider too generic a name.

[sebelga]

then I would request (ha! pun) that we give them names to differentiate them

I disagree with this, npm package publishers don't worry about other packages when they export their interfaces. We shouldn't either. The naming should come naturally, and we should not be forced to add a suffix to be sure the name won't collide in the future. The context is already given by the folder name, package name or in the solution from Spencer from the namespace we export.