Skip to content

feat(eslint): add no_sync_import_from_plugin rule (closes #171080)#266636

Merged
afharo merged 6 commits intomainfrom
issue-171080-eslint-no-sync-plugin
May 5, 2026
Merged

feat(eslint): add no_sync_import_from_plugin rule (closes #171080)#266636
afharo merged 6 commits intomainfrom
issue-171080-eslint-no-sync-plugin

Conversation

@afharo
Copy link
Copy Markdown
Member

@afharo afharo commented Apr 30, 2026

⚠️ CI is failing until all plugins are migrated (2 tiny PRs pending to be approved).

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

afharo added 2 commits April 30, 2026 13:41
- Add @kbn/eslint/no_sync_import_from_plugin for plugin server/index.ts
- Register rule; wire @kbn/eslint-config override (off until migrations merge)
- Document pattern and rule in AGENTS.md

Closes #171080

Made-with: Cursor
Enforce the rule on plugin server/index.ts so CI reports violations; counts
should drop as lazy-load migrations merge. Update AGENTS.md.

Made-with: Cursor
@afharo afharo self-assigned this Apr 30, 2026
@afharo afharo linked an issue Apr 30, 2026 that may be closed by this pull request
@afharo afharo marked this pull request as ready for review May 1, 2026 06:07
@afharo afharo requested review from a team as code owners May 1, 2026 06:07
@afharo
Copy link
Copy Markdown
Member Author

afharo commented May 1, 2026

@elasticmachine merge upstream

@afharo afharo added release_note:skip Skip the PR/issue when compiling release notes backport:version Backport to applied version labels v9.4.0 labels May 1, 2026
@afharo afharo requested review from a team as code owners May 5, 2026 14:14
@tylersmalley tylersmalley added the reviewer:claude PR review and comments with Claude label May 5, 2026
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice rule — handles the import type / await import() distinction cleanly and tests cover the main invalid shapes (value import, mixed type+value, side-effect import, named re-export, export *, require()). One small scoping observation about the override globs left inline.

Other things I checked and found fine:

  • Default and namespace imports (import Foo from './plugin', import * as p from './plugin') fall through to context.report correctly via the spec.type === 'ImportSpecifier' check, so they're caught even though they aren't in the test fixtures.
  • excludedFiles: ['**/test/**'] correctly skips test-fixture plugins under src/platform/test/**/plugins/** and x-pack/platform/test/**/plugins/**.
  • The rule message is actionable and points at the migration PR.

'src/platform/plugins/**/server/index.ts',
'x-pack/platform/plugins/**/server/index.ts',
'x-pack/solutions/**/plugins/**/server/index.ts',
'examples/**/server/index.ts',
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The examples/**/server/index.ts glob only matches the repo-root examples/ tree, so the 5 plugin entries under x-pack/examples/ (e.g. alerting_example, screenshotting_example, triggers_actions_ui_example, gen_ai_streaming_response_example, third_party_vis_lens_example) are not enforced. If example plugins are intended to follow the same pattern, consider also covering x-pack/examples/.

Suggested change
'examples/**/server/index.ts',
'examples/**/server/index.ts',
'x-pack/examples/**/server/index.ts',

@kibanamachine
Copy link
Copy Markdown
Contributor

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

✅ unchanged

History

cc @afharo

@afharo afharo enabled auto-merge (squash) May 5, 2026 15:55
@afharo afharo merged commit 99f35fa into main May 5, 2026
35 checks passed
@afharo afharo deleted the issue-171080-eslint-no-sync-plugin branch May 5, 2026 15:55
@kibanamachine
Copy link
Copy Markdown
Contributor

Starting backport for target branches: 9.4

https://github.com/elastic/kibana/actions/runs/25387219855

@kibanamachine
Copy link
Copy Markdown
Contributor

@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label May 6, 2026
@kibanamachine
Copy link
Copy Markdown
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add a backport:* label or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 266636 locally
cc: @afharo

1 similar comment
@kibanamachine
Copy link
Copy Markdown
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add a backport:* label or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 266636 locally
cc: @afharo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport missing Added to PRs automatically when the are determined to be missing a backport. backport:version Backport to applied version labels release_note:skip Skip the PR/issue when compiling release notes reviewer:claude PR review and comments with Claude v9.4.0 v9.5.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add ES Lint rule to enforce the pattern introduced in #170856

6 participants