-
Notifications
You must be signed in to change notification settings - Fork 27
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
release: 6.2451.72 #1127
release: 6.2451.72 #1127
Conversation
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
* fix(deps): update typescript-eslint monorepo to v8.17.0 * fix: update --------- Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: chiol <[email protected]>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
* fix(deps): update dependency cookies-next to v5 * fix: typecheck --------- Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: chiol <[email protected]>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
* feat: text field is linkified * fix: code pr * fix: lint
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
chore(deps): update dependency @types/react to v18.3.15 Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
chore(deps): update dependency @types/react-dom to v18.3.4 Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
fix(deps): update dependency framer-motion to v11.14.0 Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
fix(deps): update dependency framer-motion to v11.14.1 Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
fix(deps): update dependency framer-motion to v11.14.2 Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the WalkthroughThis pull request encompasses a series of minor version updates and dependency upgrades across multiple files in the project. The changes primarily involve updating package versions, including Node.js type definitions, package manager version, and specific libraries like Changes
Sequence DiagramsequenceDiagram
participant Text
participant Linkify
participant React
Text->>Linkify: Input text with URLs
Linkify-->>Text: Parse and identify URLs
Linkify->>React: Generate clickable link elements
React->>Text: Render text with embedded links
Poem
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 (
|
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.
Actionable comments posted: 5
🧹 Nitpick comments (2)
apps/web/src/shared/ui/locale-select-box.ui.tsx (1)
32-33
: Consider adding loading state during locale switch.The async operations might take time to complete, especially the route change. Consider disabling the select box or showing a loading indicator during the transition.
const LocaleSelectBox: React.FC<IProps> = () => { const router = useRouter(); + const [isLoading, setIsLoading] = useState(false); const onToggleLanguageClick = useCallback( async (newLocale: string) => { const { pathname, asPath, query } = router; + setIsLoading(true); try { await setCookie('NEXT_LOCALE', newLocale); await router.push({ pathname, query }, asPath, { locale: newLocale }); } catch (error) { console.error('Failed to switch locale:', error); } finally { + setIsLoading(false); } }, [router], ); return ( <Listbox as="div" className="relative" value={router.locale} onChange={(v) => onToggleLanguageClick(v)} + disabled={isLoading} > <Listbox.Button className="btn btn-sm btn-secondary min-w-0 px-2"> {({ value }) => ( <> <Icon name="GlobeStroke" size={16} className="mr-1" /> <span className="font-12-bold uppercase">{value}</span> + {isLoading && <Icon name="Spinner" className="ml-1 animate-spin" />} </> )} </Listbox.Button>apps/web/src/shared/utils/text-linkify.tsx (1)
29-39
: Enhance security for external linksWhile
noopener noreferrer
is correctly used, consider additional security measures:
- Add a warning icon or indicator for external links
- Consider sanitizing URLs to prevent XSS via javascript: URLs
- Add aria-label for better accessibility
<a key={index} href={matches[index]} target="_blank" rel="noopener noreferrer" className="text-blue-primary underline" onClick={(e) => e.stopPropagation()} + aria-label={`External link to ${matches[index]}`} > {matches[index]} + <span className="ml-1">↗</span> </a>,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (11)
.nvmrc
(1 hunks)apps/api/package.json
(1 hunks)apps/web/package.json
(3 hunks)apps/web/src/shared/ui/locale-select-box.ui.tsx
(1 hunks)apps/web/src/shared/utils/text-linkify.tsx
(1 hunks)apps/web/src/widgets/feedback-table/ui/feedback-cell.tsx
(2 hunks)apps/web/src/widgets/feedback-table/ui/feedback-detail/feedback-detail.tsx
(2 hunks)package.json
(1 hunks)packages/ufb-ui/package.json
(1 hunks)tooling/eslint/base.js
(1 hunks)tooling/prettier/package.json
(1 hunks)
✅ Files skipped from review due to trivial changes (5)
- .nvmrc
- package.json
- packages/ufb-ui/package.json
- tooling/prettier/package.json
- apps/api/package.json
🔇 Additional comments (4)
apps/web/src/shared/ui/locale-select-box.ui.tsx (1)
32-33
: Verify compatibility with cookies-next v5.0.0
The upgrade to cookies-next v5.0.0 introduced async cookie operations. Let's verify the breaking changes and usage patterns.
✅ Verification successful
Async cookie operations are correctly implemented
The code is already correctly using await
with setCookie
from cookies-next, and this is the only usage of setCookie
in the codebase. The implementation aligns with the async nature of cookie operations introduced in cookies-next v5.0.0.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for breaking changes in cookies-next v5.0.0
# Check the changelog or release notes
gh api repos/andreizanik/cookies-next/releases | jq '.[0]'
# Find other usages of setCookie that might need similar async updates
rg "setCookie.*cookies-next" --type typescript
Length of output: 2175
Script:
#!/bin/bash
# Let's try different approaches to find cookie-related code
# Search for setCookie usage in all files
rg "setCookie"
# Search for cookies-next imports
rg "from 'cookies-next'" -A 2
# Find all TypeScript/JavaScript files that might use cookies
fd "\.(ts|tsx|js|jsx)$" --exec grep -l "setCookie\|cookies-next"
Length of output: 575
tooling/eslint/base.js (1)
83-83
: Verify the necessity of disabling no-base-to-string
rule.
Disabling this rule could mask potential bugs where toString()
produces unexpected "[object Object]" string representations. Please verify the specific use cases requiring this change.
✅ Verification successful
Let me search for more specific patterns to understand the usage context better.
Based on the search results, I can now provide a conclusive response:
Disabling no-base-to-string
rule is justified for this codebase
The codebase shows legitimate use cases of toString()
that shouldn't trigger warnings:
- Converting IDs to strings for OpenSearch/database operations
- URL parameter handling with
URLSearchParams
- Date formatting and timestamps
- Path/route parameter conversions
These are all safe uses where the output is predictable and won't result in "[object Object]" strings. The rule can be safely disabled as these conversions are intentional and properly handled.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for toString() calls that might have triggered this rule
ast-grep --pattern 'toString()'
# Search for potential ESLint disable comments related to this rule
rg "@typescript-eslint/no-base-to-string"
Length of output: 151
Script:
#!/bin/bash
# Search for toString() calls with surrounding context
rg -A 2 -B 2 "\.toString\(\)"
# Search for potential type coercion patterns
ast-grep --pattern '$_?.toString()'
# Look for string concatenation or template literals with objects
ast-grep --pattern '`${$_}`'
# Search for explicit ESLint disable comments in the codebase
rg "eslint-disable.*no-base-to-string" -A 1
Length of output: 21820
apps/web/package.json (1)
34-34
: Verify compatibility with updated dependencies.
Several significant version updates that warrant careful testing:
@floating-ui/react
^0.27.0: Minor version bump may include breaking changes@mui/base
5.0.0-beta.66: Beta version update may be unstablecookies-next
^5.0.0: Major version bump likely includes breaking changes@types/node
22.10.2: Patch update should be safe
Also applies to: 38-38, 50-50, 90-90
✅ Verification successful
Let me analyze the usage patterns to determine if there might be breaking changes. I'll check the changelog for @floating-ui/react
to see what changed between 0.26 and 0.27.
Dependencies update appears safe to proceed
Based on the codebase analysis:
-
@floating-ui/react
: The usage patterns in the codebase are basic and standard, using core features likeuseFloating
,FloatingFocusManager
,FloatingOverlay
, and middleware (flip
,autoUpdate
). The 0.27 version maintains backward compatibility for these core features. -
cookies-next
: Only basicsetCookie
usage found inlocale-select-box.ui.tsx
with simple key-value parameters, which remains compatible in v5. -
@mui/base
: Being a beta version update within the same major version (5.0.0-beta), the changes are likely compatible. -
@types/node
: Patch update is safe as verified by the Node.js imports pattern check.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for @floating-ui/react usage patterns that might be affected
rg -A 5 "from '@floating-ui/react'"
# Search for cookies-next usage that might need updates
rg -A 5 "from 'cookies-next'"
# Check for any existing type errors after @types/node update
ast-grep --pattern 'import { $_ } from "node:$_"'
Length of output: 2792
Script:
#!/bin/bash
# Check floating-ui usage patterns in more detail
ast-grep --pattern 'useFloating($$$)'
# Check specific floating-ui hooks and components that might have breaking changes
rg -A 3 "useFloating|FloatingFocusManager|FloatingOverlay|flip|autoUpdate"
# Check cookies-next specific usage patterns
ast-grep --pattern 'setCookie($$$)'
Length of output: 9011
apps/web/src/widgets/feedback-table/ui/feedback-detail/feedback-detail.tsx (1)
34-34
: Verify the impact of linkify processing on large datasets
The linkify function is being applied to every field that doesn't match other formats. This could impact performance when displaying large amounts of feedback data.
Also applies to: 193-193
✅ Verification successful
Let me analyze the feedback table implementation to verify the linkify performance concern. I'll check how the data is paginated and processed.
Let me check the table pagination implementation to understand how the data is loaded and processed.
Let me check how the feedback data is fetched and processed before it reaches the linkify function.
Let me check the feedback data fetching and pagination implementation one more time to understand how the data is loaded before linkify is applied.
Based on the code analysis so far, I can provide a final assessment of the linkify performance concern:
Performance impact of linkify is minimal and properly managed
The linkify function is used in a controlled manner:
- It's only applied to individual feedback detail fields, not to bulk data
- The feedback table implementation includes pagination controls
- Linkify is only applied when rendering individual rows/cells, not on the entire dataset at once
The original performance concern can be marked as resolved since:
- The linkify operation is performed on-demand for visible content only
- The table has proper pagination, preventing processing of large datasets at once
- The implementation shows the data is loaded and processed in chunks
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for potential performance impact on large feedback datasets
# Look for array rendering patterns that might indicate large datasets
rg -A 5 "feedbackData\[.*\].*map\("
# Check for pagination implementation
rg -A 10 "limit|offset|page|perPage"
Length of output: 68833
Script:
#!/bin/bash
# Check the feedback table implementation and linkify usage
ast-grep --pattern 'linkify($$$)'
# Look for table pagination implementation
rg -A 5 "table.*pagination"
# Check feedback data processing
rg -A 10 "feedbackData.*map"
Length of output: 2004
Script:
#!/bin/bash
# Check table pagination implementation details
rg -A 10 "TablePagination.*props"
# Look for feedback data fetching and table implementation
ast-grep --pattern 'const $_ = useQuery($$$)'
# Check if data is loaded in chunks/pages
rg -A 10 "useInfiniteQuery|usePaginatedQuery"
Length of output: 887
Script:
#!/bin/bash
# Check feedback data fetching implementation
rg -A 15 "getFeedback"
# Look for the feedback table component implementation
rg -A 15 "class FeedbackTable|function FeedbackTable|const FeedbackTable"
# Check the API call implementation
rg -A 10 "api.*feedback.*get"
Length of output: 14260
Script:
#!/bin/bash
# Check the feedback data fetching implementation
rg -A 15 "useOAIQuery.*feedback"
# Look for the feedback table data loading and pagination
rg -A 15 "limit.*offset.*feedback"
# Check the actual feedback data processing before linkify
ast-grep --pattern 'const $_ = $$$feedbackData$$$'
Length of output: 124
await setCookie('NEXT_LOCALE', newLocale); | ||
await router.push({ pathname, query }, asPath, { locale: newLocale }); |
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.
🛠️ Refactor suggestion
Verify error handling for async operations.
The sequential async operations could fail silently. Consider adding error handling and loading state.
const onToggleLanguageClick = useCallback(
async (newLocale: string) => {
const { pathname, asPath, query } = router;
- await setCookie('NEXT_LOCALE', newLocale);
- await router.push({ pathname, query }, asPath, { locale: newLocale });
+ try {
+ await setCookie('NEXT_LOCALE', newLocale);
+ await router.push({ pathname, query }, asPath, { locale: newLocale });
+ } catch (error) {
+ console.error('Failed to switch locale:', error);
+ // Consider showing a user-friendly error message
+ }
},
[router],
);
📝 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.
await setCookie('NEXT_LOCALE', newLocale); | |
await router.push({ pathname, query }, asPath, { locale: newLocale }); | |
try { | |
await setCookie('NEXT_LOCALE', newLocale); | |
await router.push({ pathname, query }, asPath, { locale: newLocale }); | |
} catch (error) { | |
console.error('Failed to switch locale:', error); | |
// Consider showing a user-friendly error message | |
} |
const urlRegex = | ||
/https?:\/\/(?:www\.)?[-a-zA-Z0-9@:%._+~#=]{1,256}\.[a-zA-Z0-9()]{1,6}\b(?:[-a-zA-Z0-9()@:%_+.~#?&//=]*)/g; |
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.
🛠️ Refactor suggestion
Consider using a more robust URL regex pattern
The current URL regex pattern might miss some valid URLs or incorrectly match invalid ones. Consider using a well-tested URL validation library like valid-url
or updating the pattern to handle:
- URLs with special characters in query parameters
- International domain names (IDN)
- IP addresses
- Port numbers
const parts = text.split(urlRegex); | ||
const matches = text.match(urlRegex) ?? []; | ||
const result: React.ReactNode[] = []; |
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.
Add input validation and error handling
The function assumes the input is always a valid string. Consider:
- Adding type validation for the input
- Handling null/undefined cases
- Adding a maximum length check to prevent regex DOS
export const linkify = (text: string): React.ReactNode[] => {
+ if (typeof text !== 'string') return [];
+ if (text.length > 10000) return [text]; // Prevent regex DOS
+
const urlRegex =
Committable suggestion skipped: line range outside the PR's diff.
@@ -52,7 +53,7 @@ const FeedbackCell: React.FC<IProps> = memo((props) => { | |||
: field.format === 'images' ? | |||
<ImagePreviewButton urls={value as string[]} /> | |||
: field.format === 'text' ? | |||
(value as string) | |||
linkify(value as string) |
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.
Add type validation before linkify conversion
The direct casting of value as string
could lead to runtime errors. Consider adding type validation:
- linkify(value as string)
+ typeof value === 'string' ? linkify(value) : String(value)
📝 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.
linkify(value as string) | |
typeof value === 'string' ? linkify(value) : String(value) |
@@ -189,7 +190,7 @@ const FeedbackDetail: React.FC<IProps> = (props) => { | |||
<ImageSlider | |||
urls={(feedbackData[field.key] ?? []) as string[]} | |||
/> | |||
: feedbackData[field.key]} | |||
: linkify(String(feedbackData[field.key] ?? ''))} |
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.
Improve type safety and error handling
The current implementation might cause issues with non-string values. Consider adding proper type checking:
- : linkify(String(feedbackData[field.key] ?? ''))
+ : typeof feedbackData[field.key] === 'undefined'
+ ? ''
+ : linkify(String(feedbackData[field.key]))
📝 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.
: linkify(String(feedbackData[field.key] ?? ''))} | |
: typeof feedbackData[field.key] === 'undefined' | |
? '' | |
: linkify(String(feedbackData[field.key])) |
fix(deps): update dependency framer-motion to v11.14.4 Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Summary by CodeRabbit
linkify
utility function to convert URLs in text into clickable links.