Conversation
- Add `**fastapi_kwargs` to `CrossDocs.__init__` and `self.app` property that creates a fully configured FastAPI app with Inertia wired up (configure_inertia, lifespan, static files, router mounting) - Wrap SSR setup with `ComponentsProvider` to match client-side tree and fix hydration mismatch - Make `useTheme()` return safe defaults during SSR to handle duplicate ThemeContext in bundled SSR builds
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughAdds SSR-safe defaults to the theme provider, wraps server render with ComponentsProvider, and adds zero-config FastAPI app creation in CrossDocs with Inertia, lifespan, static mounting, and lazy/explicit app exposure. Changes
Sequence Diagram(s)sequenceDiagram
participant CrossDocs
participant FastAPI
participant Inertia
participant StaticFiles
CrossDocs->>CrossDocs: __init__(**fastapi_kwargs)
CrossDocs->>CrossDocs: _create_app(**fastapi_kwargs)
CrossDocs->>FastAPI: Instantiate FastAPI(...)
CrossDocs->>Inertia: configure_inertia(app)
rect rgba(100,150,200,0.5)
Note over Inertia: apply inertia_lifespan if provided
end
CrossDocs->>StaticFiles: mount('/static')
CrossDocs->>FastAPI: include_router(cross_docs_routes)
FastAPI-->>CrossDocs: configured app
CrossDocs->>CrossDocs: store in _app and expose via app property
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://waps.l3s.uni-hannover.de/live/im_/https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
python/cross_docs/routes.py (3)
50-56:**fastapi_kwargstype hint isAny— consider documenting or narrowing.Using
**fastapi_kwargs: Anyis pragmatic for forwarding toFastAPI(...), but it means IDE autocompletion and type checking are lost for the caller. SinceUnpackwithTypedDictis available in Python 3.12+ (ortyping_extensions), this may not be worth changing now, but a brief docstring listing the most common kwargs (title,docs_url,redoc_url,lifespan) would improve DX.The existing docstring on lines 64-67 partially covers this — just noting for completeness.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@python/cross_docs/routes.py` around lines 50 - 56, The __init__ parameter **fastapi_kwargs is currently typed as Any which hides IDE/autocomplete and type checking; update the __init__ docstring for the class (and the __init__ signature comment) to enumerate the common FastAPI kwargs you expect callers to pass (e.g., title, docs_url, redoc_url, lifespan) and either narrow the type by creating a small TypedDict/Unpack alias for those keys (using typing_extensions for older Pythons) or leave the var-kwargs but explicitly document the supported keys and types in the docstring for __init__ (referencing the __init__ parameter name fastapi_kwargs and the DocsConfig type for context).
86-99: Theappproperty silently creates a full FastAPI app — could surprisemount()users.When a user instantiates
CrossDocs()(no kwargs) intending to calldocs.mount(existing_app), accidentally accessingdocs.app(e.g., in logging or debugging) will silently spin up a separate FastAPI instance with its own Inertia config and static mounts. This is a side-effect-heavy property.Consider either raising an error if
_appisNoneand nofastapi_kwargswere provided, or adding a flag to distinguish "user wants auto-app" from "user will mount manually."Proposed sketch
def __init__( self, config: DocsConfig | None = None, *, docs_component: str | None = None, home_component: str | None = None, **fastapi_kwargs: Any, ): ... + self._auto_app = bool(fastapi_kwargs) self._app: FastAPI | None = None if fastapi_kwargs: self._app = self._create_app(**fastapi_kwargs) `@property` def app(self) -> FastAPI: - if self._app is None: - self._app = self._create_app() - return self._app + if self._app is None: + if not self._auto_app: + # Still allow lazy creation, but could also raise here + self._app = self._create_app() + return self._app🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@python/cross_docs/routes.py` around lines 86 - 99, The app property currently auto-creates a FastAPI instance via _create_app when self._app is None which can surprise callers who intend to mount CrossDocs into an existing app; add an explicit opt-in flag in the constructor (e.g., auto_create_app: bool = True) and store it on the instance, then change the app property to: if self._app is None and not self.auto_create_app raise a clear RuntimeError instructing the user to call mount(existing_app) (otherwise keep the existing lazy-creation behavior by calling _create_app); update __init__ to accept the new flag and ensure mount() still sets self._app so subsequent app accesses work.
101-115: Callconfigure_inertiaat module level, not inside_create_app.The current design calls
configure_inertia()inside the_create_app()instance method. This means the configuration is repeated if multipleCrossDocsinstances are created withfastapi_kwargsor if.appis accessed multiple times (though the lazy caching prevents the latter). Whileconfigure_inertia()likely handles multiple calls gracefully, this is an architectural inconsistency with the canonical usage pattern.In
website/app.py,configure_inertia()is called once at module level before app creation. Consider moving the call out of_create_app()to keep global configuration separate from instance creation, or document that only one auto-creating instance per process is supported.The hardcoded paths (
"app.tsx","static/build/ssr/ssr.js", etc.) are intentional for the "zero-config" feature and are fine.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@python/cross_docs/routes.py` around lines 101 - 115, Move the configure_inertia call out of the CrossDocs._create_app method and call it once at module import time instead: remove the configure_inertia(...) invocation from _create_app, add a single configure_inertia(...) call at module level (above the CrossDocs class) with the same arguments, and keep the rest of _create_app intact (retain fastapi_kwargs.setdefault("lifespan", inertia_lifespan), FastAPI instantiation, app.mount and self.mount usage). Ensure configure_inertia is imported in the module so the module-level call runs once when the module is loaded.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@python/cross_docs/routes.py`:
- Around line 50-56: The __init__ parameter **fastapi_kwargs is currently typed
as Any which hides IDE/autocomplete and type checking; update the __init__
docstring for the class (and the __init__ signature comment) to enumerate the
common FastAPI kwargs you expect callers to pass (e.g., title, docs_url,
redoc_url, lifespan) and either narrow the type by creating a small
TypedDict/Unpack alias for those keys (using typing_extensions for older
Pythons) or leave the var-kwargs but explicitly document the supported keys and
types in the docstring for __init__ (referencing the __init__ parameter name
fastapi_kwargs and the DocsConfig type for context).
- Around line 86-99: The app property currently auto-creates a FastAPI instance
via _create_app when self._app is None which can surprise callers who intend to
mount CrossDocs into an existing app; add an explicit opt-in flag in the
constructor (e.g., auto_create_app: bool = True) and store it on the instance,
then change the app property to: if self._app is None and not
self.auto_create_app raise a clear RuntimeError instructing the user to call
mount(existing_app) (otherwise keep the existing lazy-creation behavior by
calling _create_app); update __init__ to accept the new flag and ensure mount()
still sets self._app so subsequent app accesses work.
- Around line 101-115: Move the configure_inertia call out of the
CrossDocs._create_app method and call it once at module import time instead:
remove the configure_inertia(...) invocation from _create_app, add a single
configure_inertia(...) call at module level (above the CrossDocs class) with the
same arguments, and keep the rest of _create_app intact (retain
fastapi_kwargs.setdefault("lifespan", inertia_lifespan), FastAPI instantiation,
app.mount and self.mount usage). Ensure configure_inertia is imported in the
module so the module-level call runs once when the module is loaded.
Accessing `docs.app` without passing FastAPI kwargs now raises a RuntimeError instead of silently creating a full app with side effects.
1 participant
Summary
CrossDocsnow owns the full app setup — consumers just pass FastAPI kwargs and accessdocs.app:Internally calls
configure_inertia(), createsFastAPI(lifespan=inertia_lifespan), mounts static files, and includes the docs router.mount()still works for backward compatibility.Fix SSR hydration mismatch — the SSR setup now wraps
<App>with<ComponentsProvider>to match the client-side treeFix SSR crash from duplicate ThemeContext —
useTheme()returns safe defaults during SSR instead of throwing, since bundlers (Vite withnoExternal: true) create two copies of ThemeContext from the separate./and./ssrpackage entriesTest plan
bun run build && bun run build:ssr)Summary by CodeRabbit
New Features
Bug Fixes