Skip to content

Conversation

@weronikaolejniczak
Copy link
Contributor

@weronikaolejniczak weronikaolejniczak commented Feb 3, 2025

Summary

Closes #187

This PR is part of an On Week initiative to improve the state of the EUI ESLint plugin and enrich with useful rules.

The changes done:

  • migrated the package to ESM and TypeScript
  • added no-restricted-eui-imports rule to display a warning on JSON token imports
  • moved no-css-color and prefer-css-attribute-for-eui-components from kbn-eslint-plugin-css
    • no-css-color rule: highlights hard-coded colors
    • prefer-css-attribute-for-eui-components rule: recommends css use over style (inline styles)

Potential future changes:

  • migrate to Vitest (RuleTester) for out-of-the-box ESM and TS support, and superior performance to Jest
  • create a CI job to run the tests for when the package has been changed
  • implement design tokens specific rules

no-restricted-eui-imports

I added a new rule to our ESLint plugin, no-restricted-eui-imports, that won't conflict with the inbuilt no-restricted-imports rule and will allow us to define several error levels at once. I.e. we want some imports to be marked as warning and not an error, as it's done in Kibana.

Screenshot 2025-01-07 at 11 48 02 Screenshot 2025-01-07 at 11 50 41

Context:

The JSON token imports in @kbn/eslint/module_migration need to be removed as well:

https://github.com/elastic/kibana/blob/212b1926743ca5821992c2877d9c68f621e1875e/packages/kbn-eslint-config/.eslintrc.js#L131-L140

no-css-color and prefer-css-attribute-for-eui-components from kbn-eslint-plugin-css

Written by @eokoneyo. We agreed to move the rules to EUI repo, as the rules concern EUI components and may benefit other EUI consumers like Cloud UI.

Example files in Kibana:

  • src/platform/plugins/shared/kibana_react/public/page_template/no_data_page/no_data_card/elastic_agent_card.tsx

Screenshot 2025-02-28 at 17 00 38

  • x-pack/examples/alerting_example/public/alert_types/always_firing.tsx

Screenshot 2025-02-28 at 17 01 21

With additional configuration:

  • src/platform/plugins/private/vis_types/vega/public/data_model/utils.ts

Screenshot 2025-02-28 at 17 01 55

Useful resources

QA

  1. Follow the steps on this PR to test in Kibana.
  2. Run the tests: yarn workspace @elastic/eslint-plugin-eui test.

@weronikaolejniczak weronikaolejniczak added the skip-changelog Use on PRs to skip changelog requirement (Don't delete - used for automation) label Feb 3, 2025
@weronikaolejniczak weronikaolejniczak changed the title Chore/187 eslint plugin eui [#187] [On Week] Improve and enrich EUI ESLint plugin Feb 3, 2025
@weronikaolejniczak weronikaolejniczak added skip-changelog Use on PRs to skip changelog requirement (Don't delete - used for automation) and removed skip-changelog Use on PRs to skip changelog requirement (Don't delete - used for automation) labels Feb 3, 2025
@weronikaolejniczak weronikaolejniczak force-pushed the chore/187-eslint-plugin-eui branch from d69e9c5 to 7fc8386 Compare February 3, 2025 13:53
@weronikaolejniczak weronikaolejniczak force-pushed the chore/187-eslint-plugin-eui branch from 5248aa8 to 93831a6 Compare February 7, 2025 11:44
@weronikaolejniczak weronikaolejniczak self-assigned this Feb 7, 2025
@weronikaolejniczak weronikaolejniczak linked an issue Feb 25, 2025 that may be closed by this pull request
@weronikaolejniczak weronikaolejniczak force-pushed the chore/187-eslint-plugin-eui branch from b8f4827 to 7a40fae Compare February 26, 2025 13:28
@weronikaolejniczak weronikaolejniczak marked this pull request as ready for review March 3, 2025 12:17
@weronikaolejniczak weronikaolejniczak requested a review from a team as a code owner March 3, 2025 12:17
@weronikaolejniczak weronikaolejniczak removed their assignment Mar 7, 2025
@weronikaolejniczak weronikaolejniczak self-assigned this Mar 18, 2025
Copy link
Member

@tkajtoch tkajtoch left a comment

Choose a reason for hiding this comment

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

The changes look good to me now. Thank you for addressing my concerns!

@weronikaolejniczak weronikaolejniczak force-pushed the chore/187-eslint-plugin-eui branch from 733d059 to 81d88f7 Compare March 21, 2025 13:29
@kibanamachine
Copy link

Preview staging links for this PR:

@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

History

cc @weronikaolejniczak

@weronikaolejniczak weronikaolejniczak merged commit ff11e73 into elastic:main Mar 24, 2025
5 checks passed
weronikaolejniczak added a commit to elastic/kibana that referenced this pull request Apr 2, 2025
## Summary

Bring in the changes from elastic/eui#8304,
specifically ESLint rules:

- `no-restricted-eui-imports`
- `no-css-color` (migrated from `@kbn/eslint-plugin-css`)
- `prefer-css-attribute-for-eui-components` (migrated from
`@kbn/eslint-plugin-css`)

Relates to elastic/eui#8201,
elastic/eui-private#275

## QA

### Instructions

1. Checkout this branch: `gh pr checkout 210082`.
2. Reinstall dependencies: `yarn kbn bootstrap`.
3. See output of ESLint. There should be no errors.
4. Test below cases.

### Test cases

#### `no-restricted-eui-imports`

Example files:

- JSON imports: `src/platform/packages/shared/kbn-ui-theme/src/theme.ts`
- `@kbn/ui-theme`:
`src/platform/plugins/private/vis_types/vega/public/data_model/utils.ts`

#### `no-css-color`

Example file:
`src/platform/plugins/shared/kibana_react/public/page_template/no_data_page/no_data_card/elastic_agent_card.tsx:50`

![Screenshot 2025-02-26 at 15 01
53](https://github.com/user-attachments/assets/ec6f49bd-5832-4d1c-9cfd-74c40ad5498e)

#### `prefer-css-attribute-for-eui-components`

Example file:
`x-pack/examples/alerting_example/public/alert_types/always_firing.tsx:166`
@weronikaolejniczak weronikaolejniczak deleted the chore/187-eslint-plugin-eui branch April 16, 2025 06:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip-changelog Use on PRs to skip changelog requirement (Don't delete - used for automation)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Deprecate euiThemeVars

4 participants