Skip to content

Conversation

@jonatansberg
Copy link
Member

refs https://linear.app/ghost/issue/BER-3126/generated-bundle-is-huge

  • Added a simple helper to enable code splitting and lazy loading at the route level.
  • Disabled the redundant preloading of settings in Ember

This reduces the amount of JS we are serving to just render the shell from ~12mb to ~5.4mb pre compression. The minified pre-zip size is in many ways a more relevant metric than the gzip size since the former correlates better with actual JS parse and execution time.

Before:
Screenshot 2025-12-10 at 15 45 05

After:
Screenshot 2025-12-10 at 15 39 46

There is plenty of room to iterate on this further by pulling out some of our larger dependencies in favor of smaller ones.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 10, 2025

Walkthrough

The PR adds a new lazy-loading helper lazyComponent in admin-x-framework and re-exports it. Multiple apps (activitypub, admin, posts, stats) switch route definitions from immediate element renders to lazy-loaded routes using lazy or lazyComponent. activitypub introduces a CustomRouteObject type. The Settings component in apps/admin is changed to a default export. A guard prevents preloading AdminXSettings when this.feature.inAdminForward is true. Tests add a mock implementation for lazyComponent.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20–30 minutes

  • Focus review on dynamic-import correctness and default-export expectations for all lazy-loaded modules (apps/admin settings, posts analytics children, tags, stats views, activitypub onboarding steps).
  • Verify the composed async lazy loader in apps/posts correctly instantiates PostAnalyticsProvider and preserves props/error handling.
  • Confirm lazyComponent utility signature aligns with the router lazy contract and its re-export from admin-x-framework.
  • Check activitypub CustomRouteObject type compatibility with existing consumers and root errorElement behavior after route conversions.
  • Inspect updated test mock in apps/stats for parity with runtime lazyComponent behaviour.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Added route lazy loading' accurately and concisely summarizes the main change of implementing lazy loading for routes across multiple applications in the codebase.
Description check ✅ Passed The description clearly relates to the changeset by explaining the purpose (code splitting and lazy loading), the specific implementation details (helper function, disabled preloading), and quantifiable results (JS bundle size reduction).
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch ber-3126-generated-bundle-is-huge

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (2)
apps/admin-x-framework/src/utils/lazy-component.ts (1)

1-7: lazyComponent implementation matches React Router lazy contract

The helper correctly wraps a dynamic import and returns () => Promise<{Component: T}>, which is what React Router expects for route-level lazy handlers. Typing with T extends React.ComponentType is reasonable for route components.

If you want to make the intent clearer, you could optionally add an explicit return type:

-export function lazyComponent<T extends React.ComponentType>(
-    fn: () => Promise<{ default: T }>
-) {
-    return () => fn().then(({default: Component}) => ({
-        Component
-    }));
-}
+export function lazyComponent<T extends React.ComponentType>(
+    fn: () => Promise<{ default: T }>
+): () => Promise<{Component: T}> {
+    return () =>
+        fn().then(({default: Component}) => ({
+            Component
+        }));
+}
apps/activitypub/src/routes.tsx (1)

3-139: Lazy-loading pattern across ActivityPub routes looks sound

The switch from static element definitions to lazy: lazyComponent(() => import(...)) for reader, notes, notifications, explore, profile, preferences, onboarding steps, and the catch‑all error route matches the framework helper’s contract and React Router’s lazy API. The root error boundary remains a static errorElement, which is appropriate.

You could optionally DRY up repeated imports (for example, ./views/profile and ./views/inbox used in multiple routes) by assigning them to variables:

const lazyProfile = lazyComponent(() => import('./views/profile'));
const lazyInbox = lazyComponent(() => import('./views/inbox'));

// then reuse lazyProfile / lazyInbox in the relevant routes

Not required, but it would slightly reduce repetition.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9b4ed74 and 1fa4b96.

📒 Files selected for processing (7)
  • apps/activitypub/src/routes.tsx (4 hunks)
  • apps/admin-x-framework/src/index.ts (1 hunks)
  • apps/admin-x-framework/src/utils/lazy-component.ts (1 hunks)
  • apps/admin/src/routes.tsx (2 hunks)
  • apps/admin/src/settings/settings.tsx (1 hunks)
  • apps/posts/src/routes.tsx (3 hunks)
  • apps/stats/src/routes.tsx (2 hunks)
