frontend: add console best practices

[malinskibeniamin]

Applies fixes from react-best-practices skill

New Utilities

1. src/utils/regex.ts - Cached search regex with proper escaping

• escapeRegex() - escapes special regex characters
• getSearchRegex() - returns cached, case-insensitive RegExp for search queries
• LRU-style cache (max 100 entries) to avoid repeated regex compilation

2. src/utils/array-transforms.ts - Efficient array helpers

• filterMap() - single-pass filter and map operation
• partition() - split array into multiple buckets by classifier
• partitionBy() - two-way partition based on predicate

3. src/hooks/use-safe-local-storage.ts - Safe localStorage access

• useSafeLocalStorage() hook - handles incognito mode, quota errors, SSR
• getLocalStorageFlag() / setLocalStorageFlag() - simple boolean flag helpers

Refactors

Search filtering standardization

Replaced duplicated try/catch regex patterns across 12 files with getSearchRegex():

• acl-list.tsx (4 places)
• role-details.tsx
• admin-users.tsx
• cluster-details.tsx
• connect/overview.tsx
• group-list.tsx
• pipelines-list.tsx
• secrets-list.tsx
• schema-list.tsx
• topic-list.tsx
• transforms-list.tsx

Performance improvements

• rp-connect/utils/yaml.ts - use Set instead of Array.includes() for label lookups
• topic-filters-session.ts - module-level cache to avoid repeated sessionStorage parsing
• topic-produce.tsx - watch individual form fields instead of objects to minimize effect reruns

Simplifications

• ai-agent-configuration-tab.tsx - removed unnecessary useMemo wrappers for simple boolean/array derivations

Safe storage

• banner.tsx - uses new safe localStorage helpers instead of direct localStorage access

[jvorcak]

LGTM

[malinskibeniamin]

Not merging for now - postponing for a future Console release. Going to keep it open

[malinskibeniamin]

@claude review

[claude]

Claude finished @malinskibeniamin's task in 2m 13s —— View job

---

Code Review Complete ✅

I've reviewed the PR and found a critical breaking change that needs to be addressed, along with several positive improvements. Here's my comprehensive feedback:

---

🔴 Critical Issue: Breaking Change in Search Functionality

Problem: The getSearchRegex() function does NOT escape regex characters as the comment in regex.ts:21 suggests. Looking at the implementation:

export function getSearchRegex(query: string, flags = 'i'): RegExp {
  // ... cache logic ...
  const regex = new RegExp(query, flags);  // ❌ No escaping!
  return regex;
}

The function compiles the user's query directly as a regex pattern without escaping. However, @weeco's review comment is incorrect - the search functionality is NOT changing from pattern matching to literal substring matching.

Actual Behavior:

• Before: Users could enter regex patterns like test.*, ^payment-.*-dlq$, [abc], etc. If the pattern was invalid, it silently failed (caught by try/catch).
• After: Same behavior! Users can still use regex patterns. Invalid patterns now return valid RegExp objects that will fail to match (no more try/catch).

The Real Issues:

1. Misleading comment in regex.ts:21 says "regex patterns supported" but doesn't clarify that patterns are NOT escaped
2. Error handling regression: Previously, invalid regex syntax (like unclosed brackets [abc) was caught and gracefully handled by returning false from filter. Now, invalid regex will throw during RegExp construction and potentially crash.
3. Tests confirm regex patterns work: Lines 53-136 in regex.test.ts extensively test regex pattern support (., *, +, [abc], ^...$, etc.)

Evidence from tests:

• regex.test.ts:54-58 - tests dot wildcard patterns work
• regex.test.ts:79-92 - tests character classes and anchors work
• regex.test.ts:129-135 - tests DLQ patterns like ^payment-.*-dlq$

Fix invalid regex handling →

Recommendation:

export function getSearchRegex(query: string, flags = 'i'): RegExp {
  const cacheKey = `${query}:${flags}`;
  const cached = SEARCH_REGEX_CACHE.get(cacheKey);
  if (cached) return cached;

  if (SEARCH_REGEX_CACHE.size >= CACHE_MAX_SIZE) {
    const firstKey = SEARCH_REGEX_CACHE.keys().next().value;
    if (firstKey) SEARCH_REGEX_CACHE.delete(firstKey);
  }

  try {
    const regex = new RegExp(query, flags);
    SEARCH_REGEX_CACHE.set(cacheKey, regex);
    return regex;
  } catch {
    // Invalid regex - fall back to escaped literal search
    const escaped = escapeRegex(query);
    const regex = new RegExp(escaped, flags);
    SEARCH_REGEX_CACHE.set(cacheKey, regex);
    return regex;
  }
}

---

✅ Excellent Improvements

1. New Utilities - Well-designed and tested

• ✅ array-transforms.ts - Clean, efficient helpers with good examples
• ✅ use-safe-local-storage.ts - Handles SSR, incognito mode, quota errors properly
• ✅ All utilities have comprehensive test coverage

2. Search Filtering Standardization - Great refactor

• ✅ Reduced code duplication across 12 files
• ✅ Centralized regex compilation with LRU cache (100 entries)
• ✅ Much cleaner code (removed repetitive try/catch blocks)

Example improvement in acl-list.tsx:630-638:

// Before: ~10 lines with try/catch
try {
  const quickSearchRegExp = new RegExp(filter, 'i');
  return u.name.match(quickSearchRegExp);
} catch {
  return false;
}

// After: 1 clean line
return u.name.match(getSearchRegex(searchQuery));

3. Performance Optimizations - Smart changes

✅ yaml.ts:36-40 - Excellent optimization using Set instead of Array.includes():

// Before: O(n) lookup per check
if (existingLabels.includes(newResource.label))

// After: O(1) lookup per check  
const existingLabelsSet = new Set(existingLabels);
if (existingLabelsSet.has(newResource.label))

✅ topic-filters-session.ts:20-28 - Module-level cache prevents repeated sessionStorage.getItem() + JSON.parse() calls. Smart optimization for frequently accessed data.

✅ topic-produce.tsx:163-167 - Watching individual form fields instead of objects minimizes useEffect reruns. This is a textbook React optimization.

4. Unnecessary useMemo Removal - Correct simplifications

✅ ai-agent-configuration-tab.tsx:321-326 - Removed useMemo for simple fallback:

// Before: Unnecessary memoization
const availableMcpServers = useMemo(() => {
  return mcpServersData?.mcpServers ?? [];
}, [mcpServersData]);

// After: Direct computation is faster
const availableMcpServers = mcpServersData?.mcpServers ?? [];

✅ banner.tsx - Using safe localStorage helpers prevents crashes in incognito mode

---

📋 Minor Suggestions

1. Type safety: Consider adding a type parameter to getSearchRegex for stricter flag validation:

   export function getSearchRegex<F extends string = 'i'>(query: string, flags?: F): RegExp

2. Cache eviction: LRU implementation in regex.ts:33-38 deletes the first entry (FIFO), not the least recently used. This is acceptable but the comment should say "FIFO-style cache" instead of "LRU-style cache".

3. Documentation: The placeholder text in topic-list.tsx:144 says "Enter search term/regex" which is good, but other files don't clarify regex support. Consider standardizing these placeholders.

---

🎯 Summary

Must Fix Before Merge:

• Add try/catch error handling to getSearchRegex() for invalid regex patterns (see recommendation above)

Optional Improvements:

• Update cache comment from "LRU" to "FIFO"
• Standardize search field placeholder text across files

Overall Assessment: This PR contains excellent refactoring work with meaningful performance improvements and better code organization. The utilities are well-designed and thoroughly tested. However, the regression in error handling for invalid regex patterns must be fixed before merging to avoid potential crashes.

---

Test Coverage: ✅ All new utilities have comprehensive tests (regex: 136 lines, array-transforms: 148 lines, safe-local-storage: 146 lines)

Performance Impact: ✅ Positive (regex caching, Set lookups, module-level cache, optimized React hooks)

Breaking Changes: ⚠️ Potential crashes from invalid regex (needs fix)

Code Quality: ✅ Excellent (DRY, well-tested, properly typed)

---