feat: add MailDev as iframe in devtools #626
Conversation
WalkthroughAdds a new MailDevDevtoolPanel React component rendering a sandboxed iframe to http://localhost:1080/#/, and integrates it into the TanStack Devtools plugins list in the root route to expose a MailDev panel. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant App as App (__root.tsx)
participant Devtools as TanStack Devtools
participant Panel as MailDevDevtoolPanel
participant Iframe as iframe (maildev)
participant MailDev as MailDev Server (localhost:1080)
User->>App: Open application
App->>Devtools: Initialize devtools with plugins
Devtools->>Panel: Render MailDev panel
Panel->>Iframe: Create iframe (sandbox: same-origin, scripts)
Iframe->>MailDev: GET http://localhost:1080/#/
MailDev-->>Iframe: Serve MailDev UI
Iframe-->>User: Display MailDev interface
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
|
![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/routes/__root.tsx (1)
114-117: Consider dev-only rendering and improve naming consistency.The integration looks correct, but consider two improvements:
Dev-only rendering: The MailDev panel will always attempt to render, even in production builds. Consider adding a conditional check:
...(import.meta.env.DEV ? [{ name: 'MailDev', render: <MailDevDevtoolPanel />, }] : []),Naming consistency: Other panels use simpler names ("Tanstack Query", "Tanstack Router"). Consider changing "MailDev iframe" to just "MailDev" for consistency.
Apply this diff to implement both improvements:
{ name: 'Tanstack Router', render: <TanStackRouterDevtoolsPanel />, }, - { - name: 'MailDev iframe', - render: <MailDevDevtoolPanel />, - }, + ...(import.meta.env.DEV + ? [ + { + name: 'MailDev', + render: <MailDevDevtoolPanel />, + }, + ] + : []), ]}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/devtools/maildev.tsx(1 hunks)src/routes/__root.tsx(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/devtools/maildev.tsx (1)
app/routes/__root.tsx (1)
RootComponent(97-106)
src/routes/__root.tsx (1)
src/devtools/maildev.tsx (1)
MailDevDevtoolPanel(1-12)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: 🔬 Tests (lts/*)
- GitHub Check: 🔬 Tests (22)
- GitHub Check: 🔬 Tests (24)
- GitHub Check: Playwright E2E Tests
🔇 Additional comments (1)
src/routes/__root.tsx (1)
23-23: LGTM!The import is correctly structured.
| export function MailDevDevtoolPanel() { | ||
| return ( | ||
| <div className="h-full"> | ||
| <iframe | ||
| className="h-full w-full" | ||
| src="http://localhost:1080/#/" | ||
| // eslint-disable-next-line @eslint-react/dom/no-unsafe-iframe-sandbox | ||
| sandbox="allow-same-origin allow-scripts" | ||
| /> | ||
| </div> | ||
| ); | ||
| } |
There was a problem hiding this comment.
Address security, accessibility, and reliability concerns.
Several issues need attention:
-
Critical security issue: Combining
allow-same-originandallow-scriptsin the sandbox attribute negates the sandbox protection. Scripts from the same origin can remove the sandbox attribute entirely, defeating its purpose. Either removeallow-same-originor use a more restrictive sandbox policy. -
Hardcoded localhost URL: The URL
http://localhost:1080/#/won't work in non-local environments. Consider making this configurable via an environment variable (e.g.,VITE_MAILDEV_URL). -
Missing accessibility attribute: The iframe lacks a
titleattribute, which is required for screen readers. -
No error handling: If MailDev isn't running, users will see a broken iframe with no feedback.
Apply this diff to address the critical security issue, add accessibility, and make the URL configurable:
+const MAILDEV_URL = import.meta.env.VITE_MAILDEV_URL || 'http://localhost:1080/#/';
+
export function MailDevDevtoolPanel() {
return (
<div className="h-full">
<iframe
className="h-full w-full"
- src="http://localhost:1080/#/"
+ src={MAILDEV_URL}
+ title="MailDev Email Testing Interface"
- // eslint-disable-next-line @eslint-react/dom/no-unsafe-iframe-sandbox
- sandbox="allow-same-origin allow-scripts"
+ sandbox="allow-scripts"
/>
</div>
);
}Note: Removing allow-same-origin means the iframe content will be treated as from a unique origin, which is safer but may affect functionality if MailDev needs to access localStorage or make same-origin requests. Test thoroughly after this change.
🤖 Prompt for AI Agents
In src/devtools/maildev.tsx around lines 1-12, update the iframe to remove the
unsafe sandbox combination by dropping "allow-same-origin" and keeping only the
minimum needed (e.g., "allow-scripts"), replace the hardcoded src with a value
read from an environment variable (use VITE_MAILDEV_URL with a sensible default
like "http://localhost:1080/#/"), add a descriptive title attribute for
accessibility, and add simple error handling: track iframe load/error
(onLoad/onError or a load timeout) and render a user-friendly fallback message
when MailDev cannot be reached so users get feedback instead of a broken iframe.
2 participants
Describe your changes
Added MailDev in DevTools
Screenshots
Documentation
Checklist
Summary by CodeRabbit