🧰 Additional context used
🧠 Learnings (24)
📓 Common learnings
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Ember admin uses `AdminXComponent` to dynamically import React apps with Suspense and error boundaries
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Use React with Vite for Admin app development (admin-x-settings, admin-x-activitypub, posts, stats)
Learnt from: sagzy
Repo: TryGhost/Ghost PR: 24346
File: apps/admin-x-settings/src/components/settings/growth/Network.tsx:8-12
Timestamp: 2025-07-14T12:20:35.268Z
Learning: The Network component toggle in `apps/admin-x-settings/src/components/settings/growth/Network.tsx` is intentionally implemented as static UI with a no-op onChange handler, as part of a UI-first development approach before connecting actual ActivityPub functionality.
📚 Learning: 2025-11-26T11:05:59.314Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: apps/shade/AGENTS.md:0-0
Timestamp: 2025-11-26T11:05:59.314Z
Learning: Applies to apps/shade/src/index.ts : Place new UI components under `src/components/ui` and export them from `src/index.ts`

Applied to files:

  • apps/admin-x-framework/src/utils/lazy-component.ts
  • apps/admin-x-framework/src/index.ts
📚 Learning: 2025-11-25T14:28:50.351Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Ember admin uses `AdminXComponent` to dynamically import React apps with Suspense and error boundaries

Applied to files:

  • apps/admin-x-framework/src/utils/lazy-component.ts
  • apps/stats/src/routes.tsx
  • apps/posts/src/routes.tsx
  • apps/admin/src/routes.tsx
  • apps/activitypub/src/routes.tsx
📚 Learning: 2025-11-25T14:28:50.351Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Use React with Vite for Admin app development (admin-x-settings, admin-x-activitypub, posts, stats)

Applied to files:

  • apps/stats/src/routes.tsx
  • apps/posts/src/routes.tsx
  • apps/admin/src/settings/settings.tsx
  • apps/admin/src/routes.tsx
  • apps/activitypub/src/routes.tsx
📚 Learning: 2025-11-25T14:28:50.351Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Admin-x React apps build to `apps/*/dist` using Vite, which are then copied by `ghost/admin/lib/asset-delivery` to `ghost/core/core/built/admin/assets/*`

Applied to files:

  • apps/stats/src/routes.tsx
  • apps/posts/src/routes.tsx
  • apps/activitypub/src/routes.tsx
📚 Learning: 2025-11-25T14:28:50.351Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: When working on Admin UI features, build in React using `apps/admin-x-*` or `apps/posts` apps

Applied to files:

  • apps/stats/src/routes.tsx
  • apps/posts/src/routes.tsx
  • apps/activitypub/src/routes.tsx
📚 Learning: 2025-11-25T14:28:50.351Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Ghost admin serves admin-x apps from `/ghost/assets/{app-name}/{app-name}.js` URLs

Applied to files:

  • apps/stats/src/routes.tsx
📚 Learning: 2025-08-11T14:18:31.752Z
Learnt from: niranjan-uma-shankar
Repo: TryGhost/Ghost PR: 24626
File: apps/portal/src/components/pages/AccountHomePage/components/AccountFooter.js:15-15
Timestamp: 2025-08-11T14:18:31.752Z
Learning: In Ghost, Portal components render within iframes while admin components (like gh-member-details.hbs, dashboard/charts/recents.hbs, and posts/post-activity-feed.hbs) do not render in iframes. Therefore, iframe-related navigation issues only affect Portal components.

Applied to files:

  • apps/stats/src/routes.tsx
  • apps/posts/src/routes.tsx
  • apps/activitypub/src/routes.tsx
