Skip to content

Commit 99f35fa

Browse files
feat(eslint): add no_sync_import_from_plugin rule (closes #171080) (#266636)
⚠️ CI is failing until all plugins are migrated (2 tiny PRs pending to be approved). - [x] #266620 - [x] #266619 ## Summary Implements the ESLint rule requested in #171080 to support the plugin server entry pattern from #170856: `server/index.ts` should not synchronously load `./plugin` (value imports, re-exports, side-effect `import './plugin'`, or `require('./plugin')`); `import type` / `export type` and dynamic `import()` are allowed. - New rule: `@kbn/eslint/no_sync_import_from_plugin` in `@kbn/eslint-plugin-eslint` with Jest tests. - `@kbn/eslint-config` override for plugin `server/index.ts` paths: rule is `error` so CI enforces the pattern; the number of violations should decrease as lazy-load `server/index.ts` migrations land. - `AGENTS.md`: document the pattern and the rule. **Closes** #171080 ## Review disclaimer This PR was drafted with AI assistance (Cursor / Composer). Please give it a thorough review—especially rule edge cases and interaction with in-flight migration PRs. Made with [Cursor](https://cursor.com) --------- Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
1 parent d80ae86 commit 99f35fa

7 files changed

Lines changed: 231 additions & 4 deletions

File tree

AGENTS.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
- Kibana is organized into modules, each defined by a `kibana.jsonc`: core, packages, and plugin packages. Aside from tooling and testing, most code lives in these modules.
88
- Packages are reusable units with explicit boundaries and a single public entry point (no subpath imports), usually with a focused purpose.
99
- Plugins are a package type (`type: "plugin"`) that include a plugin class with setup/start/stop lifecycles, utilized by the core platform to enable applications.
10+
- **Server plugin entry (`server/index.ts`)** should not load `./plugin` until the plugin may run. Use `import type` (and `export type`) for types from `./plugin`, keep shared config in `config.ts` / `../common/config` (not re-exported runtime values from `./plugin` at the entry), and instantiate the implementation with `await import('./plugin')` inside the async `plugin` initializer. Static value imports, `export { … }` / `export *` of values, `import './plugin'`, and `require('./plugin')` in that entry force Node to parse and execute `plugin.ts` even when the plugin is disabled. `@kbn/eslint/no_sync_import_from_plugin` in `@kbn/eslint-config` enforces this on plugin `server/index.ts` files (see [PR #170856](https://github.com/elastic/kibana/pull/170856) and [issue #171080](https://github.com/elastic/kibana/issues/171080)).
1011
- Plugins that depend on other plugins rely on the contracts returned by those lifecycles, so circular dependencies must be avoided.
1112
- Module IDs (typically `@kbn/...`) live in `kibana.jsonc`; `package.json` names are derived where present.
1213
- Plugin IDs are additional camelCase IDs under `plugin.id` in `kibana.jsonc`, used by core platform and other plugins.
@@ -57,6 +58,7 @@ Follow existing patterns in the target area first; below are common defaults.
5758
### Linting
5859
`node scripts/eslint --fix $(git diff --name-only)`
5960
- Never suppress linting errors with `eslint-disable`; fix the root cause.
61+
- Plugin `server/index.ts` files are checked by `@kbn/eslint/no_sync_import_from_plugin` (see plugin server entry note above).
6062

6163
### Formatting
6264
- Follow existing formatting in the file; do not reformat unrelated code.

packages/kbn-eslint-config/.eslintrc.js

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -400,4 +400,27 @@ module.exports = {
400400
'@elastic/eui/prefer-eui-icon-tip': 'error',
401401
'@elastic/eui/sr-output-disabled-tooltip': 'error',
402402
},
403+
404+
overrides: [
405+
{
406+
files: [
407+
'src/platform/plugins/**/server/index.ts',
408+
'x-pack/platform/plugins/**/server/index.ts',
409+
'x-pack/solutions/**/plugins/**/server/index.ts',
410+
'examples/**/server/index.ts',
411+
'packages/kbn-mock-idp-plugin/server/index.ts',
412+
],
413+
excludedFiles: ['**/test/**'],
414+
rules: {
415+
/**
416+
* Plugin server entry should not load ./plugin until the plugin is enabled.
417+
* @see https://github.com/elastic/kibana/pull/170856
418+
* @see https://github.com/elastic/kibana/issues/171080
419+
*
420+
* Enforced in CI; violation count should fall as lazy-load `server/index.ts` migrations land.
421+
*/
422+
'@kbn/eslint/no_sync_import_from_plugin': 'error',
423+
},
424+
},
425+
],
403426
};

packages/kbn-eslint-plugin-eslint/index.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,5 +39,6 @@ module.exports = {
3939
require_kbn_fs: require('./rules/require_kbn_fs'),
4040
require_include_in_check_a11y: require('./rules/require_include_in_check_a11y'),
4141
no_wrapped_error_in_logger: require('./rules/no_wrapped_error_in_logger'),
42+
no_sync_import_from_plugin: require('./rules/no_sync_import_from_plugin'),
4243
},
4344
};
Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the "Elastic License
4+
* 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
5+
* Public License v 1"; you may not use this file except in compliance with, at
6+
* your election, the "Elastic License 2.0", the "GNU Affero General Public
7+
* License v3.0 only", or the "Server Side Public License, v 1".
8+
*/
9+
10+
/** @typedef {import("eslint").Rule.RuleModule} Rule */
11+
12+
const MESSAGE =
13+
"Do not statically import or re-export values from './plugin' in the server plugin entry. Use `import type` for types only, move shared config to `config.ts` when needed, and load the implementation with `await import('./plugin')` so disabled plugins do not load this module. See https://github.com/elastic/kibana/pull/170856.";
14+
15+
/**
16+
* @param {string | undefined} source
17+
* @returns {boolean}
18+
*/
19+
function isPluginModuleSpecifier(source) {
20+
return source === './plugin' || source === '.\\plugin';
21+
}
22+
23+
/** @type {Rule} */
24+
module.exports = {
25+
meta: {
26+
type: 'problem',
27+
docs: {
28+
description:
29+
'Disallow static value imports and value re-exports from `./plugin` in plugin server entry files.',
30+
},
31+
schema: [],
32+
messages: {
33+
noSyncImportFromPlugin: MESSAGE,
34+
},
35+
},
36+
37+
create(context) {
38+
return {
39+
ImportDeclaration(node) {
40+
if (!node.source || !isPluginModuleSpecifier(node.source.value)) {
41+
return;
42+
}
43+
44+
if (node.importKind === 'type') {
45+
return;
46+
}
47+
48+
if (node.specifiers.length === 0) {
49+
context.report({ node, messageId: 'noSyncImportFromPlugin' });
50+
return;
51+
}
52+
53+
for (const spec of node.specifiers) {
54+
if (spec.type === 'ImportSpecifier' && spec.importKind === 'type') {
55+
continue;
56+
}
57+
context.report({ node: spec, messageId: 'noSyncImportFromPlugin' });
58+
return;
59+
}
60+
},
61+
62+
ExportNamedDeclaration(node) {
63+
if (!node.source || !isPluginModuleSpecifier(node.source.value)) {
64+
return;
65+
}
66+
67+
if (node.exportKind === 'type') {
68+
return;
69+
}
70+
71+
for (const spec of node.specifiers) {
72+
if (spec.type === 'ExportSpecifier' && spec.exportKind === 'type') {
73+
continue;
74+
}
75+
context.report({ node: spec, messageId: 'noSyncImportFromPlugin' });
76+
return;
77+
}
78+
},
79+
80+
ExportAllDeclaration(node) {
81+
if (!node.source || !isPluginModuleSpecifier(node.source.value)) {
82+
return;
83+
}
84+
85+
if (node.exportKind === 'type') {
86+
return;
87+
}
88+
89+
context.report({ node, messageId: 'noSyncImportFromPlugin' });
90+
},
91+
92+
CallExpression(node) {
93+
if (
94+
node.callee.type === 'Identifier' &&
95+
node.callee.name === 'require' &&
96+
node.arguments[0]?.type === 'Literal' &&
97+
typeof node.arguments[0].value === 'string' &&
98+
isPluginModuleSpecifier(node.arguments[0].value)
99+
) {
100+
context.report({ node, messageId: 'noSyncImportFromPlugin' });
101+
}
102+
},
103+
};
104+
},
105+
};
Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the "Elastic License
4+
* 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
5+
* Public License v 1"; you may not use this file except in compliance with, at
6+
* your election, the "Elastic License 2.0", the "GNU Affero General Public
7+
* License v3.0 only", or the "Server Side Public License, v 1".
8+
*/
9+
10+
const { RuleTester } = require('eslint');
11+
const rule = require('./no_sync_import_from_plugin');
12+
const dedent = require('dedent');
13+
14+
const ruleTester = new RuleTester({
15+
parser: require.resolve('@typescript-eslint/parser'),
16+
parserOptions: {
17+
sourceType: 'module',
18+
ecmaVersion: 2020,
19+
ecmaFeatures: {
20+
jsx: true,
21+
},
22+
},
23+
});
24+
25+
const ERR = { messageId: 'noSyncImportFromPlugin' };
26+
27+
ruleTester.run('@kbn/eslint/no_sync_import_from_plugin', rule, {
28+
valid: [
29+
{
30+
code: dedent`
31+
import type { PluginInitializerContext } from '@kbn/core/server';
32+
import type { MyPluginSetup } from './plugin';
33+
export const plugin = async (ctx: PluginInitializerContext) => {
34+
const { MyPlugin } = await import('./plugin');
35+
return new MyPlugin(ctx);
36+
};
37+
`,
38+
},
39+
{
40+
code: dedent`
41+
import { config } from './config';
42+
export const plugin = async () => {
43+
const { Plugin } = await import('./plugin');
44+
return new Plugin();
45+
};
46+
export { config };
47+
`,
48+
},
49+
{
50+
code: dedent`
51+
export type { FooSetup } from './plugin';
52+
`,
53+
},
54+
{
55+
code: dedent`
56+
import type { Foo } from './other';
57+
`,
58+
},
59+
{
60+
code: dedent`
61+
const m = await import('./plugin');
62+
`,
63+
},
64+
],
65+
66+
invalid: [
67+
{
68+
code: `import { FooPlugin } from './plugin';`,
69+
errors: [ERR],
70+
},
71+
{
72+
code: dedent`
73+
import type { Setup } from './plugin';
74+
import { FooPlugin } from './plugin';
75+
`,
76+
errors: [ERR],
77+
},
78+
{
79+
code: `import './plugin';`,
80+
errors: [ERR],
81+
},
82+
{
83+
code: `export { FooPlugin } from './plugin';`,
84+
errors: [ERR],
85+
},
86+
{
87+
code: `export * from './plugin';`,
88+
errors: [ERR],
89+
},
90+
{
91+
code: `require('./plugin');`,
92+
errors: [ERR],
93+
},
94+
],
95+
});

x-pack/platform/plugins/shared/agent_context_layer/server/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import type {
1212
AgentContextLayerSetupDependencies,
1313
AgentContextLayerStartDependencies,
1414
} from './types';
15-
import { AgentContextLayerPlugin } from './plugin';
1615

1716
export type {
1817
AgentContextLayerPluginSetup,
@@ -41,5 +40,6 @@ export const plugin: PluginInitializer<
4140
AgentContextLayerSetupDependencies,
4241
AgentContextLayerStartDependencies
4342
> = async (pluginInitializerContext: PluginInitializerContext) => {
43+
const { AgentContextLayerPlugin } = await import('./plugin');
4444
return new AgentContextLayerPlugin(pluginInitializerContext);
4545
};

x-pack/platform/plugins/shared/inbox/server/index.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import type {
1313
InboxSetupDependencies,
1414
InboxStartDependencies,
1515
} from './types';
16-
import { InboxPlugin } from './plugin';
1716

1817
export type { InboxPluginSetup, InboxPluginStart };
1918

@@ -22,7 +21,9 @@ export const plugin: PluginInitializer<
2221
InboxPluginStart,
2322
InboxSetupDependencies,
2423
InboxStartDependencies
25-
> = async (pluginInitializerContext: PluginInitializerContext<InboxConfig>) =>
26-
new InboxPlugin(pluginInitializerContext);
24+
> = async (pluginInitializerContext: PluginInitializerContext<InboxConfig>) => {
25+
const { InboxPlugin } = await import('./plugin');
26+
return new InboxPlugin(pluginInitializerContext);
27+
};
2728

2829
export { config } from './config';

0 commit comments

Comments
 (0)