Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Spacetime] Enforce consistent type import rule in team's owned plugins #204412

Closed
miloszmarcinkowski opened this issue Dec 16, 2024 · 2 comments · Fixed by #204549
Closed

[Spacetime] Enforce consistent type import rule in team's owned plugins #204412

miloszmarcinkowski opened this issue Dec 16, 2024 · 2 comments · Fixed by #204549
Assignees
Labels
Spacetime Team:obs-ux-infra_services Observability Infrastructure & Services User Experience Team

Comments

@miloszmarcinkowski
Copy link
Contributor

We recognize inconsistency in using Type-Only Imports across team's owned plugins. To enforce the same code standards, add consistent-type-imports eslint rule and apply to exisisting imports.

@botelastic botelastic bot added the needs-team Issues missing a team label label Dec 16, 2024
@miloszmarcinkowski miloszmarcinkowski self-assigned this Dec 16, 2024
@miloszmarcinkowski miloszmarcinkowski added the Team:obs-ux-infra_services Observability Infrastructure & Services User Experience Team label Dec 16, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/obs-ux-infra_services-team (Team:obs-ux-infra_services)

@miloszmarcinkowski
Copy link
Contributor Author

See results in PR #204549

viduni94 pushed a commit to viduni94/kibana that referenced this issue Jan 2, 2025
…eam` owned plugins (elastic#204549)

Closes elastic#204412

## Summary

This PR enforces `consistent-type-imports` eslint rule in
`x-pack/solutions/observability/plugins/*` plugins owned by
`obs-ux-infra_services-team`.

Detailed list of plugins:
- `x-pack/solutions/observability/plugins/apm`,
- `x-pack/solutions/observability/plugins/apm_data_access`,
- `x-pack/solutions/observability/plugins/infra`,
- `x-pack/solutions/observability/plugins/inventory`,
- `x-pack/solutions/observability/plugins/metrics_data_access`,
- `x-pack/solutions/observability/plugins/profiling`,
- `x-pack/solutions/observability/plugins/profiling_data_access`

Script for fixing eslint rules in above plugins:
```bash
yarn eslint --no-error-on-unmatched-pattern --quiet --fix "x-pack/solutions/observability/plugins/apm/**/*.{js,mjs,ts,tsx}" "x-pack/solutions/observability/plugins/apm_data_access/**/*.{js,mjs,ts,tsx}" "x-pack/solutions/observability/plugins/infra/**/*.{js,mjs,ts,tsx}" "x-pack/solutions/observability/plugins/inventory/**/*.{js,mjs,ts,tsx}" "x-pack/solutions/observability/plugins/metrics_data_access/**/*.{js,mjs,ts,tsx}" "x-pack/solutions/observability/plugins/profiling/**/*.{js,mjs,ts,tsx}" "x-pack/solutions/observability/plugins/profiling_data_access/**/*.{js,mjs,ts,tsx}"
```

## Results

The affected plugins have been profiled using `node
scripts/build_kibana_platform_plugins.js --dist --profile --focus=apm
--no-cache` command and bundle size checked manually (`du -s` command).

#### APM plugin
Zero benefits in terms of size.
<img width="1728" alt="Screenshot 2024-12-19 at 12 18 36"
src="https://github.com/user-attachments/assets/d86be5d8-a4ad-4d9c-bac1-110a0c6bba81"
/>

#### Infra plugin
Zero benefits in terms of size.
<img width="1728" alt="Screenshot 2024-12-19 at 12 56 08"
src="https://github.com/user-attachments/assets/410bc068-1f20-4de8-ac4e-89c74478ec59"
/>

#### Profiling plugin
Zero benefits in terms of size.
<img width="1725" alt="image"
src="https://github.com/user-attachments/assets/bf47c319-0716-4a5b-9858-94ce7d2a3251"
/>

## Conclusions

- Using [type-only
imports](https://www.typescriptlang.org/docs/handbook/release-notes/typescript-3-8.html#type-only-imports-and-export)
in Kibana doesn't provide any benefits in terms of bundle size,
- Possible safeguarding against edge-case TS errors
- avoiding unintentional side effects
([source](https://typescript-eslint.io/blog/consistent-type-imports-and-exports-why-and-how/#avoiding-unintentional-side-effects))
- circular dependency reference error
[(source)](https://stackoverflow.com/questions/40982927/using-import-type-statement-to-fix-circular-dependency-reference-error)
- Better ground for incoming tooling (improving performance, reliability
with type definition analysis),
- Clearer code intent.
cqliu1 pushed a commit to cqliu1/kibana that referenced this issue Jan 2, 2025
…eam` owned plugins (elastic#204549)

Closes elastic#204412

## Summary

This PR enforces `consistent-type-imports` eslint rule in
`x-pack/solutions/observability/plugins/*` plugins owned by
`obs-ux-infra_services-team`.

Detailed list of plugins:
- `x-pack/solutions/observability/plugins/apm`,
- `x-pack/solutions/observability/plugins/apm_data_access`,
- `x-pack/solutions/observability/plugins/infra`,
- `x-pack/solutions/observability/plugins/inventory`,
- `x-pack/solutions/observability/plugins/metrics_data_access`,
- `x-pack/solutions/observability/plugins/profiling`,
- `x-pack/solutions/observability/plugins/profiling_data_access`

Script for fixing eslint rules in above plugins:
```bash
yarn eslint --no-error-on-unmatched-pattern --quiet --fix "x-pack/solutions/observability/plugins/apm/**/*.{js,mjs,ts,tsx}" "x-pack/solutions/observability/plugins/apm_data_access/**/*.{js,mjs,ts,tsx}" "x-pack/solutions/observability/plugins/infra/**/*.{js,mjs,ts,tsx}" "x-pack/solutions/observability/plugins/inventory/**/*.{js,mjs,ts,tsx}" "x-pack/solutions/observability/plugins/metrics_data_access/**/*.{js,mjs,ts,tsx}" "x-pack/solutions/observability/plugins/profiling/**/*.{js,mjs,ts,tsx}" "x-pack/solutions/observability/plugins/profiling_data_access/**/*.{js,mjs,ts,tsx}"
```

## Results

The affected plugins have been profiled using `node
scripts/build_kibana_platform_plugins.js --dist --profile --focus=apm
--no-cache` command and bundle size checked manually (`du -s` command).

#### APM plugin
Zero benefits in terms of size.
<img width="1728" alt="Screenshot 2024-12-19 at 12 18 36"
src="https://github.com/user-attachments/assets/d86be5d8-a4ad-4d9c-bac1-110a0c6bba81"
/>

#### Infra plugin
Zero benefits in terms of size.
<img width="1728" alt="Screenshot 2024-12-19 at 12 56 08"
src="https://github.com/user-attachments/assets/410bc068-1f20-4de8-ac4e-89c74478ec59"
/>

#### Profiling plugin
Zero benefits in terms of size.
<img width="1725" alt="image"
src="https://github.com/user-attachments/assets/bf47c319-0716-4a5b-9858-94ce7d2a3251"
/>

## Conclusions

- Using [type-only
imports](https://www.typescriptlang.org/docs/handbook/release-notes/typescript-3-8.html#type-only-imports-and-export)
in Kibana doesn't provide any benefits in terms of bundle size,
- Possible safeguarding against edge-case TS errors
- avoiding unintentional side effects
([source](https://typescript-eslint.io/blog/consistent-type-imports-and-exports-why-and-how/#avoiding-unintentional-side-effects))
- circular dependency reference error
[(source)](https://stackoverflow.com/questions/40982927/using-import-type-statement-to-fix-circular-dependency-reference-error)
- Better ground for incoming tooling (improving performance, reliability
with type definition analysis),
- Clearer code intent.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Spacetime Team:obs-ux-infra_services Observability Infrastructure & Services User Experience Team
Projects
None yet
2 participants