📚 Learning: 2025-11-03T12:33:31.093Z
Learnt from: kevinansfield
Repo: TryGhost/Ghost PR: 25320
File: apps/admin/src/layout/app-sidebar/NavHeader.tsx:10-10
Timestamp: 2025-11-03T12:33:31.093Z
Learning: The Ghost admin apps (apps/admin/**) do not use SSR, so accessing browser APIs like `navigator`, `window`, or `document` at module load time is safe and does not require typeof guards.

Applied to files:

  • apps/stats/src/routes.tsx
  • apps/posts/src/routes.tsx
  • apps/admin/src/routes.tsx
  • apps/activitypub/src/routes.tsx
📚 Learning: 2025-08-11T14:18:31.752Z
Learnt from: niranjan-uma-shankar
Repo: TryGhost/Ghost PR: 24626
File: apps/portal/src/components/pages/AccountHomePage/components/AccountFooter.js:15-15
Timestamp: 2025-08-11T14:18:31.752Z
Learning: The /dashboard route in Ghost admin was retired in version 6.0.

Applied to files:

  • apps/stats/src/routes.tsx
📚 Learning: 2025-11-12T22:26:48.725Z
Learnt from: troyciesco
Repo: TryGhost/Ghost PR: 25271
File: apps/stats/src/views/Stats/Growth/Growth.tsx:199-204
Timestamp: 2025-11-12T22:26:48.725Z
Learning: In apps/stats/src/views/Stats/Growth/Growth.tsx, the loading skeleton for the top posts/pages table intentionally uses colSpan={1} to only span the first (wider) column, creating a visual effect where the skeleton appears only in the main content column (which contains post/page titles with dates).

Applied to files:

  • apps/stats/src/routes.tsx
📚 Learning: 2025-12-09T12:37:23.267Z
Learnt from: kevinansfield
Repo: TryGhost/Ghost PR: 25501
File: apps/shade/src/hooks/use-mobile.tsx:5-10
Timestamp: 2025-12-09T12:37:23.267Z
Learning: The Ghost admin apps/shade/** components do not use SSR, so accessing browser APIs like `navigator`, `window`, or `document` at module load time is safe and does not require typeof guards.

Applied to files:

  • apps/stats/src/routes.tsx
  • apps/admin/src/routes.tsx
  • apps/activitypub/src/routes.tsx
📚 Learning: 2025-11-26T11:05:59.314Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: apps/shade/AGENTS.md:0-0
Timestamp: 2025-11-26T11:05:59.314Z
Learning: Applies to apps/shade/src/components/features/**/*.{ts,tsx} : Higher-level, opinionated components (e.g., PostShareModal, SourceTabs) should be placed in `src/components/features/*`

Applied to files:

  • apps/posts/src/routes.tsx
  • apps/activitypub/src/routes.tsx
📚 Learning: 2025-11-25T11:58:51.652Z
Learnt from: ibalosh
Repo: TryGhost/Ghost PR: 25525
File: apps/shade/src/shade-app.tsx:4-4
Timestamp: 2025-11-25T11:58:51.652Z
Learning: In apps/shade, the app wrapper file should be named `src/shade-app.tsx` (kebab-case) while the component itself is exported as `ShadeApp` (PascalCase). Context providers should be placed in `src/providers/*` using kebab-case filenames.

Applied to files:

  • apps/admin/src/settings/settings.tsx
📚 Learning: 2025-07-14T12:20:35.268Z
Learnt from: sagzy
Repo: TryGhost/Ghost PR: 24346
File: apps/admin-x-settings/src/components/settings/growth/Network.tsx:8-12
Timestamp: 2025-07-14T12:20:35.268Z
Learning: The Network component toggle in `apps/admin-x-settings/src/components/settings/growth/Network.tsx` is intentionally implemented as static UI with a no-op onChange handler, as part of a UI-first development approach before connecting actual ActivityPub functionality.

Applied to files:

  • apps/admin/src/routes.tsx
📚 Learning: 2025-09-03T12:28:11.174Z
Learnt from: kevinansfield
Repo: TryGhost/Ghost PR: 24779
File: ghost/core/core/server/services/members/members-api/controllers/RouterController.js:34-51
Timestamp: 2025-09-03T12:28:11.174Z
Learning: The extractRefererOrRedirect function in Ghost's RouterController (ghost/core/core/server/services/members/members-api/controllers/RouterController.js) is working as intended according to maintainer kevinansfield. The current handling of autoRedirect, redirect parameter parsing, and referrer logic does not need the security-related changes suggested around autoRedirect coercion, relative URL handling, or same-origin checks.

Applied to files:

  • apps/admin/src/routes.tsx
📚 Learning: 2025-11-26T11:05:59.314Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: apps/shade/AGENTS.md:0-0
Timestamp: 2025-11-26T11:05:59.314Z
Learning: Applies to apps/shade/src/components/**/*.{ts,tsx} : Prefer compound subcomponents (e.g., `Header.Title`, `Header.Meta`, `Header.Actions`) for multi-region components instead of many props

Applied to files:

  • apps/activitypub/src/routes.tsx
📚 Learning: 2025-03-13T09:02:50.102Z
Learnt from: mike182uk
Repo: TryGhost/Ghost PR: 22471
File: apps/admin-x-activitypub/src/views/Feed/components/NewPostModal.tsx:29-34
Timestamp: 2025-03-13T09:02:50.102Z
Learning: In the Ghost ActivityPub module, error handling for mutations is handled at the hook level (in use-activity-pub-queries.ts) rather than in individual components. This allows for centralized error handling across the application.

Applied to files:

  • apps/activitypub/src/routes.tsx
📚 Learning: 2025-11-26T11:05:59.314Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: apps/shade/AGENTS.md:0-0
Timestamp: 2025-11-26T11:05:59.314Z
Learning: Applies to apps/shade/src/components/layout/**/*.{ts,tsx} : Reusable layout containers (Page, Heading, Header, ViewHeader, ErrorPage) should be placed in `src/components/layout/*`

Applied to files:

  • apps/activitypub/src/routes.tsx
📚 Learning: 2025-11-26T11:05:59.314Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: apps/shade/AGENTS.md:0-0
Timestamp: 2025-11-26T11:05:59.314Z
Learning: Applies to apps/shade/src/**/*.{ts,tsx,js} : Use the `@` alias for internal imports (e.g., `@/lib/utils`)

Applied to files:

  • apps/admin-x-framework/src/index.ts
📚 Learning: 2025-11-26T11:05:59.314Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: apps/shade/AGENTS.md:0-0
Timestamp: 2025-11-26T11:05:59.314Z
Learning: Applies to apps/shade/src/lib/utils.ts : Shared utilities (class merging, formatting, chart helpers) should be centralized in `src/lib/utils.ts`

Applied to files:

  • apps/admin-x-framework/src/index.ts
📚 Learning: 2025-11-05T16:42:12.989Z
Learnt from: rob-ghost
Repo: TryGhost/Ghost PR: 25356
File: apps/admin/test-utils/fixtures/query-client.tsx:17-35
Timestamp: 2025-11-05T16:42:12.989Z
Learning: In apps/admin/test-utils/fixtures/query-client.tsx, the createTestQueryClient function is intentionally duplicated from admin-x-framework to reduce external dependencies in the admin app's test utilities.

Applied to files:

  • apps/admin-x-framework/src/index.ts
📚 Learning: 2025-11-26T11:05:59.314Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: apps/shade/AGENTS.md:0-0
Timestamp: 2025-11-26T11:05:59.314Z
Learning: Applies to apps/shade/src/components/**/*.{ts,tsx} : Use CVA (Class Variance Authority) for component variants when useful instead of heavy prop configuration

Applied to files:

  • apps/admin-x-framework/src/index.ts
📚 Learning: 2025-11-25T14:28:50.351Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Admin apps should use `tanstack/react-query` for data fetching

Applied to files:

  • apps/admin-x-framework/src/index.ts
🧬 Code graph analysis (4)
apps/admin-x-framework/src/utils/lazy-component.ts (1)
apps/admin-x-framework/src/index.ts (1)
  • lazyComponent (43-43)
apps/stats/src/routes.tsx (2)
apps/admin-x-framework/src/index.ts (1)
  • lazyComponent (43-43)
apps/admin-x-framework/src/utils/lazy-component.ts (1)
  • lazyComponent (1-7)
apps/admin/src/routes.tsx (2)
apps/admin-x-framework/src/index.ts (1)
  • lazyComponent (43-43)
apps/admin-x-framework/src/utils/lazy-component.ts (1)
  • lazyComponent (1-7)
apps/activitypub/src/routes.tsx (2)
apps/admin-x-framework/src/index.ts (1)
  • lazyComponent (43-43)
apps/admin-x-framework/src/utils/lazy-component.ts (1)
  • lazyComponent (1-7)
⏰ 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). (5)
  • GitHub Check: Admin-X Settings tests
  • GitHub Check: Unit tests (Node 22.18.0)
  • GitHub Check: ActivityPub tests
  • GitHub Check: Lint
  • GitHub Check: Build & Push Docker Image
