Skip to content

[ResponseOps] consistent-type-imports linting rule for RO packages/plugins - PR1#212348

Merged
cnasikas merged 21 commits intoelastic:mainfrom
georgianaonoleata1904:type-imports-eslint-ro
Mar 11, 2025
Merged

[ResponseOps] consistent-type-imports linting rule for RO packages/plugins - PR1#212348
cnasikas merged 21 commits intoelastic:mainfrom
georgianaonoleata1904:type-imports-eslint-ro

Conversation

@georgianaonoleata1904
Copy link
Contributor

@georgianaonoleata1904 georgianaonoleata1904 commented Feb 25, 2025

  • Enabled @typescript-eslint/consistent-type-imports eslint rule for ResponseOps packages and plugins:

    • this rule ensures that imports used only for type declarations are consistently written using import type syntax
  • fixed eslint errors for:

    • src/platform/packages/shared/response-ops
    • x-pack/platform/plugins/shared/alerting

@georgianaonoleata1904 georgianaonoleata1904 self-assigned this Mar 4, 2025
@georgianaonoleata1904 georgianaonoleata1904 added Team:ResponseOps Platform ResponseOps team (formerly the Cases and Alerting teams) t// backport:version Backport to applied version labels v9.1.0 v8.19.0 release_note:skip Skip the PR/issue when compiling release notes labels Mar 4, 2025
@georgianaonoleata1904 georgianaonoleata1904 marked this pull request as ready for review March 4, 2025 09:28
@georgianaonoleata1904 georgianaonoleata1904 requested review from a team as code owners March 4, 2025 09:28
@elasticmachine
Copy link
Contributor

Pinging @elastic/response-ops (Team:ResponseOps)

@georgianaonoleata1904 georgianaonoleata1904 changed the title WIP - [ResponseOps] consistent-type-imports linting rule [ResponseOps] consistent-type-imports linting rule for RO packages/plugins - PR1 Mar 4, 2025
@georgianaonoleata1904
Copy link
Contributor Author

The is part 1/4. The following PRs have related changes:
#212366
#212498
#212508

Copy link
Contributor

@adcoelho adcoelho left a comment

Choose a reason for hiding this comment

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

This is a nice rule to enable, I had no idea we did this in so many places. 👍

@gsoldevila
Copy link
Contributor

Related to #212221

@gsoldevila
Copy link
Contributor

gsoldevila commented Mar 4, 2025

I think it is worth addressing this globally for the whole repo, I'll create an issue for it.

If we really want to add this ESLint constraint, best strategy, as Matt suggests, would be a divide & conquer approach:

  • Make the ESLint rules a warning by default.
  • Create a list of all Kibana modules (~800 atm), grouped by owning team.
  • Add this list with each line commented out, as an override in .eslintrc.json, turning the warning into an error.
  • Create as many PRs as owning teams, and in each PR uncomment the corresponding modules in the eslint overrides.
  • Address issues with the import/no-duplicates rule, e.g. import type { …, type XXX, }.

Copy link
Member

@umbopepato umbopepato left a comment

Choose a reason for hiding this comment

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

Super happy that we're enabling this rule, it'll help a lot. Thank you Georgiana! 🤗
LGTM, just left a comment where a type import is missing

