Skip to content

Conversation

@weronikaolejniczak
Copy link
Contributor

@weronikaolejniczak weronikaolejniczak commented May 29, 2025

Summary

Closes #8708

On this PR, I:

  • renamed prefer-css-attribute-for-eui-components to prefer-css-prop-for-static-styles to highlight the case we're concerned with, eui-components is a redundancy when the rule comes from EUI ESLint plugin and css is technically a "prop" and not an "attribute" (it's not a regular HTML attribute),
  • changed the bang (something!) to actually assert if the range is truthy before passing it to fixer.replaceTextRange,
  • changed the rule message from "Prefer the css attribute for EUI components" to "Use the css prop instead of style for static styles in EUI components to ensure better performance and consistency with EUI’s styling approach.". I'm open to any suggestions that could make it better 🙏🏻

I consider this a breaking change because prefer-css-attribute-for-eui-components is technically no longer available.

QA

Testing instructions

  • The unit tests pass
  • There are no typos in the new message
  • The rule works when tested locally in Kibana
Screenshot 2025-05-29 at 13 56 58

@weronikaolejniczak weronikaolejniczak requested a review from a team as a code owner May 29, 2025 11:58
@weronikaolejniczak weronikaolejniczak self-assigned this May 29, 2025
@weronikaolejniczak weronikaolejniczak force-pushed the 8708-eslint-prefer-css-attributes-rule-is-warning-against-best-practice branch from ed01d41 to dc00cef Compare May 29, 2025 12:02
@weronikaolejniczak weronikaolejniczak requested a review from acstll May 30, 2025 18:04
Copy link
Contributor

@acstll acstll left a comment

Choose a reason for hiding this comment

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

I ran tests locally and went thru them. Also did manually test the package locally. And found no typos in the warning message. Everything working as expected 🟢

Only one comment regarding the changelog…

@weronikaolejniczak weronikaolejniczak requested a review from acstll June 2, 2025 08:58
Copy link
Contributor

@acstll acstll left a comment

Choose a reason for hiding this comment

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

Thanks for the quick update! 🌊🚢

@weronikaolejniczak weronikaolejniczak enabled auto-merge (squash) June 2, 2025 10:07
@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

History

cc @weronikaolejniczak

@weronikaolejniczak weronikaolejniczak merged commit 4dd9cca into elastic:main Jun 2, 2025
5 checks passed
@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

History

cc @weronikaolejniczak

weronikaolejniczak added a commit to elastic/kibana that referenced this pull request Jun 3, 2025
`@elastic/eslint-plugin-eui`: `0.2.0` ⏩ `1.0.0`

[Questions? Please see our Kibana upgrade
FAQ.](https://github.com/elastic/eui/blob/main/wiki/eui-team-processes/upgrading-kibana.md#faq-for-kibana-teams)

## Changes

This PR updates the `@elastic/eslint-plugin-eui` version to latest:
[v1.0.0](https://www.npmjs.com/package/@elastic/eslint-plugin-eui/v/1.0.0).

## Package updates

### `@elastic/eslint-plugin-eui`

- Changed the `prefer-css-prop-for-static-styles` rule message (formerly
`prefer-css-attribute-for-eui-components`)
([#8722](elastic/eui#8722))

**Breaking changes**

- Renamed the rule from `prefer-css-attribute-for-eui-components` to
`prefer-css-prop-for-static-styles` to align with Emotion's best
practice guidelines ([#8722](elastic/eui#8722))

**Dependency updates**

- Updated `typescript` to v5.8.3
([#8669](elastic/eui#8669))
- Updated `@typescript-eslint/eslint-plugin` to v8.31.1
([#8669](elastic/eui#8669))
- Updated `@typescript-eslint/parser` to v8.31.1
([#8669](elastic/eui#8669))
- Updated `@typescript-eslint/rule-tester` to v8.31.1
([#8669](elastic/eui#8669))
- Updated `@typescript-eslint/typescript-estree` to v8.31.1
([#8669](elastic/eui#8669))
- Updated `@typescript-eslint/utils` to v8.31.1
([#8669](elastic/eui#8669))
zacharyparikh pushed a commit to zacharyparikh/kibana that referenced this pull request Jun 4, 2025
`@elastic/eslint-plugin-eui`: `0.2.0` ⏩ `1.0.0`

[Questions? Please see our Kibana upgrade
FAQ.](https://github.com/elastic/eui/blob/main/wiki/eui-team-processes/upgrading-kibana.md#faq-for-kibana-teams)

## Changes

This PR updates the `@elastic/eslint-plugin-eui` version to latest:
[v1.0.0](https://www.npmjs.com/package/@elastic/eslint-plugin-eui/v/1.0.0).

## Package updates

### `@elastic/eslint-plugin-eui`

- Changed the `prefer-css-prop-for-static-styles` rule message (formerly
`prefer-css-attribute-for-eui-components`)
([elastic#8722](elastic/eui#8722))

**Breaking changes**

- Renamed the rule from `prefer-css-attribute-for-eui-components` to
`prefer-css-prop-for-static-styles` to align with Emotion's best
practice guidelines ([elastic#8722](elastic/eui#8722))

**Dependency updates**

- Updated `typescript` to v5.8.3
([elastic#8669](elastic/eui#8669))
- Updated `@typescript-eslint/eslint-plugin` to v8.31.1
([elastic#8669](elastic/eui#8669))
- Updated `@typescript-eslint/parser` to v8.31.1
([elastic#8669](elastic/eui#8669))
- Updated `@typescript-eslint/rule-tester` to v8.31.1
([elastic#8669](elastic/eui#8669))
- Updated `@typescript-eslint/typescript-estree` to v8.31.1
([elastic#8669](elastic/eui#8669))
- Updated `@typescript-eslint/utils` to v8.31.1
([elastic#8669](elastic/eui#8669))
nickpeihl pushed a commit to nickpeihl/kibana that referenced this pull request Jun 12, 2025
`@elastic/eslint-plugin-eui`: `0.2.0` ⏩ `1.0.0`

[Questions? Please see our Kibana upgrade
FAQ.](https://github.com/elastic/eui/blob/main/wiki/eui-team-processes/upgrading-kibana.md#faq-for-kibana-teams)

## Changes

This PR updates the `@elastic/eslint-plugin-eui` version to latest:
[v1.0.0](https://www.npmjs.com/package/@elastic/eslint-plugin-eui/v/1.0.0).

## Package updates

### `@elastic/eslint-plugin-eui`

- Changed the `prefer-css-prop-for-static-styles` rule message (formerly
`prefer-css-attribute-for-eui-components`)
([elastic#8722](elastic/eui#8722))

**Breaking changes**

- Renamed the rule from `prefer-css-attribute-for-eui-components` to
`prefer-css-prop-for-static-styles` to align with Emotion's best
practice guidelines ([elastic#8722](elastic/eui#8722))

**Dependency updates**

- Updated `typescript` to v5.8.3
([elastic#8669](elastic/eui#8669))
- Updated `@typescript-eslint/eslint-plugin` to v8.31.1
([elastic#8669](elastic/eui#8669))
- Updated `@typescript-eslint/parser` to v8.31.1
([elastic#8669](elastic/eui#8669))
- Updated `@typescript-eslint/rule-tester` to v8.31.1
([elastic#8669](elastic/eui#8669))
- Updated `@typescript-eslint/typescript-estree` to v8.31.1
([elastic#8669](elastic/eui#8669))
- Updated `@typescript-eslint/utils` to v8.31.1
([elastic#8669](elastic/eui#8669))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[ESLint] "Prefer CSS attributes" rule is warning against best practice

3 participants