🔇 Additional comments (5)
apps/admin/src/settings/settings.tsx (1)

4-22: Default export aligns with lazy import usage

Switching Settings to a default export matches lazyComponent(() => import('./settings/settings')) in the routes config and keeps the portal behavior unchanged. Looks good.

apps/admin-x-framework/src/index.ts (1)

42-43: Public re-export of lazyComponent is appropriate

Re-exporting lazyComponent from the framework index matches how other utilities are surfaced and supports its use across admin, posts, stats, and activitypub.

apps/admin/src/routes.tsx (1)

1-1: Settings route lazy loading is wired correctly

Importing lazyComponent and switching path: 'settings/*' to use lazy: lazyComponent(() => import('./settings/settings')) matches the new default export and should effectively split the settings bundle from the main admin shell. Route structure remains valid and unchanged otherwise.

Also applies to: 58-60

apps/posts/src/routes.tsx (1)

2-52: Post Analytics and Tags lazy loading are correctly implemented

The custom lazy handler for posts/analytics/:postId correctly loads both PostAnalyticsProvider and PostAnalytics in parallel and returns an element, which is supported by React Router’s lazy API. Child analytics routes and the tags index route now use lazyComponent(() => import(...)), matching the new helper and keeping route structure intact.

This should meaningfully defer the Post Analytics and Tags bundles until those routes are visited.

