fix: improve handling of pseudo classes and elements #58
Conversation
WalkthroughThe pull request modifies the Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
CodSpeed Performance ReportMerging #58 will degrade performances by 46.79%Comparing Summary
Benchmarks breakdown
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
packages/beasties/test/beasties.test.ts (1)
254-300: Consider enhancing test coverageThe test case effectively validates the handling of pseudo-classes and elements. However, consider adding these scenarios:
- Pseudo-elements with parameters (e.g.,
::nth-child(2n))- Multiple chained pseudo-classes
- Edge cases with escaped colons in selectors
packages/beasties/src/index.ts (1)
29-29: Add documentation for the regex patternConsider adding a comment explaining the pattern components:
- Negative lookbehind for escaped colons
- Single/double colon matching
- Parameter capturing
- Case insensitivity
+// Matches both pseudo-classes (e.g., :hover) and pseudo-elements (e.g., ::before) +// (?<!\\) - negative lookbehind for escaped colons +// ::? - matches both single and double colons +// (?:\(.+\))? - optionally matches parameters within parentheses const removePseudoClassesAndElementsPattern = /(?<!\\)::?[a-z-]+(?:\(.+\))?/gi
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
packages/beasties/test/__snapshots__/beasties.test.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (2)
packages/beasties/src/index.ts(2 hunks)packages/beasties/test/beasties.test.ts(2 hunks)
🔇 Additional comments (2)
packages/beasties/test/beasties.test.ts (1)
255-260: LGTM!
The logger mock implementation is concise and appropriate for the test requirements.
packages/beasties/src/index.ts (1)
680-682: LGTM!
The implementation effectively handles both pseudo selectors and trailing commas whilst maintaining the existing caching mechanism.
| @@ -26,8 +26,7 @@ import { applyMarkedSelectors, markOnly, parseStylesheet, serializeStylesheet, v | |||
| import { createDocument, serializeDocument } from './dom' | |||
| import { createLogger, isSubpath } from './util' | |||
|
|
|||
| const removePseudoElementsPattern = /(?<!\\)::?[a-z-]+(?![a-z-(])/gi | |||
| const cleanEmptyNotPattern = /::?not\(\s*\)/g | |||
There was a problem hiding this comment.
do we not need to handle an empty ::not()?
There was a problem hiding this comment.
From my understanding that is not valid
https://developer.mozilla.org/en-US/docs/Web/CSS/:not
There was a problem hiding this comment.
no... curious we were handling it before - maybe it might end up empty based on stripping of unused selectors?
There was a problem hiding this comment.
Stripping happens at a later stage, and I do not see any failing tests 🤷♂️
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #58 +/- ##
==========================================
- Coverage 84.89% 83.95% -0.94%
==========================================
Files 7 7
Lines 1205 1203 -2
Branches 282 281 -1
==========================================
- Hits 1023 1010 -13
- Misses 182 193 +11 ☔ View full report in Codecov by Sentry. |
Closes #52
✨
Description by Callstackai
This PR improves the handling of pseudo classes and elements in the Beasties library by updating the regex patterns used for normalization.
Diagrams of code changes
sequenceDiagram participant Test participant Beasties participant CSS Parser Test->>Beasties: process(HTML with stylesheet) Note over Beasties: New pattern added for<br/>pseudo-classes/elements Beasties->>CSS Parser: Parse CSS from style.css CSS Parser-->>Beasties: Return CSS rules Note over Beasties: Remove pseudo-classes/elements<br/>from selectors during normalization Beasties->>Beasties: normalizedSelector()<br/>Remove :pseudo-classes<br/>Remove ::pseudo-elements Beasties-->>Test: Return processed HTML with:<br/>- Original pseudo selectors preserved<br/>- Inlined styles in headFiles Changed
This PR includes files in programming languages that we currently do not support. We have not reviewed files with the extensions
.ts,.snap. See list of supported languages.Summary by CodeRabbit
New Features
Tests