Skip to content

Conversation

@Anemy
Copy link
Member

@Anemy Anemy commented Sep 11, 2025

COMPASS-9843

Opened as warn and not error as there are a good amount of ones that error so I'll do it in a separate pr.
Included two small fixes for existing cases in this. There are 8 in compass-components, it looks like most are behind useMemo calls though.

How it looks with error:
Screenshot 2025-09-11 at 1 10 58 PM

Warn (current):
Screenshot 2025-09-11 at 1 27 20 PM

@Anemy Anemy requested a review from a team as a code owner September 11, 2025 17:38
@Anemy Anemy requested review from Copilot and lerouxb and removed request for lerouxb September 11, 2025 17:38
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds a new ESLint rule to prevent dynamic CSS-in-JS calls within functions for performance optimization. The rule is configured as a warning to allow gradual adoption, as there are existing violations in the codebase.

  • Adds no-inline-emotion-css ESLint rule to detect performance-problematic dynamic CSS calls
  • Fixes two existing violations by moving dynamic CSS generation outside functions
  • Includes comprehensive test coverage for the new rule

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
configs/eslint-plugin-compass/rules/no-inline-emotion-css.js Implements the new ESLint rule logic
configs/eslint-plugin-compass/rules/no-inline-emotion-css.test.js Test cases for the new rule
configs/eslint-plugin-compass/index.js Exports the new rule
configs/eslint-config-compass/index.js Configures the rule as a warning
packages/compass-aggregations/src/components/search-no-results.tsx Fixes dynamic CSS usage
packages/compass-aggregations/src/components/aggregation-side-panel/stage-wizard-use-cases/sort/sort.tsx Replaces dynamic CSS with static styles
configs/eslint-plugin-compass/rules/unique-mongodb-log-id.test.js Corrects test name

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

return (
<SortFormGroup
comboboxClassName={comboboxClassName}
comboboxClassName={comboboxStyles}
Copy link
Member Author

Choose a reason for hiding this comment

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

before after
Screenshot 2025-09-11 at 1 35 03 PM Screenshot 2025-09-11 at 1 37 07 PM

Comment on lines 26 to 31
return ancestors.some(
(ancestor) =>
ancestor.type === 'FunctionDeclaration' ||
ancestor.type === 'FunctionExpression' ||
ancestor.type === 'ArrowFunctionExpression'
);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe just checking for css calls found inside JSXExpressionContainer is enough for initial iteraion, I think it's the main offender case, but I might be wrong. Otherwise it can be indeed tricky to target only functions that are React components, react hook rules for example try to identify something as a component just by function name starting with an uppercase letter, not the most precise logic 🙂

Copy link
Member Author

Choose a reason for hiding this comment

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

Sounds good, I added both!

Copy link
Collaborator

@gribnoysup gribnoysup left a comment

Choose a reason for hiding this comment

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

Very neat!

};

ruleTester.run('no-leafygreen-outside-compass-components', rule, {
ruleTester.run('unique-mongodb-log-id', rule, {
Copy link
Collaborator

Choose a reason for hiding this comment

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

ty!

@Anemy Anemy merged commit c6da42a into main Sep 12, 2025
56 of 58 checks passed
@Anemy Anemy deleted the add-no-inline-emotion-css-lint-rule branch September 12, 2025 18:02
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.

2 participants