Also applies to: 59-61

apps/stats/src/routes.tsx (1)

1-1: Stats analytics routes now lazily loaded correctly

Switching the Overview, Web, Growth, and Newsletters analytics routes to lazy: lazyComponent(() => import('./views/Stats/...')) is consistent with the new helper and React Router’s expectations. This should reduce the initial stats bundle while preserving the existing route hierarchy.

Also applies to: 16-29

@jonatansberg jonatansberg force-pushed the ber-3126-generated-bundle-is-huge branch from 1fa4b96 to 27a70c9 Compare December 10, 2025 14:58
@kevinansfield
Copy link
Member

kevinansfield commented Dec 10, 2025

Disabled the redundant preloading of settings in Ember

@jonatansberg is this part missing from the PR?

Copy link
Member Author

Yeah, I realized. I pushed that change just now.

Copy link
Member

@kevinansfield kevinansfield left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested with non-dev build locally with throttling turned on. The additional loading states weren't particularly distracting and the reduced initial loading from smaller bundle sizes was noticeable 🚀

refs https://linear.app/ghost/issue/BER-3126/generated-bundle-is-huge

Added a simple helper to enable code splitting and lazy loading at the route 
level.
@jonatansberg jonatansberg force-pushed the ber-3126-generated-bundle-is-huge branch from 27a70c9 to 9272133 Compare December 11, 2025 14:53
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (4)
apps/stats/test/unit/app.test.tsx (1)

17-23: Align lazyComponent mock with real helper’s return shape

The real lazyComponent returns a function () => Promise<{Component}>, but this mock returns the Promise directly. It works with the current test because RouterProvider is a noop wrapper, but it can bite later if tests or code rely on lazy being a function.

You can mirror the real API with:

-    lazyComponent: (fn: () => Promise<{default: React.ComponentType}>) => fn().then(({default: Component}) => ({Component}))
+    lazyComponent: (fn: () => Promise<{default: React.ComponentType}>) =>
+        () => fn().then(({default: Component}) => ({Component}))

This keeps tests closer to production behavior without impacting this test’s assertions.

apps/admin-x-framework/src/utils/lazy-component.ts (1)

1-7: Helper implementation is solid; add explicit React type import

The implementation matches React Router’s lazy API and works well for route-level code splitting.

To avoid relying on a global React namespace for types, consider importing React’s types explicitly:

+import type React from 'react';
+
 export function lazyComponent<T extends React.ComponentType>(
     fn: () => Promise<{ default: T }>
-) {
-    return () => fn().then(({default: Component}) => ({
-        Component
-    }));
-}
+): () => Promise<{Component: T}> {
+    return () => fn().then(({default: Component}) => ({
+        Component
+    }));
+}

This keeps the helper robust against tsconfig/type-definition changes.

apps/activitypub/src/routes.tsx (1)

3-4: Lazy route conversion is sound; consider unifying error component imports

The switch from inline element components to lazy: lazyComponent(() => import(...)) for reader/notes/notifications/explore/profile/preferences/onboarding/error routes looks correct and should give you the expected route-level code splitting without changing route behavior.

One minor optimization: you’re importing the error component both via alias (@components/layout/error for errorElement) and via relative path (./components/layout/error for the path: '*' route). Depending on bundler config, that can produce duplicate chunks for the same module.

If both refer to the same file, consider standardizing on a single import path, for example:

-            {
-                path: '*',
-                lazy: lazyComponent(() => import('./components/layout/error'))
-            }
+            {
+                path: '*',
+                lazy: lazyComponent(() => import('@components/layout/error'))
+            }

(or the inverse), so the error UI is bundled once.

Also applies to: 34-139

apps/posts/src/routes.tsx (1)

20-35: Lazy loading for post analytics and tags routes looks good; optional consistency tweak

The async lazy function for posts/analytics/:postId correctly imports PostAnalyticsProvider and PostAnalytics with Promise.all and returns an element, which is valid for React Router’s lazy API. Child analytics routes and the tags index route use lazyComponent as intended.

If you’d like stricter consistency with other apps, you could factor the provider/wrapper into a small component and use lazyComponent for the parent as well, but that’s purely stylistic—the current approach is clear and functionally solid.