import { FieldsMetadataPublicStart } from '@kbn/fields-metadata-plugin/public';
import {
import type { ActionConnector, ActionTypeRegistryContract } from '@kbn/alerts-ui-shared';
import type {
Copy link
Member

Choose a reason for hiding this comment

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

Seems like the FieldsMetadataPublicStart type import was lost here 🙂

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, it was lost at merge. Fixed it, thanks! :)

Copy link
Contributor

@jeramysoucy jeramysoucy left a comment

Choose a reason for hiding this comment

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

Kibana security changes LGTM - no changes to ESO registration/model versions

@georgianaonoleata1904
Copy link
Contributor Author

/ci

@cnasikas
Copy link
Member

@elasticmachine merge upstream

@cnasikas cnasikas enabled auto-merge (squash) March 11, 2025 10:04
@cnasikas cnasikas merged commit ec3b186 into elastic:main Mar 11, 2025
10 checks passed
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

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

@elasticmachine
Copy link
Contributor

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #108 / discover/group1 discover accessibility top nav menu buttons should return focus to the alerts button when dismissing the alerts popover

Metrics [docs]

✅ unchanged

History

cc @georgianaonoleata1904

@kibanamachine
Copy link
Contributor

💔 All backports failed

Status Branch Result
8.x Backport failed because of merge conflicts

You might need to backport the following PRs to 8.x:
- [ResponseOps][OnWeek] Investigate isolating flapping feature in the code (#212454)
- [ResponseOps][MW] Use date format from settings in MW UI (#211576)

Manual backport

To create the backport manually run:

node scripts/backport --pr 212348

Questions ?

Please refer to the Backport tool documentation

@georgianaonoleata1904
Copy link
Contributor Author

💚 All backports created successfully

Status Branch Result
8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

georgianaonoleata1904 added a commit to georgianaonoleata1904/kibana that referenced this pull request Mar 11, 2025
…ugins - PR1 (elastic#212348)

- Enabled @typescript-eslint/consistent-type-imports eslint rule for
ResponseOps packages and plugins:
- this rule ensures that imports used only for type declarations are
consistently written using `import type ` syntax

- fixed eslint errors for:
    - `src/platform/packages/shared/response-ops`
    - `x-pack/platform/plugins/shared/alerting`

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
(cherry picked from commit ec3b186)

# Conflicts:
#	x-pack/platform/plugins/shared/alerting/public/pages/maintenance_windows/components/fields/date_picker_range_field.test.tsx
#	x-pack/platform/plugins/shared/alerting/public/pages/maintenance_windows/components/fields/date_picker_range_field.tsx
#	x-pack/platform/plugins/shared/alerting/public/pages/maintenance_windows/components/maintenance_windows_list.test.tsx
#	x-pack/platform/plugins/shared/alerting/public/pages/maintenance_windows/components/recurring_schedule_form/custom_recurring_schedule.tsx
#	x-pack/platform/plugins/shared/alerting/public/pages/maintenance_windows/components/recurring_schedule_form/recurring_schedule.tsx
#	x-pack/platform/plugins/shared/alerting/public/pages/maintenance_windows/components/upcoming_events_popover.tsx
#	x-pack/platform/plugins/shared/alerting/public/pages/maintenance_windows/helpers/convert_to_rrule.ts
#	x-pack/platform/plugins/shared/alerting/public/pages/maintenance_windows/helpers/parse_schedule.ts
#	x-pack/platform/plugins/shared/alerting/server/alerts_client/alerts_client.ts
#	x-pack/platform/plugins/shared/alerting/server/alerts_client/legacy_alerts_client.ts
#	x-pack/platform/plugins/shared/alerting/server/alerts_client/lib/get_summarized_alerts_query.ts
#	x-pack/platform/plugins/shared/alerting/server/alerts_client/lib/inject_analyze_wildcard.ts
#	x-pack/platform/plugins/shared/alerting/server/alerts_client/lib/sanitize_bulk_response.test.ts
#	x-pack/platform/plugins/shared/alerting/server/alerts_service/default_lifecycle_policy.ts
#	x-pack/platform/plugins/shared/alerting/server/alerts_service/lib/create_concrete_write_index.test.ts
#	x-pack/platform/plugins/shared/alerting/server/alerts_service/lib/create_concrete_write_index.ts
#	x-pack/platform/plugins/shared/alerting/server/alerts_service/lib/create_or_update_component_template.test.ts
#	x-pack/platform/plugins/shared/alerting/server/alerts_service/lib/create_or_update_component_template.ts
#	x-pack/platform/plugins/shared/alerting/server/alerts_service/lib/create_or_update_ilm_policy.ts
#	x-pack/platform/plugins/shared/alerting/server/alerts_service/lib/create_or_update_index_template.ts
#	x-pack/platform/plugins/shared/alerting/server/application/rule/methods/aggregate/types/index.ts
#	x-pack/platform/plugins/shared/alerting/server/authorization/alerting_authorization_kuery.ts
#	x-pack/platform/plugins/shared/alerting/server/lib/determine_delayed_alerts.ts
#	x-pack/platform/plugins/shared/alerting/server/lib/flapping/delay_recovered_flapping_alerts.ts
#	x-pack/platform/plugins/shared/alerting/server/lib/flapping/determine_flapping_alerts.ts
#	x-pack/platform/plugins/shared/alerting/server/lib/flapping/set_flapping_history_and_tracked_alerts.test.ts
#	x-pack/platform/plugins/shared/alerting/server/lib/flapping/set_flapping_history_and_tracked_alerts.ts
#	x-pack/platform/plugins/shared/alerting/server/lib/flapping_utils.ts
#	x-pack/platform/plugins/shared/alerting/server/lib/get_execution_log_aggregation.ts
#	x-pack/platform/plugins/shared/alerting/server/lib/process_alerts.test.ts
#	x-pack/platform/plugins/shared/alerting/server/lib/process_alerts.ts
#	x-pack/platform/plugins/shared/alerting/server/lib/rule_gaps/find_gaps.ts
#	x-pack/platform/plugins/shared/alerting/server/lib/rule_gaps/types/index.ts
#	x-pack/platform/plugins/shared/alerting/server/lib/rule_gaps/update/update_gaps.ts
#	x-pack/platform/plugins/shared/alerting/server/lib/set_flapping.test.ts
#	x-pack/platform/plugins/shared/alerting/server/lib/set_flapping.ts
#	x-pack/platform/plugins/shared/alerting/server/lib/to_raw_alert_instances.ts
#	x-pack/platform/plugins/shared/alerting/server/monitoring/register_cluster_collector.ts
#	x-pack/platform/plugins/shared/alerting/server/routes/index.ts
#	x-pack/platform/plugins/shared/alerting/server/rules_client/methods/get_action_error_log.ts
#	x-pack/platform/plugins/shared/alerting/server/rules_client/methods/get_execution_log.ts
#	x-pack/platform/plugins/shared/alerting/server/rules_client/tests/get_execution_log.test.ts
#	x-pack/platform/plugins/shared/alerting/server/saved_objects/partially_update_rule.test.ts
#	x-pack/platform/plugins/shared/alerting/server/task_runner/action_scheduler/action_scheduler.ts
#	x-pack/platform/plugins/shared/alerting/server/usage/lib/get_telemetry_from_alerts.ts
#	x-pack/platform/plugins/shared/alerting/server/usage/lib/get_telemetry_from_event_log.ts
#	x-pack/platform/plugins/shared/alerting/server/usage/lib/get_telemetry_from_kibana.ts
#	x-pack/platform/plugins/shared/alerting/server/usage/lib/get_telemetry_from_task_manager.ts
#	x-pack/platform/plugins/shared/alerting/server/usage/lib/group_connectors_by_consumers.ts
georgianaonoleata1904 added a commit that referenced this pull request Mar 11, 2025
…ges/plugins - PR1 (#212348) (#213929)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[ResponseOps] consistent-type-imports linting rule for RO
packages/plugins - PR1
(#212348)](#212348)

<!--- Backport version: 9.6.6 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sorenlouv/backport)

<!--BACKPORT [{"author":{"name":"Georgiana-Andreea
Onoleață","email":"georgiana.onoleata@elastic.co"},"sourceCommit":{"committedDate":"2025-03-11T12:56:45Z","message":"[ResponseOps]
consistent-type-imports linting rule for RO packages/plugins - PR1
(#212348)\n\n- Enabled @typescript-eslint/consistent-type-imports eslint
rule for\nResponseOps packages and plugins:\n- this rule ensures that
imports used only for type declarations are\nconsistently written using
`import type ` syntax\n \n- fixed eslint errors for:\n -
`src/platform/packages/shared/response-ops`\n -
`x-pack/platform/plugins/shared/alerting`\n\n---------\n\nCo-authored-by:
kibanamachine
<42973632+kibanamachine@users.noreply.github.com>","sha":"ec3b18662a74eae2315c08a045909250c45ab37a","branchLabelMapping":{"^v9.1.0$":"main","^v8.19.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Team:ResponseOps","backport:version","v9.1.0","v8.19.0"],"title":"[ResponseOps]
consistent-type-imports linting rule for RO packages/plugins -
PR1","number":212348,"url":"https://github.com/elastic/kibana/pull/212348","mergeCommit":{"message":"[ResponseOps]
consistent-type-imports linting rule for RO packages/plugins - PR1
(#212348)\n\n- Enabled @typescript-eslint/consistent-type-imports eslint
rule for\nResponseOps packages and plugins:\n- this rule ensures that
imports used only for type declarations are\nconsistently written using
`import type ` syntax\n \n- fixed eslint errors for:\n -
`src/platform/packages/shared/response-ops`\n -
`x-pack/platform/plugins/shared/alerting`\n\n---------\n\nCo-authored-by:
kibanamachine
<42973632+kibanamachine@users.noreply.github.com>","sha":"ec3b18662a74eae2315c08a045909250c45ab37a"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.1.0","branchLabelMappingKey":"^v9.1.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/212348","number":212348,"mergeCommit":{"message":"[ResponseOps]
consistent-type-imports linting rule for RO packages/plugins - PR1
(#212348)\n\n- Enabled @typescript-eslint/consistent-type-imports eslint
rule for\nResponseOps packages and plugins:\n- this rule ensures that
imports used only for type declarations are\nconsistently written using
`import type ` syntax\n \n- fixed eslint errors for:\n -
`src/platform/packages/shared/response-ops`\n -
`x-pack/platform/plugins/shared/alerting`\n\n---------\n\nCo-authored-by:
kibanamachine
<42973632+kibanamachine@users.noreply.github.com>","sha":"ec3b18662a74eae2315c08a045909250c45ab37a"}},{"branch":"8.x","label":"v8.19.0","branchLabelMappingKey":"^v8.19.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
CAWilson94 pushed a commit to CAWilson94/kibana that referenced this pull request Mar 22, 2025
…ugins - PR1 (elastic#212348)

- Enabled @typescript-eslint/consistent-type-imports eslint rule for
ResponseOps packages and plugins:
- this rule ensures that imports used only for type declarations are
consistently written using `import type ` syntax
   
- fixed eslint errors for:
    - `src/platform/packages/shared/response-ops`
    - `x-pack/platform/plugins/shared/alerting`

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:version Backport to applied version labels release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Platform ResponseOps team (formerly the Cases and Alerting teams) t// v8.19.0 v9.1.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants