src/renderer/src/routes/error/route.tsx (1)

1-3: Same performance concern as other route files.

This file has the same lazy loading removal pattern as other route files. The performance and Suspense redundancy concerns mentioned in the omnibox route review apply here as well.

src/renderer/src/routes/pdf-viewer/route.tsx (2)

1-3: Consistent lazy loading removal pattern.

The changes match the pattern seen across all route files - removing React.lazy in favor of static imports.

---

8-10: Redundant Suspense wrapper (duplicate concern).

The Suspense wrapper is no longer needed with static imports.

src/renderer/src/routes/main-ui/route.tsx (1)

8-10: Redundant Suspense wrapper (consistent across routes).

Same concern as other route files - the Suspense wrapper is now unnecessary.

src/renderer/src/routes/extensions/route.tsx (1)

8-10: Redundant Suspense wrapper (final occurrence).

As with all other route files, the Suspense wrapper is now redundant with static imports.

src/renderer/src/routes/games/route.tsx (1)

1-3: Redundant Suspense wrapper after removing lazy loading.

Same issue as other route files - the Suspense component is no longer needed since PageComponent is now statically imported.

Consider removing the Suspense wrapper or document why it's still required (e.g., for nested lazy components).

src/renderer/src/routes/omnibox-debug/route.tsx (1)

1-3: Architectural consistency maintained but Suspense is redundant.

The static import change is consistent across all route files, indicating a deliberate architectural shift away from code-splitting. However, the Suspense wrapper remains unnecessary.

Consider a global refactor to remove Suspense wrappers from all route files that no longer use lazy loading.

src/renderer/src/routes/about/route.tsx (1)

1-3: Performance impact of removing code-splitting should be evaluated.

While the change is architecturally consistent, removing lazy loading from all routes will increase the initial bundle size. This could impact application startup time, especially for routes with heavy components.

Consider measuring the bundle size impact and initial load performance before and after this change:

#!/bin/bash
# Description: Check bundle size impact of removing lazy loading

# Find webpack bundle analyzer or similar tool configuration
fd -e json package.json --exec cat {} \; | jq '.scripts | to_entries[] | select(.value | contains("bundle-analyzer") or contains("webpack-bundle-analyzer"))'

# Look for bundle size configuration
fd -e js -e ts webpack.config babel.config vite.config --exec head -20 {}

src/renderer/src/routes/new-tab/route.tsx (1)

1-3: Systematic removal of code-splitting requires architectural review.

This change pattern across all route files represents a significant architectural shift. For the new-tab route specifically, this could impact browser startup performance since new tabs are frequently opened.

Consider:

1. Selective lazy loading: Keep code-splitting for heavy/rarely-used routes while removing it for frequently accessed ones
2. Bundle splitting strategies: Use webpack's splitChunks to create shared chunks for common dependencies
3. Progressive loading: Implement component-level lazy loading within routes instead of route-level

The introduction of the PortalsProvider (mentioned in the summary) suggests this change might be necessary for the new portal system. If so, document this architectural decision and its trade-offs.

src/renderer/src/components/portal/portal_old.tsx (2)

67-67: Invalid window features format (same issue as in provider.tsx).

The window features parameter has the same format issue as in the provider file.

---

125-125: Undefined flow object (same issue as in provider.tsx).

Multiple references to undefined flow object that will cause runtime errors.

Also applies to: 137-137, 143-143

src/renderer/src/lib/merge-refs.ts (1)

1-11: Improve type safety in the ref assignment.

The type assertion on line 7 assumes the ref is a MutableRefObject without verification. Consider using a type guard for safer handling.

export function mergeRefs<T extends HTMLElement>(refs: Array<React.Ref<T> | undefined>): React.RefCallback<T> {
  return (value) => {
    for (const ref of refs) {
      if (typeof ref === "function") {
        ref(value);
-     } else if (ref != null) {
+     } else if (ref != null && typeof ref === "object" && "current" in ref) {
        (ref as React.MutableRefObject<T | null>).current = value;
      }
    }
  };
}

src/renderer/src/routes/onboarding/route.tsx (1)

8-10: Consider removing redundant Suspense wrapper.

Since PageComponent is now imported statically, the Suspense wrapper no longer serves its intended purpose of handling loading states during async component loading.

-      <Suspense fallback={RouteConfig.Fallback}>
-        <PageComponent />
-      </Suspense>
+      <PageComponent />

src/renderer/src/routes/popup-ui/route.tsx (1)

1-3: Consider removing redundant Suspense wrapper after eliminating lazy loading.

The removal of lazy import and switch to static import eliminates code-splitting for this route. However, the Suspense component is now redundant since PageComponent loads synchronously.

Consider simplifying the component structure:

-import { Suspense } from "react";
 import { RouteConfig } from "./config";
 import PageComponent from "./page";

 export default function Route() {
   return (
     <RouteConfig.Providers>
-      <Suspense fallback={RouteConfig.Fallback}>
         <PageComponent />
-      </Suspense>
     </RouteConfig.Providers>
   );
 }

Alternatively, if Suspense is still needed for nested lazy components or streaming SSR, consider adding a comment explaining its purpose.

src/renderer/src/routes/settings/route.tsx (1)

1-3: Consider the performance implications of removing lazy loading.

Removing React.lazy eliminates code-splitting for route components, which will increase the initial bundle size. Additionally, the Suspense wrapper on lines 8-10 is now redundant since there's no asynchronous component loading.

If this change is intentional as part of the portal refactor, consider removing the unnecessary Suspense wrapper to simplify the code.

-import { Suspense } from "react";
 import { RouteConfig } from "./config";
 import PageComponent from "./page";

 export default function Route() {
   return (
     <RouteConfig.Providers>
-      <Suspense fallback={RouteConfig.Fallback}>
-        <PageComponent />
-      </Suspense>
+      <PageComponent />
     </RouteConfig.Providers>
   );
 }

Also applies to: 8-10

src/renderer/src/components/portal/portal.tsx (1)

128-131: Use optional chaining for safer portal access.

The static analysis tool correctly identified that optional chaining would be safer here.

-  const wrapper =
-    portal &&
-    portal.window &&
-    !portal.window.closed &&
-    createPortal(portalChildren, portal.window.document.body, "portal-wrapper");
+  const wrapper =
+    portal?.window &&
+    !portal.window.closed &&
+    createPortal(portalChildren, portal.window.document.body, "portal-wrapper");

src/renderer/src/components/portal/provider.tsx (1)

29-40: Potential hoisting dependency in hot module reload cleanup.

The cleanup function calls removePortal before it's defined in the file. While JavaScript function hoisting makes this work, it creates a fragile dependency on declaration order.

Consider moving the utility functions before the hot module reload section or extracting the cleanup logic:

+const cleanupPortal = (portal: Portal) => {
+  portal.window.close();
+};
+
 if (import.meta.hot) {
   const cleanup = () => {
     if (window.portals) {
       // Close all available portals
       for (const portal of window.portals.available.values()) {
-        removePortal(portal);
+        cleanupPortal(portal);
+        window.portals.available.delete(portal.id);
       }
       // Close all used portals
       for (const portal of window.portals.used.values()) {
-        removePortal(portal);
+        cleanupPortal(portal);
+        window.portals.used.delete(portal.id);
       }

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro