[discuss] Allow imports from plugin common directories

[joshdover]

The Platform currently enforces (via lint rule) that plugins can only import code from other plugins from the public/index and server/index. Due to our other linting rules that prevent plugins from importing server code into client code, there is no way for a plugin to "publish" code that is safe to use on both the server and client.

There has been a proposal that we formalize the "common" directory that many plugins are already using, allowing other plugins to import code from this directory.

Plugins that are currently using the "common" pattern will need to evaluate the exports from these modules and ensure that they are all intended to be "public" to other plugins. Given that challenge, I propose we roll this out in phases:

1. Announce the intention to allow imports from common directories
2. Give teams time to update their exports or move their common directories entirely so they are not considered part of the plugin's public API. I think 1 month would be enough.
3. Update linter rules to allow common directories
4. Add documentation support for common directories

[streamich]

Would like to suggest to export "common" code from

src/plugins/<plugin>/index.ts

instead of

src/plugins/<plugin>/common/index.ts

with the idea that we can still keep as a convention to internally use /common folder but that is internal detail of the plugin, the "contract" would be in src/plugins/<plugin>/index.ts.

[joshdover]

In this approach, would we still have exports from public and server as well?

One benefit of the "common" convention is that it's a bit more clear that code shared from there is compatible with both server and client environments.

We also would like to have linting rules to prevent the common module from importing server-only or client-only code. It's kind of strange that with the top-level index idea, this module couldn't imoort from server or public.

That said, one thing I like the top-level index approach is that it side-steps the problem of having to refactor existing common directories.

[streamich]

In this approach, would we still have exports from public and server as well?

The top level index.ts would re-export items only from /common folder, i.e. this proposal does not suggest any changes to /server and /public folders. Nor it suggests to change the /common folder, it just suggests that the export "contract" of isomorphic code would be at the top level of the plugin.

One benefit of the "common" convention is that it's a bit more clear that code shared from there is compatible with both server and client environments.

One reason for suggesting this and also the impression I got from discussing this during our team sync, was that it is very clear that top level index.ts exports code that works in both environments.

We also would like to have linting rules to prevent the common module from importing server-only or client-only code.

Internally—inside the plugin—we can still keep the convention to use /commmon folder for isomorphic code and apply the linter rule to that folder.

That said, one thing I like the top-level index approach is that it side-steps the problem of having to refactor existing common directories.

Yes, that is one of the benefits.

Another benefit is that you can import common code without needing to add /common suffix, e.g:

import { FILTERS } from 'src/plugins/data';

Similar how you do not import React from react/common:

import React from 'react';

E.g. if our plugins were published to NPM, one could import isomorphic code easily:

import { ExpressionsService } from '@kbn/plugin-expressions';

[joshdover]

Internally—inside the plugin—we can still keep the convention to use /commmon folder for isomorphic code and apply the linter rule to that folder.

Makes sense, we can easily add a rule to ensure that the index does not import any values from server or public. I'm a +1 on this proposal.