fix: Set is Frame when ending with slash #1129
Conversation
Added handling to clean url Updated to use util function
WalkthroughThe changes introduced in this pull request involve the addition of a new utility function Changes
Possibly related PRs
Suggested labels
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 (
|
Performance Comparison ReportSignificant Changes To DurationThere are no entries Meaningless Changes To DurationShow entries
Show details
Render Count ChangesThere are no entries Render IssuesThere are no entries Added ScenariosThere are no entries Removed ScenariosThere are no entries |
![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
🧹 Outside diff range and nitpick comments (2)
utils/frames.ts (1)
274-285: LGTM! Consider adding input validation.
The implementation correctly handles URL normalization and follows good separation of concerns. However, consider adding input validation for the parameters.
export const isFrameMessage = (
messageIsText: boolean,
messageContent: string,
framesStore: {
[frameUrl: string]: FrameWithType;
}
): boolean => {
+ if (messageContent == null || framesStore == null) return false;
if (!messageIsText) return false;
const content = messageContent.toLowerCase().trim();
const sanitizedContent = content.endsWith("/")
? content.slice(0, -1)
: content;
return !!framesStore[sanitizedContent];
};components/Chat/Message/Message.tsx (1)
Line range hint 391-397: Consider extracting URL handling logic
The URL handling logic in the handleUrlPress callback could be moved to a utility function to ensure consistent URL cleaning across the application.
-const handleUrlPress = useCallback((url: string) => {
- const cleanedUrl = url.toLowerCase().trim();
-
- const uri = cleanedUrl.startsWith("http")
- ? cleanedUrl
- : `https://${cleanedUrl}`;
-
- Linking.openURL(uri);
-}, []);
+import { cleanAndFormatUrl } from '@utils/url';
+
+const handleUrlPress = useCallback((url: string) => {
+ Linking.openURL(cleanAndFormatUrl(url));
+}, []);📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- components/Chat/Message/Message.tsx (2 hunks)
- utils/frames.ts (1 hunks)
🔇 Additional comments (3)
utils/frames.ts (1)
274-285: Verify URL handling consistency across the codebase.
Let's ensure that frame URLs are handled consistently throughout the codebase, particularly in the fetchFramesForMessage function where frames are stored.
✅ Verification successful
Let me gather more context about the frame URL handling by checking the implementation of related functions.
Based on the evidence gathered, I can now provide the final response:
URL handling is consistent across frame-related operations
The codebase demonstrates consistent URL handling patterns:
- Frame URLs are consistently stored in lowercase:
framesToSave[response.url.toLowerCase()] - The same normalization is applied when storing with trailing slash:
framesToSave[${response.url.toLowerCase()}/] - The lookup in
framesStorematches this pattern by normalizing the input URL similarly - No direct URL comparisons are found that could bypass this normalization
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for potential inconsistencies in URL handling
# Test 1: Check if there are any direct frame URL comparisons without normalization
echo "Checking for direct URL comparisons..."
rg -i "url\s*===\s*|url\s*!==\s*|url\.equals|url\.compare" --type ts
# Test 2: Check frame URL storage patterns
echo "Checking frame URL storage patterns..."
rg "framesToSave\[.*\]|framesStore\[.*\]" --type ts
Length of output: 606
Script:
#!/bin/bash
# Check the implementation of frame-related functions
ast-grep --pattern 'function fetchFramesForMessage($_) {
$$$
}'
# Also check for any other frame URL processing
ast-grep --pattern 'function $_ ($_) {
$$$
url.toLowerCase()
$$$
}'
# Check frame validation logic
rg -A 5 "validateFrame" --type ts
Length of output: 1724
components/Chat/Message/Message.tsx (2)
10-10: LGTM: Clean import addition
The new utility import is properly placed and follows the codebase's import organization pattern.
171-177: Verify consistent usage of isFrameMessage across the codebase
The implementation looks correct, using useShallow for performance optimization. Let's verify that all other usages of isFrameMessage have been updated to match this new signature.
✅ Verification successful
No issues found with isFrameMessage usage
The implementation in utils/frames.ts matches exactly with the usage in components/Chat/Message/Message.tsx. The function signature and parameter order are consistent:
messageIsText: boolean - Passed asisContentType("text", message.contentType)messageContent: string - Passed asmessage.contentframesStore: object - Passed ass.frames
The function is correctly used within a useShallow hook to prevent unnecessary re-renders, and all parameters are passed in the expected order. The implementation in utils/frames.ts properly handles the validation of these parameters.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for all usages of isFrameMessage to ensure consistent parameter usage
rg "isFrameMessage\(" --type ts --type tsx -A 3 -B 1
Length of output: 82
Script:
#!/bin/bash
# Search for all usages of isFrameMessage without file type restrictions
rg "isFrameMessage\(" -A 3 -B 1
Length of output: 351
Script:
#!/bin/bash
# Find the definition of isFrameMessage function
ast-grep --pattern 'function isFrameMessage($_) { $$$ }'
Length of output: 58
Script:
#!/bin/bash
# Find any const/let/var declarations of isFrameMessage
ast-grep --pattern '(const|let|var) isFrameMessage = $_'
Length of output: 58
Script:
#!/bin/bash
# Find files that might contain isFrameMessage
fd "isFrameMessage"
Length of output: 19
Script:
#!/bin/bash
# Try to find isFrameMessage in any TypeScript/JavaScript file using a more flexible pattern
rg -l "isFrameMessage" --type ts --type js --type jsx --type tsx || rg -l "isFrameMessage"
Length of output: 167
Script:
#!/bin/bash
# Look for export statements containing isFrameMessage
rg "export.*isFrameMessage"
Length of output: 77
Script:
#!/bin/bash
# Search for the file content where isFrameMessage might be defined
fd -e ts -e tsx -e js -e jsx -x cat {} | rg -A 10 -B 10 "isFrameMessage.*=.*"
Length of output: 721
Script:
#!/bin/bash
# Get the full content of frames.ts to see the complete isFrameMessage implementation
cat utils/frames.ts
Length of output: 9021
Script:
#!/bin/bash
# Get the full content of Message.tsx to see the complete usage context
cat components/Chat/Message/Message.tsx
Length of output: 22852
| const content = messageContent.toLowerCase().trim(); | ||
| const sanitizedContent = content.endsWith("/") | ||
| ? content.slice(0, -1) | ||
| : content; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider extracting URL normalization logic.
The URL normalization logic could be reused in other parts of the codebase. Consider extracting it into a separate utility function.
+const normalizeUrl = (url: string): string => {
+ const normalized = url.toLowerCase().trim();
+ return normalized.endsWith("/") ? normalized.slice(0, -1) : normalized;
+};
export const isFrameMessage = (
messageIsText: boolean,
messageContent: string,
framesStore: {
[frameUrl: string]: FrameWithType;
}
): boolean => {
if (!messageIsText) return false;
- const content = messageContent.toLowerCase().trim();
- const sanitizedContent = content.endsWith("/")
- ? content.slice(0, -1)
- : content;
+ const sanitizedContent = normalizeUrl(messageContent);
return !!framesStore[sanitizedContent];
};📝 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 content = messageContent.toLowerCase().trim(); | |
| const sanitizedContent = content.endsWith("/") | |
| ? content.slice(0, -1) | |
| : content; | |
| const normalizeUrl = (url: string): string => { | |
| const normalized = url.toLowerCase().trim(); | |
| return normalized.endsWith("/") ? normalized.slice(0, -1) : normalized; | |
| }; | |
| export const isFrameMessage = ( | |
| messageIsText: boolean, | |
| messageContent: string, | |
| framesStore: { | |
| [frameUrl: string]: FrameWithType; | |
| } | |
| ): boolean => { | |
| if (!messageIsText) return false; | |
| const sanitizedContent = normalizeUrl(messageContent); | |
| return !!framesStore[sanitizedContent]; | |
| }; |
* [create-pull-request] automated change (#1122) Co-authored-by: alexrisch <[email protected]> * fix: Set is Frame when ending with slash (#1129) Added handling to clean url Updated to use util function --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: alexrisch <[email protected]>
* [create-pull-request] automated change (#1122) Co-authored-by: alexrisch <[email protected]> * fix: Set is Frame when ending with slash (#1129) Added handling to clean url Updated to use util function * Allow ethereum deeplinks inside frames (#1198) * Allow ethereum deeplinks inside frames * Set and new file * Bump app json * [create-pull-request] automated change (#1195) Co-authored-by: alexrisch <[email protected]> * [create-pull-request] automated change (#1197) Co-authored-by: alexrisch <[email protected]> --------- Co-authored-by: Noé Malzieu <[email protected]> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: alexrisch <[email protected]> * Fix linking for ethereum: links (#1208) --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: alexrisch <[email protected]> Co-authored-by: Noé Malzieu <[email protected]>
* [create-pull-request] automated change (#1122) Co-authored-by: alexrisch <[email protected]> * fix: Set is Frame when ending with slash (#1129) Added handling to clean url Updated to use util function * Allow ethereum deeplinks inside frames (#1198) * Allow ethereum deeplinks inside frames * Set and new file * Bump app json * [create-pull-request] automated change (#1195) Co-authored-by: alexrisch <[email protected]> * [create-pull-request] automated change (#1197) Co-authored-by: alexrisch <[email protected]> --------- Co-authored-by: Noé Malzieu <[email protected]> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: alexrisch <[email protected]> * Fix linking for ethereum: links (#1208) --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: alexrisch <[email protected]> Co-authored-by: Noé Malzieu <[email protected]>
* [create-pull-request] automated change (#1122) Co-authored-by: alexrisch <[email protected]> * fix: Set is Frame when ending with slash (#1129) Added handling to clean url Updated to use util function --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: alexrisch <[email protected]>
* [create-pull-request] automated change (#1122) Co-authored-by: alexrisch <[email protected]> * fix: Set is Frame when ending with slash (#1129) Added handling to clean url Updated to use util function * Allow ethereum deeplinks inside frames (#1198) * Allow ethereum deeplinks inside frames * Set and new file * Bump app json * [create-pull-request] automated change (#1195) Co-authored-by: alexrisch <[email protected]> * [create-pull-request] automated change (#1197) Co-authored-by: alexrisch <[email protected]> --------- Co-authored-by: Noé Malzieu <[email protected]> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: alexrisch <[email protected]> * Fix linking for ethereum: links (#1208) --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: alexrisch <[email protected]> Co-authored-by: Noé Malzieu <[email protected]>
Added handling to clean url
Updated to use util function
Summary by CodeRabbit
New Features
Bug Fixes
Refactor