Skip to content

[8.x] ES Lint rules for css-in-js declarations within Kibana (#200703)#204620

Merged
stephmilovic merged 1 commit intoelastic:8.xfrom
eokoneyo:backport/8.x/pr-200703
Dec 18, 2024
Merged

[8.x] ES Lint rules for css-in-js declarations within Kibana (#200703)#204620
stephmilovic merged 1 commit intoelastic:8.xfrom
eokoneyo:backport/8.x/pr-200703

Conversation

@eokoneyo
Copy link
Copy Markdown
Contributor

Backport

This will backport the following commits from main to 8.x:

Questions ?

Please refer to the Backport tool documentation

\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>\r\nCo-authored-by: Elastic Machine ","sha":"7370cc712ee2bf23f4cf53eef2e74942340f761c","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","backport missing","v9.0.0","Team:SharedUX","backport:prev-minor"],"number":200703,"url":"https://github.com//pull/200703","mergeCommit":{"message":"ES Lint rules for css-in-js declarations within Kibana (#200703)\n\n## Summary\r\n\r\nCloses https://github.com/elastic/kibana-team/issues/1272\r\n\r\nThis PR adds implementation for eslint rules to help facilitate the\r\nmigration away from SASS files to leveraging the design tokens EUI\r\nprovides for styling.\r\n\r\nThe introduced rules in this PR are as follows;\r\n\r\n- #### No CSS Color values\r\n Consider; \r\n\r\n ```tsx\r\n Hello World!\r\n ```\r\n\r\nthis expression because it specifies the css color property, with a\r\nvalid [CSS color\r\nvalue](https://developer.mozilla.org/en-US/docs/Web/CSS/color_value),\r\nwhen the aforementioned rule is enabled depending on the set report\r\nlevel set the user would get a feedback, see screenshot below;\r\n\r\n\r\n

\"Screenshot

\r\n\r\nThis rule also works for variables defined elsewhere in the code and\r\nreferenced as a value to the style prop, see screenshot below;\r\n\r\n

\"Screenshot

\r\n\r\nfeedback will also be provided when some variable that is a literal\r\nvalue is specified as a value for any earmarked property that should not\r\nspecify literal values.\r\n\r\n

\"Screenshot

\r\n\r\nfeedback will be provided for referencing a member prop of some object\r\ndefined elsewhere as a value to any earmarked property that we have\r\ndeemed to not specify literal values\r\n\r\n

\"Screenshot

\r\n\r\n\tSupports;\r\n\t- object values\r\n\t- object references\r\n\t- template literals\r\n\t- tagged templates \r\n\r\nThis approach does not penalize variable declarations, only the usages\r\nof any said variable when it doesn't conform to expectation\r\n\r\n- #### Prefer CSS attributes for EUI components (optional)\r\n Consider; \r\n\r\n ```tsx\r\nHello\r\nWorld!\r\n ```\r\nA declaration like the one above, will be regarded as an error and can\r\nbe fixed, when it's fixed it will be re-written as\r\n \r\n ```tsx\r\nHello\r\nWorld!\r\n ```\r\n\r\n\t\r\n\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>\r\nCo-authored-by: Elastic Machine ","sha":"7370cc712ee2bf23f4cf53eef2e74942340f761c"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","labelRegex":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com//pull/200703","number":200703,"mergeCommit":{"message":"ES Lint rules for css-in-js declarations within Kibana (#200703)\n\n## Summary\r\n\r\nCloses https://github.com/elastic/kibana-team/issues/1272\r\n\r\nThis PR adds implementation for eslint rules to help facilitate the\r\nmigration away from SASS files to leveraging the design tokens EUI\r\nprovides for styling.\r\n\r\nThe introduced rules in this PR are as follows;\r\n\r\n- #### No CSS Color values\r\n Consider; \r\n\r\n ```tsx\r\n Hello World!\r\n ```\r\n\r\nthis expression because it specifies the css color property, with a\r\nvalid [CSS color\r\nvalue](https://developer.mozilla.org/en-US/docs/Web/CSS/color_value),\r\nwhen the aforementioned rule is enabled depending on the set report\r\nlevel set the user would get a feedback, see screenshot below;\r\n\r\n\r\n

\"Screenshot

\r\n\r\nThis rule also works for variables defined elsewhere in the code and\r\nreferenced as a value to the style prop, see screenshot below;\r\n\r\n

\"Screenshot

\r\n\r\nfeedback will also be provided when some variable that is a literal\r\nvalue is specified as a value for any earmarked property that should not\r\nspecify literal values.\r\n\r\n

\"Screenshot

\r\n\r\nfeedback will be provided for referencing a member prop of some object\r\ndefined elsewhere as a value to any earmarked property that we have\r\ndeemed to not specify literal values\r\n\r\n

\"Screenshot

\r\n\r\n\tSupports;\r\n\t- object values\r\n\t- object references\r\n\t- template literals\r\n\t- tagged templates \r\n\r\nThis approach does not penalize variable declarations, only the usages\r\nof any said variable when it doesn't conform to expectation\r\n\r\n- #### Prefer CSS attributes for EUI components (optional)\r\n Consider; \r\n\r\n ```tsx\r\nHello\r\nWorld!\r\n ```\r\nA declaration like the one above, will be regarded as an error and can\r\nbe fixed, when it's fixed it will be re-written as\r\n \r\n ```tsx\r\nHello\r\nWorld!\r\n ```\r\n\r\n\t\r\n\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>\r\nCo-authored-by: Elastic Machine ","sha":"7370cc712ee2bf23f4cf53eef2e74942340f761c"}}]}] BACKPORT-->

## Summary

Closes elastic/kibana-team#1272

This PR adds implementation for eslint rules to help facilitate the
migration away from SASS files to leveraging the design tokens EUI
provides for styling.

The introduced rules  in this PR are as follows;

- #### No CSS Color values
  Consider;

  ```tsx
  <EuiCode style={{ color: '#dd4040' }}>Hello World!</EuiCode>
  ```

this expression because it specifies the css color property, with a
valid [CSS color
value](https://developer.mozilla.org/en-US/docs/Web/CSS/color_value),
when the aforementioned rule is enabled depending on the set report
level set the user would get a feedback, see screenshot below;

<img width="735" alt="Screenshot 2024-11-20 at 12 46 17"
src="https://github.com/user-attachments/assets/d2f608dc-782c-4d83-88e6-92dfdd8f6101">

This rule also works for variables defined elsewhere in the code and
referenced as a value to the style prop, see screenshot below;

<img width="1658" alt="Screenshot 2024-11-26 at 13 29 45"
src="https://github.com/user-attachments/assets/f8aadf6b-318b-4c6a-b7c9-bb44fb867b58">

feedback will also be provided when some variable that is a literal
value is specified as a value for any earmarked property that should not
specify literal values.

<img width="1730" alt="Screenshot 2024-11-28 at 19 00 08"
src="https://github.com/user-attachments/assets/bc3a8674-9469-4c7a-b0c9-7a2bfa7f08dc">

feedback will be provided for referencing a member prop of some object
defined elsewhere as a value to any earmarked property that we have
deemed to not specify literal values

<img width="1676" alt="Screenshot 2024-11-29 at 11 36 44"
src="https://github.com/user-attachments/assets/c4537fbf-b2d8-46bb-ad5f-8582e8c9a932">

	Supports;
	- object values
	- object references
	- template literals
	- tagged templates

This approach does not penalize variable declarations, only the usages
of any said variable when it doesn't conform to expectation

- #### Prefer CSS attributes for EUI components (optional)
  Consider;

  ```tsx
<EuiCode style={{ someCSSProperty: 'propertyValue' }}>Hello
World!</EuiCode>
  ```
A declaration like the one above, will be regarded as an error and can
be fixed, when it's fixed it will be re-written as

  ```tsx
<EuiCode css={{ someCSSProperty: 'propertyValue' }}>Hello
World!</EuiCode>
  ```

<!--

### Checklist

Check the PR satisfies following conditions.

Reviewers should verify this PR satisfies this list as well.

- [ ] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [ ]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
- [ ] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [ ] If a plugin configuration key changed, check if it needs to be
allowlisted in the cloud and added to the [docker
list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)
- [ ] This was checked for breaking HTTP API changes, and any breaking
changes have been approved by the breaking-change committee. The
`release_note:breaking` label should be applied in these situations.
- [ ] [Flaky Test
Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was
used on any tests changed
- [ ] The PR description includes the appropriate Release Notes section,
and the correct `release_node:*` label is applied per the
[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

### Identify risks

Does this PR introduce any risks? For example, consider risks like hard
to test bugs, performance regression, potential of data loss.

Describe the risk, its severity, and mitigation for each identified
risk. Invite stakeholders and evaluate how to proceed before merging.

- [ ] [See some risk
examples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx)
- [ ] ...

-->

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
(cherry picked from commit 7370cc7)
@eokoneyo eokoneyo added the backport This PR is a backport of another PR label Dec 17, 2024
@eokoneyo eokoneyo enabled auto-merge (squash) December 17, 2024 17:05
@kapral18
Copy link
Copy Markdown
Contributor

Please do not merge this pull request. We disabled auto-merge because we are trying to merge a big PR as part of sustainable architecture migration which is impossible with ever increasing stream of backports. We will resume the automerge after our PR is merged. Reach out to #sustainable-kibana-architecture for more info.

@kapral18 kapral18 disabled auto-merge December 17, 2024 17:26
@elasticmachine
Copy link
Copy Markdown
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

@kapral18
Copy link
Copy Markdown
Contributor

Auto-merge has been re-enabled. Thank you for your patience. :heart

@stephmilovic stephmilovic enabled auto-merge (squash) December 17, 2024 19:28
@stephmilovic stephmilovic merged commit f535325 into elastic:8.x Dec 18, 2024
@eokoneyo eokoneyo deleted the backport/8.x/pr-200703 branch December 18, 2024 09:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport This PR is a backport of another PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants