-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Add lint rule for child :global selectors #17908
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 issue found across 6 files
Confidence score: 4/5
- ESLint location reporting may be off because
eslint-local-rules/index.jscompares against the literal "\n" instead of a newline, which affectsbuildLineOffsetsaccuracy - Severity is moderate and limited to developer tooling rather than runtime behavior, so risk is low overall
- Pay close attention to
eslint-local-rules/index.js- fix newline comparison to ensure correct offset calculations.
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="eslint-local-rules/index.js">
<violation number="1" location="eslint-local-rules/index.js:27">
P2: The newline check in buildLineOffsets compares against a literal "\\n", so offsets never include actual line breaks and reported ESLint locations will be wrong. Compare to `"\n"` instead.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this have any performance overhead?
This is some quite complex code - in terms of what it is and how it has to do it over everything?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
almost definitely
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll do a performance test now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not as bad as I thought it would be actually:
- Before (rules off): 40.43s
- After (rules on): 40.48s
- Delta: +0.05s (~0.1%), effectively within run‑to‑run noise
Going to run some additional tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm I think this run is probably more realistic:
- Before (rules off): 22.66s real
- After (rules on): 23.54s real
- Delta: +0.88s (~3.9% of ESLint time)
Description
Adds a local ESLint rule that flags comma-separated selectors containing multiple child combinator :global(...) entries, preventing the CSS minification issue fixed in bb648c7.
Screenshots
Example errors
Launchcontrol
Prevents unsafe child :global selector lists by linting for multiple child combinator :global entries in a single rule.