Skip to content

Conversation

@kyrie25
Copy link
Member

@kyrie25 kyrie25 commented Dec 6, 2025

bleh

Summary by CodeRabbit

  • Bug Fixes

    • Improved pattern matching logic for JSX route handling to ensure correct identification of configuration elements.
    • Enhanced parsing reliability by refining parenthesis detection to prevent premature termination during pattern analysis.
  • Refactor

    • Optimized code pattern processing approach for more efficient and accurate handling of lazy-loaded component mappings.

✏️ Tip: You can customize this high-level summary in your review settings.

@kyrie25 kyrie25 requested a review from rxri December 6, 2025 02:57
@rxri rxri merged commit e12717c into main Dec 6, 2025
6 of 7 checks passed
@rxri rxri deleted the fix/1.2.78-custom-app branch December 6, 2025 02:59
@coderabbitai
Copy link

coderabbitai bot commented Dec 6, 2025

Walkthrough

Changes to React pattern matching logic in apply.go and parsing logic in utils.go. The apply module now uses SeekToCloseParen for locating React.lazy patterns with targeted string replacement. The utils module adds a guard flag to SeekToCloseParen to ensure it processes at least one left parenthesis before terminating.

Changes

Cohort / File(s) Summary
React Pattern Matching
src/apply/apply.go
Replaces JSX route pattern for settings page; refactors React.lazy pattern handling to use SeekToCloseParen for locating full patterns, then injects app-specific lazy mapping via targeted string replacement instead of in-place replacement
Parenthesis Seeking Logic
src/utils/utils.go
Adds switch-based detection for left/right parentheses in SeekToCloseParen; introduces init flag to guard against immediate termination, ensuring the function processes at least one left parenthesis before closing

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Careful review needed for the new SeekToCloseParen logic and its guard condition—verify init flag correctly prevents premature termination
  • Confirm the React.lazy pattern matching correctly handles nested parentheses via the updated SeekToCloseParen behavior
  • Test edge cases where patterns have multiple or deeply nested parentheses

Poem

🐰 A rabbit hops through patterns fine,
Seeking parens, line by line,
With guards and flags to mark the way,
React's lazy route will play!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title references a specific version (1.2.78) and mentions custom apps, which aligns with changes to apply logic and React pattern handling for app-specific lazy mapping.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/1.2.78-custom-app

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/utils/utils.go (1)

348-372: SeekToCloseParen: add bounds check to avoid potential out-of-range panic

The new init guard is a nice improvement to avoid breaking before seeing a leftChar, but the loop still assumes there will always be a matching rightChar:

for {
    switch content[end] {
    case leftChar:
        count += 1
        init = true
    case rightChar:
        count -= 1
    }
    end += 1
    if count == 0 && init {
        break
    }
}

If a future Spotify build matches regexpTerm but the parentheses never balance in the remainder of content (or the pattern shifts), this will panic on content[end].

You can make this helper more robust with a small defensive change:

-       end := start
-       count := 0
-       init := false
-
-       for {
-           switch content[end] {
+       end := start
+       count := 0
+       init := false
+
+       for end < len(content) {
+           switch content[end] {
            case leftChar:
                count += 1
                init = true
            case rightChar:
                count -= 1
            }
-           end += 1
-           if count == 0 && init {
-               break
-           }
-       }
-       return content[start:end]
+           end++
+           if count == 0 && init {
+               return content[start:end]
+           }
+       }
+       // No matching close found; fail safely instead of panicking.
+       return ""

This keeps existing behavior when a balanced pair exists, but fails gracefully if it doesn’t.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 53b97e8 and 9854067.

📒 Files selected for processing (2)
  • src/apply/apply.go (2 hunks)
  • src/utils/utils.go (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/apply/apply.go (1)
src/utils/utils.go (2)
  • SeekToCloseParen (348-372)
  • Replace (167-173)
🔇 Additional comments (1)
src/apply/apply.go (1)

274-280: New /settings JSX route pattern: captures align with existing usage; verify against real xpui.js

The first elementPatterns entry now targets the /settings route:

// Settings page should be more consistent with having no conditional renders
`(\([\w$\.,]+\))\(([\w\.]+),\{path:"/settings(?:/[\w\*]+)?",?(element|children)?`,

This still yields three capture groups:

  • eleSymbs[0]: the JSX factory (e.g. (0,S.jsx)),
  • eleSymbs[1]: the route component/symbol (e.g. se.qh),
  • eleSymbs[2]: element / children / "",

which matches the later usage and wildcard handling at Lines 305–310. The older /collection pattern is still present as a fallback.

I’d just recommend double-checking on 1.2.78+ that:

  • there’s exactly one matching /settings route of this shape, and
  • any variants you care about (e.g. /settings/account) are covered by (?:/[\w\*]+)?.

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.

3 participants