Make ircs:// links work#62
Conversation
- Remove problematic IRC link handling code that was causing parse errors - Fix syntax issues in MessageItem component by properly structuring conditional returns - Add missing imports (useEffect, useRef, useState, ircColors, uuidv4, platform, hooks) - Reorganize imports for better consistency
- Clean up import organization - Fix EnhancedLinkWrapper component formatting
|
Warning Rate limit exceeded@ValwareIRC has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 23 minutes and 10 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughEnhancedLinkWrapper and MessageItem signatures were extended to accept an optional onIrcLinkClick callback; link clicks for irc:// and ircs:// are intercepted and routed via that callback. ChatArea implements handleIrcLinkClick which parses IRC URLs and dispatches store actions (connect/joinChannel). Dev tooling versions (biome/@biomejs) were bumped. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant ELW as EnhancedLinkWrapper
participant Msg as MessageItem
participant Chat as ChatArea
participant Store
User->>ELW: Click irc:// or ircs:// link
ELW->>ELW: detect scheme, preventDefault()
ELW->>Msg: call onIrcLinkClick(url)
Msg->>Chat: forward onIrcLinkClick(url)
Chat->>Store: connect(parsedServer)
Chat->>Store: joinChannel(parsedChannel)
Note right of Store: log errors if actions fail
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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. Comment |
ValwareIRC
changed the title
|
Automated deployment preview for the PR in the Cloudflare Pages. |
- Implement handleIrcLinkClick function to connect to IRC servers from irc:// and ircs:// links - Update MessageItem component to accept onIrcLinkClick prop - Pass handleIrcLinkClick to all EnhancedLinkWrapper components in MessageItem - Fix parseInt radix warning in linting
- Update biome.json schema version to 2.2.4 - Run biome migrate to fix configuration format - Format all files according to current biome rules
- Change IRC link clicks to open the 'Connect to server' modal with pre-filled server details - Remove automatic connection logic - Add toggleAddServerModal to useStore destructuring - Parse IRC URL and populate modal with host, port, and current username
![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 (1)
src/components/layout/ChatArea.tsx (1)
122-131: Anchor handler: use currentTarget.href and stop propagationAvoid relying on closure index and ensure parent handlers don’t double-handle clicks.
Apply:
- onClick={(e) => { - if ( - (matches[index].startsWith("ircs://") || - matches[index].startsWith("irc://")) && - onIrcLinkClick - ) { - e.preventDefault(); - onIrcLinkClick(matches[index]); - } - }} + onClick={(e) => { + const href = (e.currentTarget as HTMLAnchorElement).href; + if ( + (href.startsWith("ircs://") || href.startsWith("irc://")) && + onIrcLinkClick + ) { + e.preventDefault(); + e.stopPropagation(); + onIrcLinkClick(href); + } + }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/layout/ChatArea.tsx(7 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/components/layout/ChatArea.tsx (2)
src/lib/ircUtils.tsx (1)
mircToHtml(167-279)src/lib/ircClient.ts (2)
connect(79-144)joinChannel(165-189)
🔇 Additional comments (8)
src/components/layout/ChatArea.tsx (8)
95-99: Prop plumbing looks goodIntroducing onIrcLinkClick here is clean and optional; no concerns.
184-192: MessageItem prop addition is consistentOptional onIrcLinkClick threaded correctly through props.
220-223: System message wrapping LGTMWrapping system content with EnhancedLinkWrapper enables IRC link handling uniformly.
360-363: Reply preview link handling LGTMPassing onIrcLinkClick through keeps behavior consistent in reply blocks.
365-367: Primary message link handling LGTMMain message content is wrapped correctly.
432-434: Store methods are correctly selectedconnect/joinChannel exposure from the store reads well.
1158-1159: Wiring onIrcLinkClick to MessageItem LGTMPropagation looks correct.
439-468: Ensure channel joins occur only after IRC registration completes
I didn’t find any handlers for 001/RPL_WELCOME or anonRegisteredevent—please verify thatjoinChannel(server.id, channel)is invoked only after the registration event (e.g., post-001 or your store’s “registered” event) to avoid “451 You have not registered” errors.
- Parse IRC URLs with proper handling of channels in pathname or hash - Extract nick and password from URL query parameters - Reuse existing server connections to avoid duplicates - Clean trailing punctuation from URLs in chat text - Connect directly instead of opening modal for better UX - Fix linting issues with void return in forEach callback
- Update @biomejs/biome from 2.2.0 to 2.2.4 in package.json - Update biome.json schema version to 2.2.4 - Run biome migrate to update configuration - Format all files to match CI expectations
- Remove automatic connection logic from IRC link clicks - Open the 'Connect to server' modal with pre-filled server details - Add toggleAddServerModal back to useStore destructuring - Make handleIrcLinkClick synchronous since no async operations - Fix default ports (6697 for ircs, 6667 for irc)
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
biome.json(1 hunks)package.json(1 hunks)src/components/layout/ChatArea.tsx(7 hunks)
✅ Files skipped from review due to trivial changes (2)
- biome.json
- package.json
🧰 Additional context used
🧬 Code graph analysis (1)
src/components/layout/ChatArea.tsx (3)
src/lib/ircUtils.tsx (1)
mircToHtml(167-279)src/hooks/useTabCompletion.ts (1)
useTabCompletion(31-154)src/lib/ircClient.ts (2)
connect(79-144)joinChannel(165-189)
🔇 Additional comments (1)
src/components/layout/ChatArea.tsx (1)
478-479: Don’t passfalseinto the password slot
ircClient.connectis defined asconnect(host, port, nickname, password?, _saslAccountName?, _saslPassword?)(seesrc/lib/ircClient.ts). Supplyingfalseas the fourth argument makes the client skip the PASS command, and the real password lands in the SASL placeholder, so auth-protected links can’t succeed. Callconnectwith the parsed password directly (and only supply SASL args if you have them).- const server = - existing ?? (await connect(host, port, nick, false, password, "", "")); + const server = + existing ?? (await connect(host, port, nick, password));
matheusfillipe
left a comment
matheusfillipe
left a comment
There was a problem hiding this comment.
In general not super happy that we are handling irc/ircs and assuming it would be ws and wss respectively. Mixed feelings about that but since idk the native TCP will even work fine to merge.
| const urlObj = new URL(sanitized); | ||
| const host = urlObj.hostname; | ||
| const scheme = urlObj.protocol.replace(":", ""); | ||
| const port = urlObj.port |
There was a problem hiding this comment.
How do we distinct TCP from websocket... when it is stupported if it is ever supported. Technically we are connecting to ws://irc.something.com or wss://irc.something.com, because the default irc ports would be 6667 and 6697. If we say the native clients should support these urls they probably should use the TCP ones when that is supported, as most urls like this would have a TCP one but not necessarily websocket. Uff not sure what to do here.
There was a problem hiding this comment.
I think when we support TCP we can change the default to be 6697, but for now this is fine as a default, and usually the port would be specified in the URL anyway so we could go by that when specified
There was a problem hiding this comment.
But the web version will never support TCP, is only the native, and if native URL handling is different from web is weird. We could have a bouncer but still the web version should run without a bouncer...
| const handleIrcLinkClick = (rawUrl: string) => { | ||
| // Tolerate trailing punctuation in chat text | ||
| const sanitized = rawUrl.trim().replace(/[),.;:]+$/, ""); | ||
| const urlObj = new URL(sanitized); | ||
| const host = urlObj.hostname; | ||
| const scheme = urlObj.protocol.replace(":", ""); | ||
| const port = urlObj.port | ||
| ? Number.parseInt(urlObj.port, 10) | ||
| : scheme === "ircs" | ||
| ? 443 | ||
| : 8000; | ||
|
|
||
| // Channels may be in pathname (/chan1,chan2) or in hash (#chan1,chan2) | ||
| const rawChannelStr = | ||
| urlObj.pathname.length > 1 | ||
| ? urlObj.pathname.slice(1) | ||
| : urlObj.hash.startsWith("#") | ||
| ? urlObj.hash.slice(1) | ||
| : ""; | ||
| const channels = rawChannelStr | ||
| .split(",") | ||
| .filter(Boolean) | ||
| .map((c) => decodeURIComponent(c)) | ||
| .map((c) => | ||
| c.startsWith("#") || | ||
| c.startsWith("&") || | ||
| c.startsWith("+") || | ||
| c.startsWith("!") | ||
| ? c | ||
| : `#${c}`, | ||
| ); |
There was a problem hiding this comment.
This adds a lot of business logic for the url parser that should be not part of the ChatArea.tsx, this could be a separated thing that can be tested independently with a few test cases. Specially for cases that are easy and testable like this it is easy to add unit tests.
There was a problem hiding this comment.
Yes, I agree. Later!
Make ircs:// links work
2 participants
This PR fixes persistent linting parse errors in ChatArea.tsx, adds comprehensive IRC link handling functionality, and updates the biome configuration:
Linting Fixes:
IRC Link Handling:
Biome Configuration Updates:
The codebase now passes all lint checks and provides comprehensive IRC URL support that handles real-world IRC link formats and provides a safe, user-controlled connection experience.