fix(convertUtilitiesToV4): drop js-regexp-lookbehind for improved browser support
#1555
Conversation
…or of more simpler regex for improved browser support
SutuSebastian
added
🐛 bug
🦋 Changeset detectedLatest commit: 27bcf1f The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis pull request introduces updates in several parts of the project. A new changeset entry documents a patch for the "flowbite-react" package by removing the Changes
Sequence Diagram(s)sequenceDiagram
participant Caller as Caller
participant Converter as convertUtilitiesToV4
participant RegexMapper as Regex Matching
Caller->>Converter: Pass input string (class names)
Converter->>Converter: Split input by whitespace
loop Process each part
Converter->>Converter: Check if part is non-whitespace
Converter->>RegexMapper: Apply regex matching for modifiers & base classes
RegexMapper-->>Converter: Return match (if any)
Converter->>Converter: Replace text based on regexMap
end
Converter-->>Caller: Return the converted class string
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
packages/ui/src/helpers/convert-utilities-to-v4.tsOops! Something went wrong! :( ESLint: 8.57.0 ESLint couldn't find the plugin "eslint-plugin-react". (The package "eslint-plugin-react" was not found when loaded as a Node module from the directory "/packages/ui".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-react" was referenced from the config file in "packages/ui/.eslintrc.cjs". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. ✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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 (
|
SutuSebastian
changed the title
SutuSebastian
changed the title
SutuSebastian
changed the title
js-regexp-lookbehind for improved browser support
SutuSebastian
deleted the
1554-safari-until-164-syntaxerror-invalid-regular-expression-invalid-group-specifier-name
branch
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/ui/src/helpers/convert-utilities-to-v4.ts (2)
28-29: Minor redundancy in variable naming.
const processed = part;doesn’t add additional context overpart. You can usepartdirectly in the subsequent call tomatch(...)for improved clarity.- const processed = part; - const modifierMatch = processed.match(/^([^:]+:)?(.+)$/); + const modifierMatch = part.match(/^([^:]+:)?(.+)$/);
65-75: Confirm updated mappings and revisit the commented-out outline rule.
The revised regex patterns look consistent for converting v3 to v4 utility classes. However, the commented line foroutline-noneremains unresolved. Ensure that future updates address keyboard accessibility and tab focus properly before reintroduction.Would you like help drafting a solution that maintains keyboard accessibility while adjusting outline behavior?
.changeset/open-toys-sniff.md (1)
5-5: Fix comparative form for proper grammar.
Use "simpler" or "more simple," not "more simpler."- refactor(ui~convertUtilitiesToV4): drop `js-regexp-lookbehind` in favor of more simpler regex for improved browser support + refactor(ui~convertUtilitiesToV4): drop `js-regexp-lookbehind` in favor of simpler regex for improved browser support🧰 Tools
🪛 LanguageTool
[grammar] ~5-~5: Use only “simpler” (without ‘more’) when you use the comparative.
Context: ...dropjs-regexp-lookbehindin favor of more simpler regex for improved browser support(MOST_COMPARATIVE)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.changeset/open-toys-sniff.md(1 hunks)apps/web/content/docs/getting-started/ai-integration.mdx(0 hunks)packages/ui/src/helpers/convert-utilities-to-v4.ts(2 hunks)
💤 Files with no reviewable changes (1)
- apps/web/content/docs/getting-started/ai-integration.mdx
🧰 Additional context used
🪛 LanguageTool
.changeset/open-toys-sniff.md
[grammar] ~5-~5: Use only “simpler” (without ‘more’) when you use the comparative.
Context: ...drop js-regexp-lookbehind in favor of more simpler regex for improved browser support
(MOST_COMPARATIVE)
🔇 Additional comments (1)
packages/ui/src/helpers/convert-utilities-to-v4.ts (1)
21-22: Use of whitespace capture to preserve spacing looks good.
Splitting the class string into an array while capturing the whitespace ensures that any spacing or separation between class names is accurately preserved, which avoids unintended concatenation of classes.
| const modifierMatch = processed.match(/^([^:]+:)?(.+)$/); | ||
|
|
||
| if (modifierMatch) { |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Potential issue with multiple colons in modifiers.
The regex ^([^:]+:)?(.+)$ captures only one optional colon-delimited prefix. This may fail to handle scenarios like hover:focus:bg-red-500. If supporting multiple modifiers is intended, consider updating the capture group to handle multiple colons.
🏁 Script executed:
#!/bin/bash
# Description: Search codebase for possible usage of multiple-colon class names
# to confirm if multiple modifiers are used.
rg -A 3 "hover:.*:.*" Length of output: 72460
Attention: Update Regex to Support Compound Modifiers
The current regex in packages/ui/src/helpers/convert-utilities-to-v4.ts (lines 29–31):
- const modifierMatch = processed.match(/^([^:]+:)?(.+)$/);
+ const modifierMatch = processed.match(/^((?:[^:]+:)+)?(.+)$/);only allows for one optional modifier prefix. However, our codebase and tests demonstrate compound modifier patterns (e.g. hover:focus:bg-red-500, dark:hover:!text-lg). This means that when multiple modifiers are used, only the first (e.g. "hover:") is captured, leaving the rest in the second group—which can lead to incorrect processing or conversion.
Recommendation:
- Update the regex as shown above so that the optional group can match one or more colon-delimited prefixes.
- Verify that the rest of the conversion logic correctly handles the split between modifiers and the utility.
Please address this to ensure consistency when multiple modifiers are present.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const modifierMatch = processed.match(/^([^:]+:)?(.+)$/); | |
| if (modifierMatch) { | |
| const modifierMatch = processed.match(/^((?:[^:]+:)+)?(.+)$/); | |
| if (modifierMatch) { |
Issue
#1554
Summary by CodeRabbit
Chores
Documentation
Refactor