Conversation
WalkthroughThis update introduces comprehensive browser extension support to Flow Browser, spanning backend management, IPC communication, localization, permission warnings, and a full-featured user interface. Key changes include new modules for extension management, localization, and permission warning generation; integration of extension lifecycle with user profiles and tabs; and the addition of extension APIs to the global Flow API. The UI is enhanced with extension management pages, sidebar actions, and context providers. Routing and window management are updated to support extension popups and management flows. The permission system is modularized, and new utility functions and types are added to support these features. Changes
Sequence Diagram(s)sequenceDiagram
participant Renderer
participant Preload
participant MainProcess
participant ExtensionManager
participant ProfileManager
Renderer->>Preload: flow.extensions.getAllInCurrentProfile()
Preload->>MainProcess: IPC: extensions:get-all-in-current-profile
MainProcess->>ExtensionManager: getInstalledExtensions()
ExtensionManager-->>MainProcess: [ExtensionData]
MainProcess-->>Preload: [SharedExtensionData]
Preload-->>Renderer: [SharedExtensionData]
Renderer->>Preload: flow.extensions.setExtensionEnabled(id, enabled)
Preload->>MainProcess: IPC: extensions:set-enabled
MainProcess->>ExtensionManager: setExtensionDisabled(id, !enabled)
ExtensionManager-->>MainProcess: success/failure
MainProcess-->>Preload: result
Preload-->>Renderer: result
Renderer->>Preload: flow.extensions.uninstallExtension(id)
Preload->>MainProcess: IPC: extensions:uninstall
MainProcess->>ExtensionManager: uninstallExtension(id)
ExtensionManager-->>MainProcess: success/failure
MainProcess-->>Preload: result
Preload-->>Renderer: result
ExtensionManager-->>MainProcess: "cache-updated" event
MainProcess->>Renderer: IPC: extensions:on-updated
Possibly related PRs
Suggested labels
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
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. 🪧 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 (
|
iamEvanYT
force-pushed
the
evan/extensions
branch
from
eaa1390 to
20f76ed
Compare
iamEvanYT
force-pushed
the
evan/extensions
branch
from
20f76ed to
805cb41
Compare
iamEvanYT
force-pushed
the
evan/extensions
branch
from
44513c2 to
6faa0e5
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
🔭 Outside diff range comments (1)
electron/browser/profile-manager.ts (1)
323-330: 🛠️ Refactor suggestionDispose extension resources during profile unload
handleProfileUnloaddestroys tabs but never calls any tear-down on
profile.extensionsorprofile.extensionsManager.
Both objects keep filesystem watchers and IPC listeners alive, leaking memory and potentially leaking PII (e.g. icon protocol handler) after the profile is “unloaded”.- if (this.profiles.delete(profileId)) { + const profile = this.profiles.get(profileId); + if (this.profiles.delete(profileId)) { + // Gracefully dispose extension-related resources + profile?.extensions?.removeAllListeners?.(); + await profile?.extensionsManager?.dispose?.();(Adapt the exact API names; both libraries expose
dispose/destroyfunctions.)
🧹 Nitpick comments (36)
docs/api/extensions/index.md (1)
7-7: Consider adding consistent punctuationAdd a period at the end of the line to maintain consistent punctuation across list items.
- - [Permission Warnings](./permission-warnings.md): Converts extension manifest permissions into user-facing warning messages + - [Permission Warnings](./permission-warnings.md): Converts extension manifest permissions into user-facing warning messages.🧰 Tools
🪛 LanguageTool
[uncategorized] ~7-~7: A punctuation mark might be missing here.
Context: ...ssions into user-facing warning messages - Locales: Handles internat...(AI_EN_LECTOR_MISSING_PUNCTUATION)
vite/src/routes/main-ui/route.tsx (1)
3-14: Consider adding a loading indicator.The component currently renders null while the page is loading, which might appear as a blank screen to users, especially on slower devices. Adding a simple loading indicator would improve user experience.
export default function Route() { const [Page, setPage] = useState<ReactNode>(null); + const [isLoading, setIsLoading] = useState(true); useEffect(() => { import("./page").then((module) => { const Page = module.default; setPage(<Page />); + setIsLoading(false); }); }, []); + if (isLoading) { + return <div className="flex items-center justify-center h-screen">Loading...</div>; + } return Page; }vite/src/lib/flow/interfaces/settings/settings.ts (1)
1-1: Standardized event listener patternUsing the
IPCListenertype for theonSettingsChangedevent standardizes the event registration pattern across the codebase, improving consistency and maintainability.Consider addressing the static analysis warning
The static analysis tool flags using
voidinIPCListener<[void]>. While this usage works, you might consider usingundefinedinstead:- onSettingsChanged: IPCListener<[void]>; + onSettingsChanged: IPCListener<[undefined]>;Or alternatively:
- onSettingsChanged: IPCListener<[void]>; + onSettingsChanged: IPCListener<[]>;The empty tuple
[]would more clearly indicate a callback with no parameters.Also applies to: 26-26
vite/src/lib/flow/interfaces/browser/interface.ts (1)
1-2: Standardized event listener patternUsing the
IPCListenertype for theonToggleSidebarevent standardizes the event registration pattern across the codebase, improving consistency and maintainability.Consider addressing the static analysis warning
The static analysis tool flags using
voidinIPCListener<[void]>. While this usage works, you might consider usingundefinedinstead:- onToggleSidebar: IPCListener<[void]>; + onToggleSidebar: IPCListener<[undefined]>;Or alternatively:
- onToggleSidebar: IPCListener<[void]>; + onToggleSidebar: IPCListener<[]>;The empty tuple
[]would more clearly indicate a callback with no parameters.Also applies to: 22-22
electron/modules/extensions/locales.ts (2)
61-74: Consider optimizing the string lookup mechanism.The function assumes that the input string directly matches a key in the locale data. In some cases, extensions might use a different key scheme than the displayed string.
You might want to consider adding support for a mapping between displayed strings and message keys if needed:
async function translateString(extensionPath: string, locale: string, str: string): Promise<string> { try { const localeData = await getLocaleData(extensionPath, locale); if (!localeData) return str; const translation = localeData[str]; - if (!translation) return str; + if (!translation) { + // Try alternative lookup methods or mappings here if needed + return str; + } return translation.message; } catch (error) { console.error(`Error translating string "${str}" for locale ${locale}:`, error); return str; } }
76-105: Consider implementing a caching mechanism for translations.The
transformStringToLocalefunction is well-implemented with a proper fallback chain from user locale to default locale. However, for frequently translated strings, you might want to add caching to improve performance.You could add a cache for translated strings to avoid repeated filesystem operations and JSON parsing:
+ // Simple in-memory cache for translations + const translationCache: Record<string, Record<string, string>> = {}; export async function transformStringToLocale( extensionPath: string, str: string, targetLocale?: string ): Promise<string> { try { + // Check cache first + const cacheKey = `${extensionPath}:${str}`; + const userLocale = app.getLocale(); + if (translationCache[cacheKey]?.[userLocale]) { + return translationCache[cacheKey][userLocale]; + } const locales = await getAllLocales(extensionPath); if (locales.length === 0) return str; // Try to translate to user locale - const userLocale = app.getLocale(); const hasUserLocale = locales.includes(userLocale); if (hasUserLocale) { - return await translateString(extensionPath, userLocale, str); + const translation = await translateString(extensionPath, userLocale, str); + // Cache the result + if (!translationCache[cacheKey]) translationCache[cacheKey] = {}; + translationCache[cacheKey][userLocale] = translation; + return translation; } // Rest of the function remains the same...electron/browser/window.ts (2)
140-146: Implement the popup UI as noted in the TODO.The code loads different URLs based on window type, but the TODO comment indicates that popup UI implementation might be incomplete.
Would you like assistance with implementing the popup UI mentioned in the TODO comment?
211-218: Consider documenting the rationale for the 500ms timeout.The code includes a 500ms timeout before destroying tabs, but it's not immediately clear why this specific value was chosen.
// Destroy all tabs in the window // Do this so that it won't run if the app is closing // Technically after 500ms, the app is dead, so it won't run. + // Use 500ms timeout to ensure we don't block the app exit process while still + // allowing enough time for cleanup in normal window closure scenarios setTimeout(() => { for (const tab of this.browser.tabs.getTabsInWindow(this.id)) { tab.destroy(); } }, 500);vite/src/lib/flow/interfaces/app/extensions.ts (1)
16-30: Consider adding a method to retrieve a single extension.The API provides methods for managing multiple extensions but lacks a method to retrieve a single extension by ID, which could be useful for individual extension operations.
Consider adding a method to retrieve a single extension by ID:
export interface FlowExtensionsAPI { /** * Get all extensions in the current profile */ getAllInCurrentProfile: () => Promise<SharedExtensionData[]>; + /** + * Get a specific extension by ID in the current profile + */ + getExtensionById: (extensionId: string) => Promise<SharedExtensionData | null>; + /** * Listen for updates to the extensions in the current profile */ onUpdated: IPCListener<[string, SharedExtensionData[]]>; // Rest of the interface... }electron/ipc/app/extensions.ts (3)
60-61: Simplify boolean expressions in extension data generation.The ternary operations with boolean literals can be simplified.
- enabled: extensionData.disabled ? false : true, - pinned: extensionData.pinned ? true : false, + enabled: !extensionData.disabled, + pinned: !!extensionData.pinned,🧰 Tools
🪛 Biome (1.9.4)
[error] 60-60: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with(lint/complexity/noUselessTernary)
[error] 61-61: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with(lint/complexity/noUselessTernary)
66-67: Implement the inspect views functionality.The TODO comment indicates that inspect views functionality is not yet implemented.
Would you like help implementing the inspect views functionality for extensions debugging?
133-172: Consider handling the case where extension icon cannot be loaded.The uninstall confirmation dialog uses the extension icon, but there's no fallback handling if the icon can't be loaded.
const extensionIcon = await getExtensionIcon(sharedExtensionData.path); const returnValue = await dialog.showMessageBox(window.window, { - icon: extensionIcon ?? undefined, + // Only set icon if we successfully retrieved one + ...(extensionIcon ? { icon: extensionIcon } : {}), title: "Uninstall Extension", message: `Are you sure you want to uninstall "${sharedExtensionData.name}"?`, buttons: ["Cancel", "Uninstall"] });vite/src/routes/extensions/components/extension-card.tsx (1)
8-20: Consider using a shared type instead of duplicating the Extension interface.The comment indicates this interface is kept for backward compatibility, but duplicating types can lead to maintenance issues.
Consider importing the shared type from a common location and extending it if needed, rather than duplicating the interface definition. This will make future updates easier to maintain.
-// Keeping this for backward compatibility -export interface Extension { - id: string; - name: string; - description: string; - icon: string; - enabled: boolean; - version: string; - path?: string; - size?: string; - permissions?: string[]; - inspectViews?: string[]; -} +// If needed for backward compatibility +export type Extension = Omit<SharedExtensionData, 'pinned' | 'type'> & { + path?: string; + size?: string; + permissions?: string[]; + inspectViews?: string[]; +};vite/src/routes/extensions/page.tsx (4)
20-20: Use consistent URL parameter handlingYou're directly using URLSearchParams for parameter extraction. Consider using the router's built-in facilities for handling URL parameters for better consistency with the rest of the application.
- const selectedExtensionId = new URLSearchParams(router.search).get("id"); + const { id: selectedExtensionId } = router.getParams();
99-110: Disabled developer mode toggle needs explanationThe developer mode toggle is disabled but still rendered in the UI. Either add a tooltip explaining why it's disabled or hide it completely if it's not functional yet.
<Switch checked={isDeveloperMode} onCheckedChange={setIsDeveloperMode} disabled id="developer-mode" + aria-label="Developer mode toggle - currently unavailable" /> <label htmlFor="developer-mode" className="text-sm font-medium leading-none peer-disabled:cursor-not-allowed peer-disabled:opacity-70" + title="Developer mode is not yet available" > Developer mode </label>
188-188: Consider applying user theme preferenceCurrently forcing dark theme for the extensions page, which may not respect user preferences. Consider using the user's theme preference instead of forcing dark mode.
- <ThemeProvider forceTheme="dark"> + <ThemeProvider>
26-38: Add error logging for failed operationsWhile you're showing error toasts to the user, it would be helpful to log errors to the console as well for debugging purposes.
const setExtensionEnabled = async (id: string, enabled: boolean) => { setIsProcessing(true); const success = await flow.extensions.setExtensionEnabled(id, enabled); if (success) { toast.success(`This extension has been successfully ${enabled ? "enabled" : "disabled"}!`); } else { toast.error(`Failed to ${enabled ? "enable" : "disable"} this extension!`); + console.error(`Failed to ${enabled ? "enable" : "disable"} extension with ID: ${id}`); } setIsProcessing(false); return success; };docs/api/extensions/permission-warnings.md (1)
65-67: Consider adding direct links to referencesFor better developer experience, consider adding direct links to the Chromium source files you're referencing, if they're publicly accessible.
- Rules logic: `chrome/common/extensions/permissions/chrome_permission_message_rules.cc` - Message strings: `chrome/app/generated_resources.grd` + Rules logic: [`chrome/common/extensions/permissions/chrome_permission_message_rules.cc`](https://source.chromium.org/chromium/chromium/src/+/main:chrome/common/extensions/permissions/chrome_permission_message_rules.cc) + Message strings: [`chrome/app/generated_resources.grd`](https://source.chromium.org/chromium/chromium/src/+/main:chrome/app/generated_resources.grd)shared/types/extensions.ts (2)
1-2: Export ExtensionInspectView typeThe
ExtensionInspectViewtype is used in theSharedExtensionDatainterface but isn't exported. Consider exporting it for reuse in other files.-type ExtensionInspectView = "service_worker" | "background"; +export type ExtensionInspectView = "service_worker" | "background";
17-17: Consider adding more specific permission typesThe
permissionsproperty is currently typed as a string array. Consider creating a more specific union type for common extension permissions to improve type safety.- permissions: string[]; + permissions: ExtensionPermission[]; // Add somewhere in the file: +export type ExtensionPermission = + | "tabs" + | "activeTab" + | "storage" + | "cookies" + | "webNavigation" + | "history" + | "downloads" + | "clipboardRead" + | "clipboardWrite" + | "notifications" + | string; // For any other permissionsvite/src/routes/extensions/components/extension-details.tsx (1)
59-68: Reuse this size formatting logicThe inline function for formatting file sizes could be extracted into a separate utility function for reuse across the application. This would improve maintainability and reduce duplication if similar formatting is needed elsewhere.
- {((size) => { - const units = ["bytes", "KB", "MB", "GB", "TB"]; - let i = 0; - let sizeValue = size; - while (sizeValue >= 1024 && i < units.length - 1) { - sizeValue /= 1024; - i++; - } - return i === 0 ? `${sizeValue} ${units[i]}` : `${sizeValue.toFixed(1)} ${units[i]}`; - })(extension.size)} + {formatFileSize(extension.size)}Consider adding a
formatFileSizeutility function in an appropriate utils file.vite/src/components/providers/extensions-provider.tsx (1)
28-36: Handle edge case when flow API is unavailableThe
fetchExtensionsfunction has a guard for whenflowis undefined, but it doesn't update state in this case. Consider setting an error state or empty extensions array to prevent stale data.const fetchExtensions = useCallback(async () => { if (!flow) return; try { const data = await flow.extensions.getAllInCurrentProfile(); setExtensions(data); } catch (error) { console.error("Failed to fetch extensions data:", error); + setExtensions([]); } }, []);vite/src/components/browser-ui/sidebar/header/address-bar/pinned-browser-actions.tsx (2)
8-8: Consider making MAX_PINNED_ACTIONS configurable.The maximum number of pinned actions is currently hardcoded as a constant. For flexibility, consider making this configurable through settings or a prop.
87-93: Consider adding error logging for icon loading failures.While the fallback to PuzzleIcon works well, it might be helpful to log errors when icon loading fails for debugging purposes.
- <image href={iconUrl} className="size-4 object-contain shrink-0" onError={() => setIsError(true)} /> + <image + href={iconUrl} + className="size-4 object-contain shrink-0" + onError={(e) => { + console.debug(`Failed to load extension icon: ${iconUrl}`, e); + setIsError(true); + }} + />electron/ipc/listeners-manager.ts (1)
120-123: Consider adding validation for listener inputs.The listener registration doesn't validate the channel or listener ID from the renderer. Consider adding basic validation.
ipcMain.on("listeners:add", (event, channel: string, listenerId: string) => { const webContents = event.sender; + if (!channel || typeof channel !== 'string' || !listenerId || typeof listenerId !== 'string') { + console.warn('Invalid channel or listenerId received'); + return; + } addListener(channel, listenerId, webContents); });vite/src/components/providers/browser-action-provider.tsx (2)
118-119: Consider making Y_PADDING configurable or platform-dependent.The fixed padding value might need adjustment based on the platform or window state.
- // Padding adjustment for native frame - const Y_PADDING = 20; + // Padding adjustment for native frame - may vary by platform + const Y_PADDING = process.platform === 'darwin' ? 20 : 22; // Example for platform-specific values
163-175: Consider error handling in useBrowserAction hook.The hook quietly returns default values when used outside a provider. Consider logging a warning to help debug misuse.
export function useBrowserAction(): BrowserActionContextType { const context = useContext(BrowserActionContext); if (context === null) { + console.warn("useBrowserAction() called outside of BrowserActionProvider. Using default values."); return { activeTabId: undefined, actions: [], activate: () => {}, isLoading: false, partition: "_self" }; } return context; }docs/api/extensions/locales.md (1)
17-20: Clarify thetargetLocaleargument or remove itThe documentation lists
targetLocaleas a third parameter but explicitly says it is “currently unused in implementation”.
Readers may assume it works. Either:
- Explain that the parameter is reserved for future use (and when it will become active), or
- Drop the parameter from the public signature to avoid confusion and keep docs & code in sync.
electron/browser/utility/protocols.ts (1)
231-238: Possible performance win: avoid PNG re-encode on every request
icon.toPNG()allocates a new buffer each time. If multiple renderer requests ask for the same icon (e.g., on every sidebar re-render), this will repeatedly hit the CPU.
Storing the PNG buffer alongside the cachedNativeImage(or memoisingtoPNG()) would eliminate redundant work.electron/browser/profile-manager.ts (1)
30-42:PopupViewtype mixes data & constantsThe
POSITION_PADDINGandBOUNDS.*properties are typed as numbers but hold no literal values.
If the intent is to expose real constants, define them outside the type:export const POPUP_POSITION_PADDING = 8; export const POPUP_BOUNDS = { minWidth: 330, /* … */ }; type PopupView = { browserWindow?: BrowserWindow; parent?: Electron.BaseWindow; extensionId: string; };Having “phantom” fields in the type leads to incorrect assumptions, harms IntelliSense, and can be misleading for future maintainers.
electron/modules/extensions/permission-warnings.ts (2)
57-63: Typographical error in user-facing stringsMessages use “Browse history”; the correct phrase is “Browsing history”.
- IDS_EXTENSION_PROMPT_WARNING_HISTORY: "Read and change your Browse history", + IDS_EXTENSION_PROMPT_WARNING_HISTORY: "Read and change your browsing history",(The same capitalisation appears in the subsequent three strings.)
These are end-user-visible so copy accuracy matters.
17-125: Consider externalising the 600-line literals to avoid bundle bloat
permissionMessagesandpermissionRulesembed > 1 kB of static JSON-like data directly in the module.
Shipping this code in the renderer bundle means every window pays the parse cost, even if the API isn’t used.Recommended:
- Move the mappings to a
*.jsonfile andimportit (tree-shaken by bundlers).- Lazy-load the module (
await import(...)) only when permissions must be displayed.This keeps initial renderer startup snappy, especially important for multi-window Electron apps.
electron/modules/extensions/management.ts (2)
370-377: Persistpinnedwith a default value
pinnedis omitted when a new extension is added, so subsequent reads may yieldundefinedinstead offalse, forcing extra nullish checks elsewhere.-.set(extensionId, { type, disabled: false }) +.set(extensionId, { type, disabled: false, pinned: false })
291-303: Wrapsession.loadExtension()in try/catch to avoid uncaught rejectionsA failed load (e.g., malformed manifest) will currently propagate an unhandled rejection that crashes the Electron main process. Consider defensive wrapping:
-let extension = await session.loadExtension(extensionPath); -if (!extension) { - return null; -} +let extension: Extension | null = null; +try { + extension = await session.loadExtension(extensionPath); +} catch (e) { + console.error(`Failed to load extension ${extensionId} from ${extensionPath}:`, e); + return null; +}electron/preload.ts (2)
76-85: Reuse existing UUID utility to avoid duplicated logic
generateUUID()duplicates the helper already exported fromvite/src/lib/utils.ts. Importing the shared implementation reduces bundle size and keeps behaviour consistent.-import { generateUUID } from "@/some/relative/path"; // adjust path - -function generateUUID(): string { - return "10000000-1000-4000-8000-100000000000".replace(/[018]/g, (c) => - (+c ^ (crypto.getRandomValues(new Uint8Array(1))[0] & (15 >> (+c / 4)))).toString(16) - ); -} +import { generateUUID } from "~/lib/utils"; // path relative to preload build alias
100-135:wrapAPI()should iterate over own properties onlyUsing
for…inwill also traverse inherited enumerable properties if the object is ever created viaObject.create(...). Switch toObject.keys()(orObject.entries()) to safeguard against prototype pollution.-for (const key in api) { +for (const key of Object.keys(api) as Array<keyof T>) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (61)
docs/api/extensions/index.md(1 hunks)docs/api/extensions/locales.md(1 hunks)docs/api/extensions/permission-warnings.md(1 hunks)electron/browser/browser.ts(1 hunks)electron/browser/components/omnibox.ts(1 hunks)electron/browser/profile-manager.ts(4 hunks)electron/browser/sessions.ts(1 hunks)electron/browser/tabs/tab-context-menu.ts(3 hunks)electron/browser/tabs/tab-manager.ts(3 hunks)electron/browser/tabs/tab.ts(10 hunks)electron/browser/utility/protocols.ts(5 hunks)electron/browser/window.ts(4 hunks)electron/ipc/app/extensions.ts(1 hunks)electron/ipc/listeners-manager.ts(1 hunks)electron/ipc/main.ts(1 hunks)electron/ipc/session/profiles.ts(2 hunks)electron/ipc/session/spaces.ts(2 hunks)electron/ipc/window/settings.ts(2 hunks)electron/modules/basic-settings.ts(2 hunks)electron/modules/extensions/locales.ts(1 hunks)electron/modules/extensions/main.ts(1 hunks)electron/modules/extensions/management.ts(1 hunks)electron/modules/extensions/permission-warnings.ts(1 hunks)electron/modules/typed-event-emitter.ts(1 hunks)electron/modules/utils.ts(1 hunks)electron/modules/windows.ts(1 hunks)electron/package.json(1 hunks)electron/preload.ts(4 hunks)electron/saving/tabs.ts(1 hunks)shared/types/extensions.ts(1 hunks)vite/src/App.tsx(3 hunks)vite/src/components/browser-ui/browser-action.tsx(1 hunks)vite/src/components/browser-ui/browser-sidebar.tsx(1 hunks)vite/src/components/browser-ui/main.tsx(4 hunks)vite/src/components/browser-ui/sidebar/header/action-buttons.tsx(2 hunks)vite/src/components/browser-ui/sidebar/header/address-bar/address-bar.tsx(2 hunks)vite/src/components/browser-ui/sidebar/header/address-bar/pinned-browser-actions.tsx(1 hunks)vite/src/components/omnibox/main.tsx(2 hunks)vite/src/components/providers/browser-action-provider.tsx(1 hunks)vite/src/components/providers/extensions-provider.tsx(1 hunks)vite/src/components/providers/spaces-provider.tsx(3 hunks)vite/src/components/settings/sections/external-apps/section.tsx(1 hunks)vite/src/lib/flow/flow.ts(1 hunks)vite/src/lib/flow/interfaces/app/extensions.ts(1 hunks)vite/src/lib/flow/interfaces/app/windows.ts(1 hunks)vite/src/lib/flow/interfaces/browser/interface.ts(2 hunks)vite/src/lib/flow/interfaces/browser/tabs.ts(2 hunks)vite/src/lib/flow/interfaces/sessions/profiles.ts(1 hunks)vite/src/lib/flow/interfaces/sessions/spaces.ts(2 hunks)vite/src/lib/flow/interfaces/settings/settings.ts(2 hunks)vite/src/lib/flow/types.ts(1 hunks)vite/src/lib/omnibox/omnibox.ts(1 hunks)vite/src/lib/omnibox/providers/pedal.ts(1 hunks)vite/src/routes/about/page.tsx(1 hunks)vite/src/routes/extensions/components/extension-card.tsx(1 hunks)vite/src/routes/extensions/components/extension-details.tsx(1 hunks)vite/src/routes/extensions/page.tsx(1 hunks)vite/src/routes/main-ui/page.tsx(1 hunks)vite/src/routes/main-ui/route.tsx(1 hunks)vite/src/routes/popup-ui/page.tsx(1 hunks)vite/src/routes/popup-ui/route.tsx(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (27)
vite/src/routes/main-ui/page.tsx (1)
vite/src/components/browser-ui/main.tsx (1)
BrowserUI(101-135)
vite/src/components/browser-ui/sidebar/header/action-buttons.tsx (1)
vite/src/components/browser-ui/browser-action.tsx (1)
BrowserActionList(144-201)
vite/src/components/browser-ui/sidebar/header/address-bar/address-bar.tsx (2)
vite/src/lib/utils.ts (1)
cn(5-7)vite/src/components/browser-ui/sidebar/header/address-bar/pinned-browser-actions.tsx (1)
PinnedBrowserActions(10-34)
electron/ipc/window/settings.ts (1)
electron/ipc/listeners-manager.ts (1)
sendMessageToListeners(34-43)
electron/ipc/session/profiles.ts (2)
electron/index.ts (1)
browser(10-10)electron/sessions/spaces.ts (1)
getSpace(52-61)
vite/src/routes/popup-ui/route.tsx (1)
vite/src/routes/main-ui/route.tsx (1)
Route(3-14)
vite/src/routes/main-ui/route.tsx (1)
vite/src/routes/popup-ui/route.tsx (1)
Route(3-14)
vite/src/lib/flow/interfaces/settings/settings.ts (1)
vite/src/lib/flow/types.ts (1)
IPCListener(8-8)
vite/src/lib/flow/interfaces/browser/tabs.ts (2)
vite/src/lib/flow/types.ts (1)
IPCListener(8-8)shared/types/tabs.ts (1)
WindowTabsData(49-54)
electron/modules/extensions/locales.ts (2)
electron/modules/utils.ts (1)
getFsStat(41-43)electron/modules/extensions/management.ts (1)
getManifest(68-75)
vite/src/lib/flow/interfaces/browser/interface.ts (1)
vite/src/lib/flow/types.ts (1)
IPCListener(8-8)
vite/src/routes/extensions/page.tsx (5)
vite/src/components/providers/extensions-provider.tsx (2)
useExtensions(12-18)ExtensionsProvider(25-76)vite/src/components/ui/button.tsx (1)
Button(50-50)vite/src/components/ui/card.tsx (2)
Card(56-56)CardContent(56-56)vite/src/components/ui/switch.tsx (1)
Switch(28-28)vite/src/components/main/theme.tsx (1)
ThemeProvider(22-99)
vite/src/routes/popup-ui/page.tsx (1)
vite/src/components/browser-ui/main.tsx (1)
BrowserUI(101-135)
vite/src/lib/flow/interfaces/app/extensions.ts (2)
shared/types/extensions.ts (1)
SharedExtensionData(5-19)vite/src/lib/flow/types.ts (1)
IPCListener(8-8)
vite/src/routes/extensions/components/extension-card.tsx (3)
shared/types/extensions.ts (1)
SharedExtensionData(5-19)vite/src/components/ui/switch.tsx (1)
Switch(28-28)vite/src/components/ui/button.tsx (1)
Button(50-50)
vite/src/components/providers/browser-action-provider.tsx (1)
vite/src/components/providers/spaces-provider.tsx (1)
useSpaces(18-24)
electron/browser/window.ts (1)
electron/index.ts (1)
browser(10-10)
vite/src/components/providers/extensions-provider.tsx (2)
shared/types/extensions.ts (1)
SharedExtensionData(5-19)vite/src/components/providers/spaces-provider.tsx (1)
useSpaces(18-24)
electron/browser/tabs/tab.ts (2)
electron/browser/profile-manager.ts (1)
LoadedProfile(20-28)electron/saving/tabs.ts (1)
removeTabFromStorage(38-41)
electron/modules/extensions/main.ts (1)
electron/browser/sessions.ts (1)
getSessionWithoutCreating(70-72)
electron/ipc/listeners-manager.ts (1)
electron/browser/window.ts (1)
TabbedBrowserWindow(28-250)
vite/src/lib/flow/interfaces/sessions/spaces.ts (1)
vite/src/lib/flow/types.ts (1)
IPCListener(8-8)
electron/browser/tabs/tab-context-menu.ts (1)
electron/browser/tabs/tab.ts (1)
Tab(114-931)
electron/browser/utility/protocols.ts (2)
electron/index.ts (1)
browser(10-10)electron/modules/extensions/management.ts (1)
getExtensionIcon(91-143)
vite/src/App.tsx (3)
vite/src/routes/extensions/route.tsx (1)
Route(3-14)vite/src/routes/main-ui/route.tsx (1)
Route(3-14)vite/src/routes/popup-ui/route.tsx (1)
Route(3-14)
vite/src/components/providers/spaces-provider.tsx (1)
vite/src/components/browser-ui/main.tsx (1)
WindowType(19-19)
electron/preload.ts (2)
vite/src/lib/utils.ts (1)
generateUUID(30-34)shared/types/extensions.ts (1)
SharedExtensionData(5-19)
🪛 LanguageTool
docs/api/extensions/index.md
[uncategorized] ~7-~7: A punctuation mark might be missing here.
Context: ...ssions into user-facing warning messages - Locales: Handles internat...
(AI_EN_LECTOR_MISSING_PUNCTUATION)
docs/api/extensions/locales.md
[uncategorized] ~52-~52: Loose punctuation mark.
Context: ...n - getLocalesRootPath(extensionPath): Returns the path to the extension's _l...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~57-~57: Loose punctuation mark.
Context: ...Retrieval - hasLocales(extensionPath): Checks if an extension has any locale f...
(UNLIKELY_OPENING_PUNCTUATION)
🪛 Biome (1.9.4)
vite/src/lib/flow/interfaces/settings/settings.ts
[error] 26-26: void is confusing outside a return type or a type parameter.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
vite/src/lib/flow/interfaces/browser/interface.ts
[error] 22-22: void is confusing outside a return type or a type parameter.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
electron/ipc/app/extensions.ts
[error] 60-60: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with
(lint/complexity/noUselessTernary)
[error] 61-61: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with
(lint/complexity/noUselessTernary)
vite/src/lib/flow/interfaces/sessions/spaces.ts
[error] 62-62: void is confusing outside a return type or a type parameter.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
🔇 Additional comments (109)
vite/src/components/settings/sections/external-apps/section.tsx (1)
16-16: Verify the updated import path for ExternalAppPermission.
The import has been moved to the new settings namespace—please confirm that@/lib/flow/interfaces/settings/openExternalindeed exportsExternalAppPermissionand that no stale imports remain under the old path.vite/src/lib/flow/types.ts (1)
8-8: Well-designed generic type for IPC event handling.The new
IPCListenertype provides a standardized pattern for IPC event registration and cleanup across the codebase. This type elegantly captures the event subscription pattern where you provide a callback and receive an unsubscribe function in return.electron/browser/browser.ts (1)
12-12: Extension module integration looks good.The new import brings in the extensions system module for browser integration. This is a clean way to initialize the extensions functionality without modifying the Browser class itself.
electron/modules/windows.ts (1)
16-16: Extension popup window type successfully added.The addition of
EXTENSION_POPUPto theWindowTypeenum properly supports the new extension popup windows functionality. This enum value will be used throughout the codebase to differentiate extension popups from other window types.vite/src/routes/main-ui/page.tsx (1)
4-4: Window type properly specified for BrowserUI.This change correctly sets the window type to "main" for the BrowserUI component, which is necessary for the component to render with the appropriate UI elements for the main window (versus popup windows).
vite/src/routes/about/page.tsx (1)
8-8: LGTM: Added extensions to hostnames listThis addition makes
flow://extensionsaccessible from the About page alongside other Flow browser URLs, which aligns perfectly with the PR's objective of adding extension support.vite/src/lib/omnibox/providers/pedal.ts (1)
24-28: LGTM: New omnibox pedal for extensionsThe implementation follows the established pattern for pedal commands, with appropriate triggers and description. This provides users with a convenient way to access the extensions manager through the omnibox.
vite/src/components/browser-ui/browser-sidebar.tsx (1)
89-89: LGTM: Updated to use the new windows APIGood architectural change that moves settings window management to the centralized
windowsnamespace, improving code organization and maintainability.electron/saving/tabs.ts (1)
23-23: LGTM: Added validation for navigation historyThis is an important safeguard that prevents tabs with empty navigation history from being persisted to storage. This helps maintain data integrity and consistent tab state, which is especially important with the new extension integration.
vite/src/routes/popup-ui/page.tsx (1)
1-10: Clean implementation of popup UI componentThe implementation is concise and follows React best practices. The component structure is clear, with proper separation of concerns between the
Pagecomponent that handles the specific UI type configuration and theAppwrapper component.vite/src/lib/flow/interfaces/sessions/profiles.ts (1)
27-31: Good API extension for profile context awarenessThe
getUsingProfilemethod is a good addition that will enable profile-aware features, which is essential for proper extension support. The method signature and documentation follow the established patterns in the codebase.docs/api/extensions/index.md (1)
1-8: Well-structured API documentation indexGood introduction to the Extensions API with clear organization and links to sub-APIs. This provides a solid entry point for developers to understand the extension capabilities.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~7-~7: A punctuation mark might be missing here.
Context: ...ssions into user-facing warning messages - Locales: Handles internat...(AI_EN_LECTOR_MISSING_PUNCTUATION)
electron/package.json (1)
24-25: Appropriate dependency updates for extension supportThe updates to
electron-chrome-extensions(4.6.9 → 4.7.5) andelectron-chrome-web-store(0.10.0 → 0.11.2) are appropriate for the extension support being added. Using scoped/namespaced packages (@iamevan/...) indicates you're using custom forks, which is a good practice when you need specific modifications.Do you have a changelog or list of specific features/fixes these updated versions provide that are required for your implementation?
vite/src/lib/flow/interfaces/app/windows.ts (1)
1-12: Clean, well-documented interface implementation.The
FlowWindowsAPIinterface is well-structured with clear method signatures and proper JSDoc comments. This interface appropriately encapsulates window management functionality that was previously mixed with other concerns.vite/src/lib/omnibox/omnibox.ts (2)
83-83: Good API migration to the new windows namespace.This correctly implements the migration from
flow.settings.open()to the newflow.windows.openSettingsWindow()method, maintaining consistent functionality while using the improved API structure.
86-87: Good implementation of the new extensions menu shortcut.The new pedal action for "open_extensions" properly integrates with the extension management system by opening the extensions page in a new tab.
electron/browser/components/omnibox.ts (1)
90-91: Good UX improvement for handling smaller window sizes.This change prevents the omnibox from exceeding the parent window dimensions, which improves usability when the window is resized to smaller dimensions.
- const omniboxWidth = 750; - const omniboxHeight = 350; + const omniboxWidth = Math.min(750, windowBounds.width); + const omniboxHeight = Math.min(350, windowBounds.height);vite/src/components/omnibox/main.tsx (2)
5-5: LGTM! Clean import statement addition.The PuzzleIcon import was added to the lucide-react imports correctly, maintaining alphabetical order.
31-33: LGTM! Well-implemented extension icon handler.The new conditional for "open_extensions" follows the same pattern as other pedal destinations, using the PuzzleIcon with consistent styling. The purple color choice maintains visual consistency with other "action" icons.
vite/src/components/browser-ui/sidebar/header/action-buttons.tsx (2)
1-1: LGTM! Clean import addition.The import for BrowserActionList is correctly added and follows project conventions.
167-170: LGTM! Good placement of browser actions in the sidebar.The BrowserActionList component is appropriately placed in the left side buttons container with correct alignment properties. The added comment makes the code more maintainable.
electron/browser/sessions.ts (1)
70-72: LGTM! Useful non-creating session accessor.This function correctly provides a way to retrieve a session without implicitly creating one, which is important for extension management. It simply returns the session from the map or undefined if not found.
vite/src/components/browser-ui/sidebar/header/address-bar/address-bar.tsx (3)
2-2: LGTM - Import of PinnedBrowserActions componentThis import supports the extension action functionality to be rendered in the address bar.
59-59: Good addition of truncate classThe
truncateclass ensures that long URLs display gracefully within the constrained space of the address bar by applying text truncation with an ellipsis.
62-62: Well-integrated extension actions componentThe
PinnedBrowserActionscomponent is nicely positioned before the copy link button in the right side container, allowing users to access pinned extension actions directly from the address bar.electron/ipc/window/settings.ts (1)
1-1: Improved message handling with centralized listeners managerReplacing direct browser message dispatch with the listeners manager improves the code architecture by:
- Centralizing message routing logic
- Enabling multiple listeners for the same event
- Reducing coupling with the browser object
This change aligns with the broader refactoring to standardize IPC communication patterns.
Also applies to: 31-31
vite/src/routes/popup-ui/route.tsx (1)
1-14: Implementation follows established patternsThe component structure follows the same lazy-loading pattern as other route components in the codebase, which is good for consistency and performance. It correctly uses React hooks and properly types the state variable.
vite/src/lib/flow/interfaces/browser/tabs.ts (1)
1-1: Improved type consistency for IPC listenersThe change to use the standardized
IPCListenergeneric type improves consistency across the codebase by aligning with the unified IPC event listener pattern. This will make the code more maintainable as all event listeners now follow the same typing pattern.Also applies to: 16-16
electron/ipc/session/profiles.ts (1)
4-5: Properly implemented profile context retrievalThe new IPC handler correctly retrieves the current profile ID by following the chain from window to space to profile. The implementation includes appropriate null checks at each step to handle edge cases gracefully.
Also applies to: 25-35
electron/ipc/main.ts (1)
1-26: Improved organization of IPC importsThe reorganization of imports into logical groupings (App APIs, Browser APIs, etc.) enhances readability and maintainability. The addition of new imports for extensions, omnibox, and the centralized listeners manager properly supports the new functionality being introduced.
electron/ipc/session/spaces.ts (3)
17-17: Good refactoring approach for centralized IPC communication.The import of centralized messaging functions from the listeners manager aligns with the broader IPC refactoring pattern. This approach will make the codebase more maintainable by standardizing how messages are sent across the application.
65-65: Well-implemented migration to centralized messaging.The function has been properly updated to use the new centralized IPC messaging system while maintaining the same event name. This preserves backward compatibility while improving the architectural pattern.
69-69: Consistent application of the messaging pattern.This change follows the same pattern as the previous one, replacing direct messaging with the centralized approach. The refactoring is consistent throughout the file.
electron/modules/basic-settings.ts (2)
86-95: Extension compatibility setting properly labeled as unstable.Good job marking this setting as [UNSTABLE] and placing it in the advanced section. Since Manifest V2 extensions are being deprecated by browser vendors in favor of Manifest V3, this setting provides useful backward compatibility while correctly warning users about potential instability.
118-125: Well-structured Advanced Settings card.The new settings card is properly categorized as "Advanced Settings" with appropriate messaging that it's for power users and may require a restart. This provides good UX by clearly separating experimental features from the main settings.
vite/src/lib/flow/interfaces/sessions/spaces.ts (3)
1-2: Good type standardization.Importing the generic
IPCListenertype helps standardize event listener patterns across the codebase, making the API more consistent and easier to understand.
62-62: Type improvement for event listener.The change to use the generic
IPCListener<[void]>type standardizes the event listener pattern. The static analysis warning about usingvoidin a tuple can be ignored, as this is a common TypeScript pattern for callbacks that don't receive arguments.🧰 Tools
🪛 Biome (1.9.4)
[error] 62-62: void is confusing outside a return type or a type parameter.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
67-67: Consistent type pattern application.This change follows the same pattern as line 62, applying the generic
IPCListenertype consistently throughout the interface.vite/src/components/providers/spaces-provider.tsx (4)
5-5: Good window type awareness addition.Adding the
WindowTypeprop to theSpacesProvidercomponent enables proper handling of different window contexts, which is essential for extension popups to behave correctly.Also applies to: 27-27, 31-31
67-69: Proper handling of popup windows.This check correctly prevents space switching in popup windows when a current space is already set, which is important for maintaining the expected context for extension popups.
74-74: Optimization to prevent redundant space switches.This check avoids unnecessary space switching operations when the requested space is already current, which is a good performance optimization.
83-83: Correctly updated dependency array.The dependency array has been properly updated to include the new
windowTypedependency, ensuring the callback is regenerated when the window type changes.electron/modules/extensions/locales.ts (5)
1-12: Well-structured type definition and imports.The
LocaleDatatype correctly represents Chrome extension message format, and the imports are appropriate for file system operations and extension management functions.
13-19: Good use of utility functions for path construction.These utility functions follow the standard Chrome extension directory structure for locales, making the code more maintainable.
22-31: Robust implementation of locale directory checking.The
hasLocalesfunction includes proper error handling and follows async patterns correctly. The implementation safely handles the case where locales might not exist.
33-45: Proper error handling in locale data retrieval.The
getLocaleDatafunction correctly handles file reading errors and returns null in case of failures, providing robust error recovery.
47-59: Well-implemented locale discovery function.The
getAllLocalesfunction correctly handles the directory reading operation and returns an empty array as a fallback when errors occur.electron/browser/window.ts (5)
14-14: Good type definition for browser window types.The
BrowserWindowTypetype clearly defines the supported window types.
47-49: Appropriate window size constraints based on window type.Setting different minimum dimensions based on window type is a good approach - normal windows need more space than popup windows.
78-84: Improved window sizing logic.Good improvement to only maximize the window when no explicit size or position options are provided, respecting user-provided dimensions.
130-138: Proper cleanup logic for popup windows.The auto-destruction of popup windows when they have no tabs left is appropriate behavior and prevents leaving empty popup windows around.
201-201: Good defensive programming to check for destroyed web contents.Checking if web contents are destroyed before sending messages prevents potential errors when trying to access destroyed objects.
vite/src/lib/flow/interfaces/app/extensions.ts (2)
1-3: Appropriate imports for extension API interface.The imports are correctly set up to reference the necessary types for IPC communication and extension data.
4-15: Well-documented extension management API.The interface methods and event listener for retrieving and monitoring extensions are clearly documented with JSDoc comments.
electron/ipc/app/extensions.ts (7)
16-23: Good implementation of manifest string translation.The
translateManifestStringfunction correctly extracts message keys using regex and passes them to the localization system.
71-88: Well-structured function to retrieve extension data.The
getExtensionDataFromProfilefunction properly handles all the steps needed to retrieve extension data with appropriate error checking.
90-103: Good implementation of profile ID retrieval.The
getCurrentProfileIdFromWebContentsfunction correctly navigates from WebContents to window to space to profile ID with proper null checking at each step.
105-113: Well-structured IPC handler for retrieving extensions.The handler correctly gets the current profile ID and returns extension data with proper error handling.
115-131: Good implementation of extension enable/disable functionality.The handler properly checks for browser, profile, and extensions manager existence before performing the operation.
174-190: Consistent implementation of extension pinning functionality.The handler follows the same pattern as the other IPC handlers, with proper error checking at each step.
192-197: Good implementation of extension update notification.The
fireOnExtensionsUpdatedfunction properly retrieves updated extension data and broadcasts it to listeners.electron/browser/tabs/tab-context-menu.ts (2)
43-43: Updated function call to include the tab and parameters.The extension menu items are now dynamically retrieved based on the tab's profile and context parameters, properly integrating with the new extension system.
66-67: Improved conditional logic for image items.This change refines the context menu behavior by only showing image-specific actions when there's no link and no text selection but there is an image. This provides a more intuitive user experience based on the current context.
electron/modules/extensions/main.ts (3)
5-22: Well-designed custom session grabber for profile-based extensions.The implementation elegantly intercepts partitions with "profile:" prefix to retrieve sessions tied to user profiles, maintaining a clean separation between profile-specific and general partition handling.
25-57: Robust protocol handler implementation for Chrome extensions.The crx protocol handler is well-implemented with appropriate error handling for missing parameters, sessions, and extensions. The handler delegates to the electron-chrome-extensions library which completes the integration.
27-28: 🛠️ Refactor suggestionAddress TypeScript error rather than using ts-ignore comment.
The code uses a TypeScript ignore comment for the URL.parse operation.
Instead of suppressing the TypeScript error, fix it by using the URL constructor which is the modern approach:
- // @ts-ignore: URL.parse should work, but tsc thinks it doesn't - const url = URL.parse(request.url); + const url = new URL(request.url);⛔ Skipped due to learnings
Learnt from: iamEvanYT PR: MultiboxLabs/flow-browser#24 File: electron/browser/tabs/tab.ts:309-310 Timestamp: 2025-04-18T23:20:02.623Z Learning: In the Flow Browser codebase, `URL.parse` is preferred over `new URL()` for URL parsing because it works without errors in the Electron runtime environment, even though TypeScript's type definitions require `@ts-ignore` comments.vite/src/routes/extensions/components/extension-card.tsx (2)
37-47: Well-implemented extension uninstall with proper state management.The uninstall handler correctly manages the removing state, provides user feedback via toast notification, and handles the async flow properly.
49-83: Well-designed extension card UI with proper animation and responsive layout.The component follows good React practices with clear separation of UI and behavior. The animation effects provide a polished user experience, and the layout adapts well to different content lengths.
electron/browser/tabs/tab-manager.ts (3)
96-98: Flexibility improvement by making windowId parameter optional.Making windowId optional improves the API ergonomics and is a good developer experience enhancement.
107-119: Robust windowId fallback mechanism with proper error handling.The implementation correctly falls back to focused window, then to the first available window, and finally throws a meaningful error if no window is found. This provides a robust solution for determining the appropriate window ID.
190-191: Passing loadedProfile to Tab constructor enables extension integration.Adding the loadedProfile to the Tab constructor is essential for the extension integration, allowing tabs to access extension functionality from their associated profile.
docs/api/extensions/permission-warnings.md (1)
1-74: Well-structured and comprehensive documentationThis documentation is well-organized and provides clear explanations of the permission warnings module, its API, usage examples, and implementation details. The inclusion of Chromium references and explicit limitations is particularly valuable.
vite/src/App.tsx (3)
11-12: Good architecture for supporting different UI typesThe separation of
MainUIRouteandPopupUIRoutewith appropriate protocol designations is a good architectural decision that will help with managing different UI contexts.Also applies to: 28-33
61-63: Good integration of extensions page into routingThe addition of the extensions route under the
flow:protocol is properly integrated into the application's routing structure.
55-57:Details
❓ Verification inconclusive
Potential issue with duplicate hostname
Both routes at lines 52-54 and 55-57 use the hostname "omnibox" but with different protocols. Verify that this is intentional and won't cause conflicts.
🏁 Script executed:
#!/bin/bash # Check if there are any other routes with the same hostname but different protocols echo "Checking for potential routing conflicts with 'omnibox' hostname:" rg -A 1 -B 1 "hostname=\"omnibox\"" --type=tsx --type=jsxLength of output: 232
#!/bin/bash echo "Searching for hostname=\"omnibox\" occurrences in TSX/JSX files:" rg -n 'hostname="omnibox"' -C2 -g '*.tsx' -g '*.jsx'
#!/usr/bin/env bash echo "Inspecting vite/src/App.tsx around the Route declarations:" rg -n -C3 '<Route' vite/src/App.tsx echo echo "Searching project-wide for any <Route> using hostname=\"omnibox\":" rg -n -C2 '<Route[^>]*hostname="omnibox"' -g '*.tsx' -g '*.jsx'
Confirm routing key uniqueness for “omnibox” hostname
It appears you have two
<Route>entries invite/src/App.tsxusing the samehostname="omnibox"but differentprotocolprops. Verify that your router implementation treats the combination ofprotocolandhostnameas a unique key—otherwise one route may override the other or cause unexpected behavior.• File: vite/src/App.tsx
Lines: 52–57If this is intentional, consider adding a comment or updating the
Routecomponent to make the dual‐protocol use explicit and maintainable.shared/types/extensions.ts (1)
5-19: Well-structured extension data interfaceThe
SharedExtensionDatainterface is comprehensive and covers all the necessary properties for extension management. Good job making most properties mandatory while keeping optional ones likeshort_nameanddescriptionappropriately marked.vite/src/components/browser-ui/main.tsx (9)
12-13: Good addition of extension-related providersThe addition of
BrowserActionProviderandExtensionsProviderWithSpacesis well-structured and follows the existing pattern of context providers in the app.
19-19: Well-defined WindowType enumThe
WindowTypetype with values "main" and "popup" is clear and properly exported. This will help maintain consistency across the application when dealing with different window types.
21-21: Component signature update with WindowTypeGood update to the component signature to include the
WindowType. This allows the UI to conditionally render based on the window context.
51-51: Clear conditional for sidebar renderingThe
hasSidebarvariable provides a clear, semantic way to determine when the sidebar should be shown based on window type.
56-56: Conditional sidebar renderingGood implementation of conditional sidebar rendering using the
hasSidebarvariable. This ensures the sidebar is only shown for main windows and not for popup windows.
61-62: Window type-specific stylingWell-implemented conditional styling for window types. This ensures proper spacing and padding depending on whether it's a main window with sidebar or a popup window.
101-101: Updated BrowserUI component signatureGood update to the
BrowserUIcomponent signature to support the window type parameter. This ensures proper UI rendering based on context.
122-122: Space provider window type awarenessThe
SpacesProvidernow correctly receives the window type, which allows for context-aware space management depending on whether it's a main window or popup.
124-128: Well-structured provider nestingThe nesting order of providers is logical and maintains proper dependency flow.
BrowserActionProviderandExtensionsProviderWithSpaceswrap theInternalBrowserUIcomponent, ensuring all extension-related contexts are available within the UI.vite/src/routes/extensions/components/extension-details.tsx (2)
6-13: Well-designed component props interfaceThe
ExtensionDetailsPropsinterface is well-structured with clear type definitions. The inclusion of theisProcessingstate to disable UI elements during asynchronous operations is a good practice.
38-44: Good implementation of extension toggleThe implementation of the enable/disable switch is clean and properly handles the async state with the
isProcessingflag to prevent multiple concurrent actions.electron/modules/utils.ts (4)
6-10: Good JSDoc documentationExcellent addition of JSDoc comments for all utility functions. The documentation clearly describes each function's purpose, parameters, and return values, which improves code maintainability and helps other developers understand how to use these utilities.
Also applies to: 18-22, 27-31, 36-40, 45-49
32-34: Simple and effective Promise-based delay functionThe
waitForfunction is a clean implementation of a Promise-based delay that will be useful for various asynchronous operations throughout the application.
41-43: Robust error handling in getFsStatGood implementation of error handling in
getFsStatby catching and returning null instead of throwing an exception. This makes the function more robust when dealing with non-existent paths.
50-67:Details
✅ Verification successful
Potentially costly recursive size calculation
The
getActualSizefunction recursively calculates directory sizes, which could be expensive for large directories. For better performance with large extensions:
- Consider adding a progress callback parameter.
- Implement a more efficient algorithm using streaming or worker threads for very large extensions.
- Cache results where appropriate.
Also, there's a potential for excessive stack usage with deeply nested directories.
🏁 Script executed:
#!/bin/bash # Check for potentially large extensions in the wild curl -s https://chrome.google.com/webstore/detail/ublock-origin/cjpalhdlnbpafiamejdnhcphjbkeiagm | grep -o "Size: [0-9.]* [KMG]B" || echo "Size information not found"Length of output: 194
No action required:
getActualSizerecursion is acceptable for now
The function correctly handles files, directories and other file system objects. While adding a progress callback, streaming/worker-based traversal or caching could help in extreme edge cases, there’s no evidence that directory depths or sizes in our target workloads will trigger stack overflows or unacceptable performance. These optimizations can be revisited later if profiling identifies a real bottleneck.vite/src/components/providers/extensions-provider.tsx (4)
5-8: Well-defined context interfaceThe
ExtensionsContextValueinterface clearly defines the shape of the context data with a list of extensions and a revalidation method. This makes the API contract explicit and easy to understand.
12-18: Robust context hook implementationThe
useExtensionshook follows best practices by:
- Checking if the context exists
- Throwing a clear error message if used outside the provider
- Returning the properly typed context value
This ensures that consumers of the hook will get appropriate feedback if used incorrectly.
53-64: Good subscription management with cleanupThe code properly sets up a subscription to extension updates and correctly unsubscribes on component unmount. The check to ensure updates only apply to the current profile prevents unnecessary state updates.
78-81: Efficient integration with SpacesProviderThe
ExtensionsProviderWithSpacescomponent efficiently integrates with the existing spaces context to provide space-aware extension data. This is a clean pattern for composing context providers together.vite/src/components/browser-ui/sidebar/header/address-bar/pinned-browser-actions.tsx (3)
14-16: Efficient filtering of pinned extensions.Good implementation of filtering and limiting the pinned extensions to display. The approach is clean and efficient.
94-105: Well-implemented badge display.The badge implementation is well done with good styling and positioning. The conditional rendering and custom color support are properly implemented.
69-75:Details
❓ Verification inconclusive
Verify the icon URL protocol handling.
The URL uses
crx://extension-icon/protocol which must be registered correctly in the electron protocol handler.
🏁 Script executed:
#!/bin/bash # Check if the crx:// protocol handler is properly registered grep -r "registerFileProtocol('crx:" --include="*.ts" --include="*.js" .Length of output: 77
🏁 Script executed:
#!/bin/bash # Search for any registration of the 'crx' protocol and occurrences of 'crx://' in code grep -R "registerFileProtocol" -n --include="*.ts" --include="*.js" . grep -R "protocol.register" -n --include="*.ts" --include="*.js" . grep -R "crx://" -n .Length of output: 613
Ensure
crx://extension-iconIs Registered in Electron
We found two usages of thecrx://extension-icon/…URL in your renderer code but didn’t locate any correspondingprotocol.register*orregisterFileProtocol('crx', …)calls in the codebase:• vite/src/components/browser-ui/sidebar/header/address-bar/pinned-browser-actions.tsx:74
• vite/src/components/browser-ui/browser-action.tsx:50Please verify (in your main-process or initialization code) that the
crxprotocol is properly registered with Electron’sprotocolAPI so these URLs resolve as expected.vite/src/lib/flow/flow.ts (4)
1-4: Well-organized imports with clear categorization.The imports are well-organized and properly grouped by functionality, making the code more maintainable.
27-31: Clean API organization with clear categories.Organizing the Flow API into clear categories (App APIs, Browser APIs, Settings APIs) improves code readability and maintainability.
45-49: Consistent API grouping under Settings.The grouping of settings-related APIs under a dedicated section improves code organization.
38-39:Details
✅ Verification successful
Verify omnibox and newTab implementations.
Ensure that these new Browser APIs have proper implementation on the backend side.
🏁 Script executed:
#!/bin/bash # Check for omnibox and newTab implementations grep -r "omnibox" --include="*.ts" electron/ grep -r "newTab" --include="*.ts" electron/Length of output: 7577
🏁 Script executed:
#!/bin/bash # Display full contents of new-tab IPC handler to verify registration sed -n '1,200p' electron/ipc/app/new-tab.tsLength of output: 1415
Confirmed backend support for omnibox and newTab
Both Flow APIs have matching implementations in the Electron codebase:
- Omnibox managed in electron/browser/components/omnibox.ts and exposed via IPC in electron/ipc/window/omnibox.ts
- newTab handled in electron/ipc/app/new-tab.ts (using tabManager under the hood, with fall-back to Flow’s omnibox mode)
No further backend work required.
electron/ipc/listeners-manager.ts (4)
1-3: Clear documentation of module purpose.Good documentation explaining the purpose of the module and the problem it solves.
7-9: Well-designed data structure for listener management.Using a nested Map structure is an efficient way to organize listeners by channel and ID.
33-43: Efficient message broadcasting to active listeners.The implementation ensures messages are only sent to active, non-destroyed WebContents, avoiding unnecessary IPC communication.
49-51: Good defensive programming.Checking if the window is destroyed before accessing its properties prevents potential errors.
vite/src/components/providers/browser-action-provider.tsx (2)
5-33: Well-defined type interfaces.The interfaces are well-structured and provide clear typing for the browser action components, improving code readability and type safety.
147-161: Good integration with other providers.The
BrowserActionProviderproperly integrates with spaces and tabs providers to determine the current profile and focused tab.electron/browser/tabs/tab.ts (1)
786-791: Race-condition: ensureselectTabisn’t called after tab destruction
updateLayoutcallsextensions.selectTabwhenjustShownis true. If the tab becomes visible afterdestroy()has started (rare, but possible with async timing),selectTabcould be invoked on a destroyedWebContents.Recommend short-circuiting when
this.isDestroyedorthis.webContents.isDestroyed().- if (justShown) { - extensions.selectTab(this.webContents); - } + if (justShown && !this.isDestroyed && !this.webContents.isDestroyed()) { + extensions.selectTab(this.webContents); + }
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
electron/browser/tabs/tab.ts (1)
925-928:⚠️ Potential issueTab gets added to the extensions system but is never removed
You correctly call
extensions.addTaband hooktabUpdated/selectTab, yetdestroy()only comments out the matchingremoveTabcall. LeakingWebContentsreferences prevents GC and keeps stale tab data in the extensions manager.- // Should be automatically removed when the webContents is destroyed - // const extensions = this.loadedProfile.extensions; - // extensions.removeTab(this.webContents); + const extensions = this.loadedProfile.extensions; + // Remove tab from the extensions system to allow cleanup + extensions.removeTab(this.webContents);If automatic cleanup is indeed guaranteed elsewhere, please reference that path explicitly in a comment so readers don't assume a leak.
🧹 Nitpick comments (1)
electron/modules/extensions/management.ts (1)
273-281: Consider more detailed error handling for service worker startupThe current implementation silently catches and logs service worker startup failures without any recovery mechanism.
if (extension.manifest.manifest_version === 3 && extension.manifest.background?.service_worker) { const scope = `chrome-extension://${extension.id}`; - await session.serviceWorkers.startWorkerForScope(scope).catch(() => { - console.error(`Failed to start worker for extension ${extension.id}`); - }); + try { + await session.serviceWorkers.startWorkerForScope(scope); + } catch (error) { + console.error(`Failed to start worker for extension ${extension.id}:`, error); + // Consider tracking failed extensions or implementing a retry mechanism + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
electron/browser/tabs/tab-context-menu.ts(3 hunks)electron/browser/tabs/tab.ts(10 hunks)electron/browser/utility/protocols.ts(5 hunks)electron/ipc/listeners-manager.ts(1 hunks)electron/modules/extensions/management.ts(1 hunks)electron/modules/typed-event-emitter.ts(1 hunks)vite/src/components/browser-ui/browser-action.tsx(1 hunks)vite/src/routes/extensions/page.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- vite/src/routes/extensions/page.tsx
- electron/ipc/listeners-manager.ts
- vite/src/components/browser-ui/browser-action.tsx
- electron/modules/typed-event-emitter.ts
- electron/browser/utility/protocols.ts
- electron/browser/tabs/tab-context-menu.ts
🧰 Additional context used
🧬 Code Graph Analysis (2)
electron/browser/tabs/tab.ts (2)
electron/browser/profile-manager.ts (1)
LoadedProfile(20-28)electron/saving/tabs.ts (1)
removeTabFromStorage(38-41)
electron/modules/extensions/management.ts (5)
shared/types/extensions.ts (1)
ExtensionType(3-3)electron/saving/datastore.ts (2)
getDatastore(266-280)DataStore(253-253)electron/modules/utils.ts (2)
getFsStat(41-43)getActualSize(50-67)electron/modules/typed-event-emitter.ts (1)
TypedEventEmitter(8-111)electron/ipc/app/extensions.ts (1)
fireOnExtensionsUpdated(192-197)
🔇 Additional comments (4)
electron/modules/extensions/management.ts (1)
92-144: Good implementation of icon retrieval with robust error handlingThe function correctly handles icon selection preference (128px preferred, falling back to largest available), with thorough error handling for file access, validation, and image creation. Nice defensive programming!
electron/browser/tabs/tab.ts (3)
217-226: Good asynchronous history restoration with deferred sleepThe improved tab restoration now correctly defers putting the tab to sleep until after the navigation history has been restored, which prevents potential race conditions.
280-292: Extension integration properly handles tab creation and updatesThe tab correctly registers itself with the extension system and notifies it of updates, enhancing extension functionality with browser tabs.
787-790: Great implementation of extension tab selection notificationProperly notifies the extension system when a tab becomes visible, ensuring extensions can respond to tab focus events.
1 participant
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Documentation
Chores