Also applies to: 37-52, 59-61

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 27a70c9 and 9272133.

📒 Files selected for processing (8)
  • apps/activitypub/src/routes.tsx (4 hunks)
  • apps/admin-x-framework/src/index.ts (1 hunks)
  • apps/admin-x-framework/src/utils/lazy-component.ts (1 hunks)
  • apps/admin/src/routes.tsx (2 hunks)
  • apps/admin/src/settings/settings.tsx (1 hunks)
  • apps/posts/src/routes.tsx (3 hunks)
  • apps/stats/src/routes.tsx (2 hunks)
  • apps/stats/test/unit/app.test.tsx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • apps/admin-x-framework/src/index.ts
  • apps/admin/src/settings/settings.tsx
🧰 Additional context used
🧠 Learnings (20)
📓 Common learnings
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Ember admin uses `AdminXComponent` to dynamically import React apps with Suspense and error boundaries
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Use React with Vite for Admin app development (admin-x-settings, admin-x-activitypub, posts, stats)
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: When working on Admin UI features, build in React using `apps/admin-x-*` or `apps/posts` apps
Learnt from: sagzy
Repo: TryGhost/Ghost PR: 24346
File: apps/admin-x-settings/src/components/settings/growth/Network.tsx:8-12
Timestamp: 2025-07-14T12:20:35.268Z
Learning: The Network component toggle in `apps/admin-x-settings/src/components/settings/growth/Network.tsx` is intentionally implemented as static UI with a no-op onChange handler, as part of a UI-first development approach before connecting actual ActivityPub functionality.
Learnt from: mike182uk
Repo: TryGhost/Ghost PR: 22471
File: apps/admin-x-activitypub/src/utils/pending-activity.ts:13-71
Timestamp: 2025-03-13T09:00:20.205Z
Learning: The pending activity utilities in the Ghost ActivityPub module are covered by tests in the file `apps/admin-x-activitypub/test/unit/utils/pending-activity.ts`.
📚 Learning: 2025-11-26T11:05:59.314Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: apps/shade/AGENTS.md:0-0
Timestamp: 2025-11-26T11:05:59.314Z
Learning: Applies to apps/shade/src/index.ts : Place new UI components under `src/components/ui` and export them from `src/index.ts`

Applied to files:

  • apps/admin-x-framework/src/utils/lazy-component.ts
  • apps/stats/test/unit/app.test.tsx
📚 Learning: 2025-11-25T14:28:50.351Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Ember admin uses `AdminXComponent` to dynamically import React apps with Suspense and error boundaries

Applied to files:

  • apps/admin-x-framework/src/utils/lazy-component.ts
  • apps/activitypub/src/routes.tsx
  • apps/stats/src/routes.tsx
  • apps/posts/src/routes.tsx
  • apps/admin/src/routes.tsx
📚 Learning: 2025-11-25T14:28:50.351Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Use React with Vite for Admin app development (admin-x-settings, admin-x-activitypub, posts, stats)

Applied to files:

  • apps/activitypub/src/routes.tsx
  • apps/stats/src/routes.tsx
  • apps/posts/src/routes.tsx
  • apps/admin/src/routes.tsx
📚 Learning: 2025-11-25T14:28:50.351Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Admin-x React apps build to `apps/*/dist` using Vite, which are then copied by `ghost/admin/lib/asset-delivery` to `ghost/core/core/built/admin/assets/*`

Applied to files:

  • apps/activitypub/src/routes.tsx
  • apps/stats/src/routes.tsx
  • apps/posts/src/routes.tsx
  • apps/admin/src/routes.tsx
📚 Learning: 2025-11-26T11:05:59.314Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: apps/shade/AGENTS.md:0-0
Timestamp: 2025-11-26T11:05:59.314Z
Learning: Applies to apps/shade/src/components/features/**/*.{ts,tsx} : Higher-level, opinionated components (e.g., PostShareModal, SourceTabs) should be placed in `src/components/features/*`

Applied to files:

  • apps/activitypub/src/routes.tsx
  • apps/stats/test/unit/app.test.tsx
  • apps/posts/src/routes.tsx
📚 Learning: 2025-03-13T09:02:50.102Z
Learnt from: mike182uk
Repo: TryGhost/Ghost PR: 22471
File: apps/admin-x-activitypub/src/views/Feed/components/NewPostModal.tsx:29-34
Timestamp: 2025-03-13T09:02:50.102Z
Learning: In the Ghost ActivityPub module, error handling for mutations is handled at the hook level (in use-activity-pub-queries.ts) rather than in individual components. This allows for centralized error handling across the application.

Applied to files:

  • apps/activitypub/src/routes.tsx
📚 Learning: 2025-11-26T11:05:59.314Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: apps/shade/AGENTS.md:0-0
Timestamp: 2025-11-26T11:05:59.314Z
Learning: Applies to apps/shade/src/components/layout/**/*.{ts,tsx} : Reusable layout containers (Page, Heading, Header, ViewHeader, ErrorPage) should be placed in `src/components/layout/*`

Applied to files:

  • apps/activitypub/src/routes.tsx
📚 Learning: 2025-11-25T14:28:50.351Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: When working on Admin UI features, build in React using `apps/admin-x-*` or `apps/posts` apps

Applied to files:

  • apps/activitypub/src/routes.tsx
  • apps/stats/src/routes.tsx
  • apps/posts/src/routes.tsx
📚 Learning: 2025-11-03T12:33:31.093Z
Learnt from: kevinansfield
Repo: TryGhost/Ghost PR: 25320
File: apps/admin/src/layout/app-sidebar/NavHeader.tsx:10-10
Timestamp: 2025-11-03T12:33:31.093Z
Learning: The Ghost admin apps (apps/admin/**) do not use SSR, so accessing browser APIs like `navigator`, `window`, or `document` at module load time is safe and does not require typeof guards.

Applied to files:

  • apps/activitypub/src/routes.tsx
  • apps/stats/src/routes.tsx
  • apps/posts/src/routes.tsx
  • apps/admin/src/routes.tsx
📚 Learning: 2025-08-11T14:18:31.752Z
Learnt from: niranjan-uma-shankar
Repo: TryGhost/Ghost PR: 24626
File: apps/portal/src/components/pages/AccountHomePage/components/AccountFooter.js:15-15
Timestamp: 2025-08-11T14:18:31.752Z
Learning: In Ghost, Portal components render within iframes while admin components (like gh-member-details.hbs, dashboard/charts/recents.hbs, and posts/post-activity-feed.hbs) do not render in iframes. Therefore, iframe-related navigation issues only affect Portal components.

Applied to files:

  • apps/activitypub/src/routes.tsx
  • apps/stats/src/routes.tsx
  • apps/posts/src/routes.tsx
📚 Learning: 2025-12-09T12:37:30.664Z
Learnt from: kevinansfield
Repo: TryGhost/Ghost PR: 25501
File: apps/shade/src/hooks/use-mobile.tsx:5-10
Timestamp: 2025-12-09T12:37:30.664Z
Learning: The Ghost admin apps/shade/** components do not use SSR, so accessing browser APIs like `navigator`, `window`, or `document` at module load time is safe and does not require typeof guards.

Applied to files:

  • apps/activitypub/src/routes.tsx
  • apps/stats/test/unit/app.test.tsx
  • apps/stats/src/routes.tsx
  • apps/admin/src/routes.tsx
📚 Learning: 2025-11-05T16:42:12.989Z
Learnt from: rob-ghost
Repo: TryGhost/Ghost PR: 25356
File: apps/admin/test-utils/fixtures/query-client.tsx:17-35
Timestamp: 2025-11-05T16:42:12.989Z
Learning: In apps/admin/test-utils/fixtures/query-client.tsx, the createTestQueryClient function is intentionally duplicated from admin-x-framework to reduce external dependencies in the admin app's test utilities.

Applied to files:

  • apps/stats/test/unit/app.test.tsx
📚 Learning: 2025-11-25T11:58:51.652Z
Learnt from: ibalosh
Repo: TryGhost/Ghost PR: 25525
File: apps/shade/src/shade-app.tsx:4-4
Timestamp: 2025-11-25T11:58:51.652Z
Learning: In apps/shade, the app wrapper file should be named `src/shade-app.tsx` (kebab-case) while the component itself is exported as `ShadeApp` (PascalCase). Context providers should be placed in `src/providers/*` using kebab-case filenames.

Applied to files:

  • apps/stats/test/unit/app.test.tsx
📚 Learning: 2025-11-25T14:28:50.351Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Ghost admin serves admin-x apps from `/ghost/assets/{app-name}/{app-name}.js` URLs

Applied to files:

  • apps/stats/src/routes.tsx
📚 Learning: 2025-08-11T14:18:31.752Z
Learnt from: niranjan-uma-shankar
Repo: TryGhost/Ghost PR: 24626
File: apps/portal/src/components/pages/AccountHomePage/components/AccountFooter.js:15-15
Timestamp: 2025-08-11T14:18:31.752Z
Learning: The /dashboard route in Ghost admin was retired in version 6.0.

Applied to files:

  • apps/stats/src/routes.tsx
📚 Learning: 2025-11-12T22:26:48.725Z
Learnt from: troyciesco
Repo: TryGhost/Ghost PR: 25271
File: apps/stats/src/views/Stats/Growth/Growth.tsx:199-204
Timestamp: 2025-11-12T22:26:48.725Z
Learning: In apps/stats/src/views/Stats/Growth/Growth.tsx, the loading skeleton for the top posts/pages table intentionally uses colSpan={1} to only span the first (wider) column, creating a visual effect where the skeleton appears only in the main content column (which contains post/page titles with dates).

Applied to files:

  • apps/stats/src/routes.tsx
📚 Learning: 2025-07-14T12:20:35.268Z
Learnt from: sagzy
Repo: TryGhost/Ghost PR: 24346
File: apps/admin-x-settings/src/components/settings/growth/Network.tsx:8-12
Timestamp: 2025-07-14T12:20:35.268Z
Learning: The Network component toggle in `apps/admin-x-settings/src/components/settings/growth/Network.tsx` is intentionally implemented as static UI with a no-op onChange handler, as part of a UI-first development approach before connecting actual ActivityPub functionality.

Applied to files:

  • apps/admin/src/routes.tsx
📚 Learning: 2025-09-03T12:28:11.174Z
Learnt from: kevinansfield
Repo: TryGhost/Ghost PR: 24779
File: ghost/core/core/server/services/members/members-api/controllers/RouterController.js:34-51
Timestamp: 2025-09-03T12:28:11.174Z
Learning: The extractRefererOrRedirect function in Ghost's RouterController (ghost/core/core/server/services/members/members-api/controllers/RouterController.js) is working as intended according to maintainer kevinansfield. The current handling of autoRedirect, redirect parameter parsing, and referrer logic does not need the security-related changes suggested around autoRedirect coercion, relative URL handling, or same-origin checks.

Applied to files:

  • apps/admin/src/routes.tsx
📚 Learning: 2025-03-13T09:00:20.205Z
Learnt from: mike182uk
Repo: TryGhost/Ghost PR: 22471
File: apps/admin-x-activitypub/src/utils/pending-activity.ts:13-71
Timestamp: 2025-03-13T09:00:20.205Z
Learning: The pending activity utilities in the Ghost ActivityPub module are covered by tests in the file `apps/admin-x-activitypub/test/unit/utils/pending-activity.ts`.

Applied to files:

  • apps/admin/src/routes.tsx
🧬 Code graph analysis (3)
apps/admin-x-framework/src/utils/lazy-component.ts (1)
apps/admin-x-framework/src/index.ts (1)
  • lazyComponent (43-43)
apps/activitypub/src/routes.tsx (2)
apps/admin-x-framework/src/index.ts (1)
  • lazyComponent (43-43)
apps/admin-x-framework/src/utils/lazy-component.ts (1)
  • lazyComponent (1-7)
apps/posts/src/routes.tsx (2)
apps/admin-x-framework/src/index.ts (1)
  • lazyComponent (43-43)
apps/admin-x-framework/src/utils/lazy-component.ts (1)
  • lazyComponent (1-7)
⏰ 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). (6)
  • GitHub Check: Admin-X Settings tests
  • GitHub Check: ActivityPub tests
  • GitHub Check: Admin tests - Chrome
  • GitHub Check: Unit tests (Node 22.18.0)
  • GitHub Check: Lint
  • GitHub Check: Build & Push Docker Image
🔇 Additional comments (2)
apps/admin/src/routes.tsx (1)

1-1: Settings route lazy-loading looks correct

Using lazy: lazyComponent(() => import("./settings/settings")) for path: "settings/*" matches the new helper’s contract and keeps the route shape intact while deferring loading of the settings bundle. No issues from a routing or typing perspective assuming ./settings/settings now has a default export.

Also applies to: 58-60

apps/stats/src/routes.tsx (1)

1-29: Route-level lazy-loading for stats views looks consistent

Swapping the analytics child routes to lazy: lazyComponent(() => import('./views/Stats/...')) keeps the route structure intact while deferring view bundles until needed. The index route and named paths are preserved, and the usage matches the new helper’s contract.

@jonatansberg jonatansberg merged commit 362bda8 into main Dec 15, 2025
35 checks passed
@jonatansberg jonatansberg deleted the ber-3126-generated-bundle-is-huge branch December 15, 2025 07:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants