-
Notifications
You must be signed in to change notification settings - Fork 37
feat: Add Fireproof Dashboard POC with tenant/ledger management #659
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for fireproof-ai-builder ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const tenantsQuery = useQuery<ResListTenantsByUser>({ | ||
| queryKey: ["listTenantsByUser"], | ||
| queryFn: wrapResultToPromise(() => api.listTenantsByUser({})), | ||
| enabled: isLoaded && isSignedIn, | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid leaking cached tenant/ledger data across users
React Query caches both tenant and ledger fetches under fixed keys (["listTenantsByUser"] and ["listLedgersByUser"], lines 56‑66) while the global QueryClient in root.tsx marks query data fresh for five minutes (lines 25‑30). If user A signs out and user B signs into the same browser within that window, React Query treats the cached data as fresh and will render A’s tenants/ledgers for B without refetching, leaking account data across sessions. The cache needs to be invalidated on auth changes or the keys scoped by user identity.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The main functional concern is that DashboardApi is imported from @fireproof/core-protocols-dashboard even though it is not yet exported by the published package, which will cause build failures and tightly couples this POC to an unpublished upstream change. There is also a compatibility risk from unguarded window.fetch usage in the route component, which will break SSR or non-DOM environments. UX-wise, the dashboard does not clearly handle the !isLoaded Clerk state, and uses array indices as React keys for ledger users, which could lead to subtle rendering issues once mutations are added. Overall structure and React Query integration look reasonable, but these issues should be addressed before merging into a mainline branch.
Additional notes (1)
- Maintainability |
vibes.diy/pkg/app/routes/fireproof.tsx:114-271
The tenant and ledger sections both implement nearly identical patterns for loading, error, empty, and list states, with only labels and field mappings differing. While acceptable for a POC, this duplication will be hard to keep in sync as you evolve the dashboard (e.g. shared styling, skeletons, or error behaviors).
Extracting a small reusable Section/List component that accepts title, query, and a render function for items would simplify future feature additions and reduce the chance of diverging UX between related lists.
Summary of changes
Summary of Changes
- Added
@fireproof/core-protocols-dashboardand@tanstack/react-queryas app dependencies and wired them into thepnpm-lock.yaml. - Introduced a global
QueryClientinstance and wrapped the app tree withQueryClientProviderinsideClerkProviderinroot.tsx. - Registered a new
/fireproofroute inroutes.ts. - Implemented a new
FireproofDashboardroute component that:- Uses Clerk auth (
useAuth) and aDashboardApiclient (from@fireproof/core-protocols-dashboard). - Uses React Query to fetch and display tenants and ledgers for the signed-in user.
- Provides loading, error, and empty states, plus an unauthenticated view.
- Uses Clerk auth (
- Added basic
metatags for the Fireproof dashboard route.
| const api = useMemo(() => { | ||
| return new DashboardApi({ | ||
| apiUrl: VibesDiyEnv.CONNECT_API_URL(), | ||
| fetch: window.fetch.bind(window), | ||
| getToken: async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instantiating DashboardApi with window.fetch.bind(window) will throw at render time in any non-DOM environment (SSR, tests, or future worker contexts) because window is undefined there. Since this route component is imported from the routes table, any server-side evaluation or pre-rendering will crash before auth or query logic runs.
For route components in frameworks that may execute code on the server, you should avoid unconditional window access in the render path. A safer approach is to:
- Use the global
fetch(which is available in more environments) if the API client supports it, or - Lazily create the client only when
typeof window !== 'undefined', and show a loading or unsupported message otherwise.
As-is, this POC could break SSR or automated tests that render the route.
Suggestion
Switch to the global fetch if DashboardApi does not require a bound window instance, e.g. fetch: fetch, or guard the creation with a runtime check such as if (typeof window === 'undefined') return null; (with a corresponding UI fallback) before you call new DashboardApi(...). I can suggest a concrete guard pattern and minimal UI fallback for non-browser environments if you’d like — reply with "@CharlieHelps yes please" and I’ll add it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just simplify dont test window
| export default function FireproofDashboard() { | ||
| const { isSignedIn, isLoaded, getToken } = useAuth(); | ||
|
|
||
| // Create DashboardApi instance with Clerk auth | ||
| const api = useMemo(() => { | ||
| return new DashboardApi({ | ||
| apiUrl: VibesDiyEnv.CONNECT_API_URL(), | ||
| fetch: window.fetch.bind(window), | ||
| getToken: async () => { | ||
| const token = await getToken({ template: "with-email" }); | ||
| return { | ||
| type: "clerk" as const, | ||
| token: token || "", | ||
| }; | ||
| }, | ||
| }); | ||
| }, [getToken]); | ||
|
|
||
| // Query to list all tenants for the logged-in user | ||
| const tenantsQuery = useQuery<ResListTenantsByUser>({ | ||
| queryKey: ["listTenantsByUser"], | ||
| queryFn: wrapResultToPromise(() => api.listTenantsByUser({})), | ||
| enabled: isLoaded && isSignedIn, | ||
| }); | ||
|
|
||
| // Query to list all ledgers for the logged-in user | ||
| const ledgersQuery = useQuery<ResListLedgersByUser>({ | ||
| queryKey: ["listLedgersByUser"], | ||
| queryFn: wrapResultToPromise(() => api.listLedgersByUser({})), | ||
| enabled: isLoaded && isSignedIn, | ||
| }); | ||
|
|
||
| // Not authenticated view | ||
| if (isLoaded && !isSignedIn) { | ||
| return ( | ||
| <SimpleAppLayout | ||
| headerLeft={ | ||
| <div className="flex items-center"> | ||
| <a | ||
| href="/" | ||
| className="text-light-primary dark:text-dark-primary hover:text-accent-02-light dark:hover:text-accent-02-dark flex items-center px-3 py-2" | ||
| aria-label="Go to home" | ||
| > | ||
| <HomeIcon className="h-6 w-6" /> | ||
| </a> | ||
| </div> | ||
| } | ||
| > | ||
| <div className="mx-auto max-w-4xl px-4 py-8"> | ||
| <h1 className="text-2xl font-bold mb-6">Fireproof Dashboard</h1> | ||
| <div className="border-light-decorative-01 dark:border-dark-decorative-01 rounded-sm border p-6"> | ||
| <p className="text-light-secondary dark:text-dark-secondary"> | ||
| Please sign in to view your Fireproof tenants and ledgers. | ||
| </p> | ||
| </div> | ||
| </div> | ||
| </SimpleAppLayout> | ||
| ); | ||
| } | ||
|
|
||
| return ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The FireproofDashboard component renders tenant and ledger sections even when isLoaded is false, which means you can hit a state where Clerk has not finished loading but the React Query hooks are disabled (enabled: false). In that case, the UI shows neither loading nor auth status explicitly, which can be confusing — the page appears mostly empty instead of showing a loading indicator while auth initializes.
A small explicit !isLoaded loading state before the signed-in/unsigned branches would improve perceived correctness and make the auth lifecycle clearer.
Suggestion
Add an early if (!isLoaded) branch that returns a simple loading view (e.g., a spinner or "Loading account..." message) before checking isSignedIn. This keeps the UX consistent while Clerk initializes. If you’d like a concrete snippet added to the component, reply with "@CharlieHelps yes please" and I’ll propose one.
| {ledger.users.map((user, idx) => ( | ||
| <span | ||
| key={idx} | ||
| className="px-2 py-1 text-xs rounded bg-gray-100 dark:bg-gray-800 text-light-secondary dark:text-dark-secondary" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The users.map in the ledger section uses the array index (idx) as the React key. This can cause subtle UI bugs if the users array changes over time (e.g., order or membership), because React will reuse DOM nodes incorrectly. Since each user appears to have a stable identifier (user.userId), that would be a more appropriate key.
In a dashboard that will eventually support mutations (add/remove users), using a stable key now will avoid headaches later.
Suggestion
Prefer a stable identifier over the array index for the React key, e.g. key={user.userId} (or a composite of ledger.ledgerId and user.userId if needed). I can draft the exact change in this map call if you reply with "@CharlieHelps yes please".
| // Helper to convert Result monad to Promise for React Query | ||
| function wrapResultToPromise<T>(pro: () => Promise<Result<T>>) { | ||
| return async (): Promise<T> => { | ||
| const res = await pro(); | ||
| if (res.isOk()) { | ||
| return res.Ok(); | ||
| } | ||
| throw res.Err(); | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wrapResultToPromise is a nice adapter, but it currently throws res.Err() directly, which for non-Error values will produce opaque error output in React Query (e.g. "Error: [object Object]") and can break places that do instanceof Error checks. In this file you are already checking tenantsQuery.error instanceof Error, so non-Error values will silently fall back to the generic "Unknown error" message even if the underlying error contained useful details.
Given you mentioned following the pattern of throwing errors directly from .Err(), it's worth normalizing here at the boundary with React Query to guarantee consumers always see an Error with a meaningful message while still preserving the original error for logging or debugging.
Suggestion
You can wrap non-Error results into an Error instance while preserving details for inspection, so React Query and your UI logic behave consistently:
function wrapResultToPromise<T>(pro: () => Promise<Result<T>>) {
return async (): Promise<T> => {
const res = await pro();
if (res.isOk()) {
return res.Ok();
}
const err = res.Err();
if (err instanceof Error) {
throw err;
}
// Preserve original error object for debugging
// while ensuring React Query always gets an Error
const wrapped = new Error(
typeof err === "string" ? err : "Request failed",
);
(wrapped as any).cause = err;
throw wrapped;
};
}This keeps the Result-monad semantics but makes error handling in the UI predictable. Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this suggestion.
| // Query to list all tenants for the logged-in user | ||
| const tenantsQuery = useQuery<ResListTenantsByUser>({ | ||
| queryKey: ["listTenantsByUser"], | ||
| queryFn: wrapResultToPromise(() => api.listTenantsByUser({})), | ||
| enabled: isLoaded && isSignedIn, | ||
| }); | ||
|
|
||
| // Query to list all ledgers for the logged-in user | ||
| const ledgersQuery = useQuery<ResListLedgersByUser>({ | ||
| queryKey: ["listLedgersByUser"], | ||
| queryFn: wrapResultToPromise(() => api.listLedgersByUser({})), | ||
| enabled: isLoaded && isSignedIn, | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both tenants and ledgers queries use static keys without incorporating the current user or auth state. If a user signs out and another user signs in within the same browser session, React Query will happily serve the cached data for the previous user under the same keys ("listTenantsByUser", "listLedgersByUser"), leading to potential cross-user data leakage within the client session.
Using Clerk, you likely have access to a stable user identifier (e.g. user ID or email from the token/template). Incorporating this identifier into the query keys and/or clearing the query cache on auth changes avoids accidental reuse of previous users' cached dashboard data.
Suggestion
Include a user-specific element in the query keys so cache entries are properly segmented per user, or explicitly clear these queries on sign-out:
const { isSignedIn, isLoaded, getToken, userId } = useAuth();
const tenantsQuery = useQuery<ResListTenantsByUser>({
queryKey: ["listTenantsByUser", userId],
queryFn: wrapResultToPromise(() => api.listTenantsByUser({})),
enabled: isLoaded && isSignedIn && !!userId,
});
const ledgersQuery = useQuery<ResListLedgersByUser>({
queryKey: ["listLedgersByUser", userId],
queryFn: wrapResultToPromise(() => api.listLedgersByUser({})),
enabled: isLoaded && isSignedIn && !!userId,
});Alternatively, you can subscribe to Clerk auth events and call queryClient.removeQueries({ queryKey: ["listTenantsByUser"] }) / removeQueries({ queryKey: ["listLedgersByUser"] }) on sign-out. Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this suggestion.
| import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; | ||
|
|
||
| // Create a client instance for React Query | ||
| const queryClient = new QueryClient({ | ||
| defaultOptions: { | ||
| queries: { | ||
| staleTime: 1000 * 60 * 5, // 5 minutes | ||
| refetchOnWindowFocus: false, | ||
| }, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The global QueryClient instance is created at module scope with default options, which is good, but there is no strategy for cache invalidation on auth changes. Because the QueryClientProvider wraps the entire app, when a user signs out and another user signs in, any user-scoped queries (like your dashboard data) will remain in the cache unless keys or session boundaries are managed explicitly.
Especially when combined with static query keys, this can create subtle cross-user data exposure within the same browser session. Even if you fix the keys, it's generally a good idea to reset or partially clear the query cache on auth state transitions for user-scoped data.
Suggestion
Consider either scoping the QueryClient to the current auth session or listening to Clerk auth events to clear user-specific queries on sign-out:
// example: create a hook or effect near your auth boundary
useEffect(() => {
if (!isSignedIn && isLoaded) {
queryClient.clear();
}
}, [isSignedIn, isLoaded]);Alternatively, if you segment query keys by userId, target only those keys with removeQueries. This keeps the global cache while preventing leakage of user-specific data across sessions. Reply with "@CharlieHelps yes please" if you'd like me to add a commit wiring this up with Clerk auth events.
🎉 Hosting Preview DeploymentPreview URL: https://pr-659-vibes-hosting-v2.jchris.workers.dev Test the preview:
This preview will be automatically updated on each push to this PR |
25d0366 to
1488985
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| useEffect(() => { // Moved useEffect here | ||
| if (!isLoaded || !isSignedIn || fpCloudToken) return; | ||
|
|
||
| const fetchFpCloudToken = async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reset fp-cloud token on sign-out
The effect that fetches fp-cloud-jwt short-circuits whenever fpCloudToken is truthy, but the token state is never cleared when a user signs out. If user A opens /fireproof, gets a token, signs out, and user B signs in, fpCloudToken still holds A’s token so the effect never requests a new session token and all subsequent API calls use the stale UCAN. That results in B seeing A’s data or hitting auth errors until the page is refreshed.
Useful? React with 👍 / 👎.
| const tenantsQuery = useQuery<ResListTenantsByUser>({ | ||
| queryKey: ["listTenantsByUser"], | ||
| queryFn: wrapResultToPromise( | ||
| () => api.listTenantsByUser({}), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Scope query cache to current user
Both tenant and ledger queries use static keys (e.g. queryKey: ["listTenantsByUser"]) while the new QueryClient is configured with a 5‑minute stale window (root.tsx lines 26‑30). If user A loads the dashboard and signs out, then user B signs in within that window, React Query will treat the cached response as fresh and serve A’s tenants/ledgers to B without refetching. The query keys should include the signed-in user (or the cache should be invalidated on auth changes) to avoid cross-user data leakage.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- The
DashboardApiimport from@fireproof/core-protocols-dashboardwill continue to break builds until the upstream package exports it, tightly coupling this PR to an unpublished change. - The
FireproofDashboardroute directly useswindow.fetch, which will throw in non-DOM environments and should be guarded or replaced with globalfetch. - React Query usage currently lacks user-scoped keys and cache/session management, risking cross-user data leakage within a single browser session.
- Error normalization and minor UI/logging details (e.g.
wrapResultToPromise, missing!isLoadedstate, array index keys) can be improved for robustness and maintainability.
Summary of changes
Summary of Changes
- Added
@tanstack/react-queryand@fireproof/core-protocols-dashboardas dependencies and wired them intopnpm-lock.yamlandpackage.json. - Wrapped the app in a global
QueryClientProvider(insideClerkProvider) inroot.tsx, configuring default query options. - Registered a new
/fireproofroute inroutes.ts. - Implemented a new
FireproofDashboardroute that:- Uses Clerk auth and a
DashboardApiclient to obtain afp-cloud-jwtviagetCloudSessionTokenand then fetch tenants and ledgers with React Query. - Adds extensive logging, JWT decoding, and special handling for JWKS verification failures.
- Renders tenants and ledgers sections with loading, error, and empty states.
- Uses Clerk auth and a
- Added
token-auth-and-exchange.mddocumenting the current auth/token exchange debugging findings and hypotheses.
| const [fpCloudToken, setFpCloudToken] = useState<string | null>(null); // Moved useState here | ||
|
|
||
| // Create DashboardApi instance with Clerk auth | ||
| const api = useMemo(() => { | ||
| const apiUrl = VibesDiyEnv.CONNECT_API_URL(); | ||
| console.log( | ||
| "[Fireproof Dashboard] 🔧 Creating DashboardApi instance with URL:", | ||
| apiUrl, | ||
| ); | ||
| return new DashboardApi({ | ||
| apiUrl, | ||
| fetch: window.fetch.bind(window), | ||
| getToken: async () => { | ||
| if (fpCloudToken) { | ||
| console.log("[Fireproof Dashboard] 🔑 Using existing fp-cloud-jwt."); | ||
| return { | ||
| type: "ucan" as const, // Corrected type here | ||
| token: fpCloudToken, | ||
| }; | ||
| } | ||
|
|
||
| console.log( | ||
| "[Fireproof Dashboard] 🔑 Getting Clerk token with template: with-email", | ||
| ); | ||
| const token = await getToken({ template: "with-email" }); | ||
|
|
||
| if (token) { | ||
| try { | ||
| const claims = decodeJwt(token); | ||
| const header = decodeProtectedHeader(token); | ||
|
|
||
| // Check KID match for debugging | ||
| const expectedKid = "ins_35qNS5Jwyc7z4aJRBIS7o205yzb"; | ||
| if (header.kid === expectedKid) { | ||
| console.log("[Fireproof Dashboard] ✅ KID MATCHES expected dev key:", header.kid); | ||
| } else { | ||
| console.warn("[Fireproof Dashboard] ⚠️ KID MISMATCH:", { | ||
| tokenKid: header.kid, | ||
| expectedKid, | ||
| note: "This might be why verification fails if the backend is using the 'expected' key." | ||
| }); | ||
| } | ||
|
|
||
| const now = Date.now() / 1000; | ||
| const exp = claims.exp || 0; | ||
| const ttl = exp - now; | ||
|
|
||
| console.log("[Fireproof Dashboard] 🕵️♀️ Token Details:", { | ||
| header, | ||
| payload: { | ||
| iss: claims.iss, | ||
| aud: claims.aud, | ||
| exp: claims.exp, | ||
| iat: claims.iat, | ||
| ttl: ttl.toFixed(2) + "s", | ||
| }, | ||
| }); | ||
|
|
||
| if (exp < now) { | ||
| console.error( | ||
| "/[Fireproof Dashboard] ❌ Token is EXPIRED! Client clock may be wrong or Clerk returned old token.", | ||
| ); | ||
| } | ||
| } catch (e) { | ||
| console.error("[Fireproof Dashboard] ⚠️ Failed to parse token:", e); | ||
| } | ||
| } | ||
|
|
||
| console.log( | ||
| "/[Fireproof Dashboard] 🎫 Token retrieved:", | ||
| token ? `${token.substring(0, 20)}...` : "null", | ||
| ); | ||
| return { | ||
| type: "clerk" as const, | ||
| token: token || "", | ||
| }; | ||
| }, | ||
| }); | ||
| }, [getToken, fpCloudToken]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The memoized DashboardApi client is configured to use an fp-cloud-jwt when fpCloudToken is present, otherwise it falls back to a Clerk token. Once fpCloudToken is set, you never attempt to refresh it or check its expiry, and getToken will continue to return it indefinitely.
Given that you already decode the Clerk token and log expiry information, not doing any TTL/expiry management on the exchanged token risks 401s/failures later in the session due to an expired fp-cloud-jwt being reused. At minimum, this should be called out as a known limitation, but ideally there should be a basic expiry check and refresh mechanism.
Suggestion
Augment the fpCloudToken state to also track its expiry (if available from the getCloudSessionToken response) or last-issued timestamp, and teach getToken to fall back to a fresh Clerk-based exchange when the cached fp-cloud-jwt is stale. This can be as simple as storing { token, exp } and checking exp against Date.now()/1000 before deciding whether to reuse or refresh. If you’d like, I can propose a specific shape for this state and the corresponding refresh logic—reply with "@CharlieHelps yes please" and I’ll include a concrete code change.
| useEffect(() => { // Moved useEffect here | ||
| if (!isLoaded || !isSignedIn || fpCloudToken) return; | ||
|
|
||
| const fetchFpCloudToken = async () => { | ||
| console.log("[Fireproof Dashboard] 🚀 Attempting to get fp-cloud-jwt..."); | ||
| try { | ||
| const result = await api.getCloudSessionToken({}); | ||
| if (result.isOk()) { | ||
| setFpCloudToken(result.Ok().token); | ||
| console.log("[Fireproof Dashboard] ✅ Successfully retrieved fp-cloud-jwt. Enabling data queries."); | ||
| } else { | ||
| console.error("[Fireproof Dashboard] ❌ Error getting fp-cloud-jwt:", result.Err()); | ||
| } | ||
| } catch (e) { | ||
| console.error("[Fireproof Dashboard] ❌ Exception getting fp-cloud-jwt:", e); | ||
| } | ||
| }; | ||
|
|
||
| fetchFpCloudToken(); | ||
| }, [isLoaded, isSignedIn, fpCloudToken, api]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fetchFpCloudToken is called from a useEffect that depends on api, and api is recreated whenever fpCloudToken changes. After setFpCloudToken, the component re-renders, api is recreated with a new closure over the updated fpCloudToken, and the effect runs again. The guard if (!isLoaded || !isSignedIn || fpCloudToken) return; currently prevents an infinite loop, but the dependency on api is unnecessary and makes the effect’s behaviour harder to reason about.
Since the effect only needs a stable client reference plus auth flags and fpCloudToken, tying it directly to api is over-coupling and can surprise future maintainers if api’s dependencies change.
Suggestion
Consider decoupling the effect from the full api object by either (1) extracting just the getCloudSessionToken function into a stable callback or (2) suppressing api in the dependency list with a clear comment and relying on the existing fpCloudToken/auth guards to avoid stale reads. Option (1) is cleaner long term; I can sketch the minimal refactor that memoizes getCloudSessionToken separately and uses that in the effect—reply with "@CharlieHelps yes please" and I’ll provide a patch.
| <div className="space-y-8"> | ||
| {/* Tenants Section */} | ||
| <div> | ||
| <h2 className="text-xl font-semibold mb-4">Tenants</h2> | ||
| {tenantsQuery.isLoading && ( | ||
| <div className="border-light-decorative-01 dark:border-dark-decorative-01 rounded-sm border p-6"> | ||
| <p className="text-light-secondary dark:text-dark-secondary"> | ||
| Loading tenants... | ||
| </p> | ||
| </div> | ||
| )} | ||
| {tenantsQuery.isError && ( | ||
| <div className="border-red-500 rounded-sm border p-6 bg-red-50 dark:bg-red-900/20"> | ||
| <p className="text-red-700 dark:text-red-300"> | ||
| Error loading tenants:{" "} | ||
| {tenantsQuery.error instanceof Error | ||
| ? tenantsQuery.error.message | ||
| : "Unknown error"} | ||
| </p> | ||
| </div> | ||
| )} | ||
| {tenantsQuery.isSuccess && ( | ||
| <div className="space-y-3"> | ||
| {tenantsQuery.data.tenants.length === 0 ? ( | ||
| <div className="border-light-decorative-01 dark:border-dark-decorative-01 rounded-sm border p-6"> | ||
| <p className="text-light-secondary dark:text-dark-secondary"> | ||
| No tenants found. | ||
| </p> | ||
| </div> | ||
| ) : ( | ||
| tenantsQuery.data.tenants.map((tenant: UserTenant) => ( | ||
| <div | ||
| key={tenant.tenantId} | ||
| className="border-light-decorative-01 dark:border-dark-decorative-01 rounded-sm border p-4" | ||
| > | ||
| <div className="flex justify-between items-start"> | ||
| <div> | ||
| <h3 className="font-medium text-light-primary dark:text-dark-primary"> | ||
| {tenant.tenant.name || "Unnamed Tenant"} | ||
| </h3> | ||
| <p className="text-sm text-light-secondary dark:text-dark-secondary font-mono"> | ||
| ID: {tenant.tenantId} | ||
| </p> | ||
| </div> | ||
| <div className="flex gap-2"> | ||
| <span | ||
| className={`px-2 py-1 text-xs rounded ${ | ||
| tenant.role === "admin" | ||
| ? "bg-blue-100 text-blue-800 dark:bg-blue-900/30 dark:text-blue-300" | ||
| : "bg-gray-100 text-gray-800 dark:bg-gray-800 dark:text-gray-300" | ||
| }`} | ||
| > | ||
| {tenant.role} | ||
| </span> | ||
| {tenant.default && ( | ||
| <span className="px-2 py-1 text-xs rounded bg-green-100 text-green-800 dark:bg-green-900/30 dark:text-green-300"> | ||
| default | ||
| </span> | ||
| )} | ||
| </div> | ||
| </div> | ||
| <div className="mt-2 text-sm text-light-secondary dark:text-dark-secondary"> | ||
| <p>Status: {tenant.tenant.status}</p> | ||
| <p> | ||
| Created:{" "} | ||
| {new Date( | ||
| tenant.tenant.createdAt, | ||
| ).toLocaleDateString()} | ||
| </p> | ||
| </div> | ||
| </div> | ||
| )) | ||
| )} | ||
| </div> | ||
| )} | ||
| </div> | ||
|
|
||
| {/* Ledgers Section */} | ||
| <div> | ||
| <h2 className="text-xl font-semibold mb-4">Ledgers</h2> | ||
| {ledgersQuery.isLoading && ( | ||
| <div className="border-light-decorative-01 dark:border-dark-decorative-01 rounded-sm border p-6"> | ||
| <p className="text-light-secondary dark:text-dark-secondary"> | ||
| Loading ledgers... | ||
| </p> | ||
| </div> | ||
| )} | ||
| {ledgersQuery.isError && ( | ||
| <div className="border-red-500 rounded-sm border p-6 bg-red-50 dark:bg-red-900/20"> | ||
| <p className="text-red-700 dark:text-red-300"> | ||
| Error loading ledgers:{" "} | ||
| {ledgersQuery.error instanceof Error | ||
| ? ledgersQuery.error.message | ||
| : "Unknown error"} | ||
| </p> | ||
| </div> | ||
| )} | ||
| {ledgersQuery.isSuccess && ( | ||
| <div className="space-y-3"> | ||
| {ledgersQuery.data.ledgers.length === 0 ? ( | ||
| <div className="border-light-decorative-01 dark:border-dark-decorative-01 rounded-sm border p-6"> | ||
| <p className="text-light-secondary dark:text-dark-secondary"> | ||
| No ledgers found. | ||
| </p> | ||
| </div> | ||
| ) : ( | ||
| ledgersQuery.data.ledgers.map((ledger: LedgerUser) => ( | ||
| <div | ||
| key={ledger.ledgerId} | ||
| className="border-light-decorative-01 dark:border-dark-decorative-01 rounded-sm border p-4" | ||
| > | ||
| <div className="flex justify-between items-start"> | ||
| <div> | ||
| <h3 className="font-medium text-light-primary dark:text-dark-primary"> | ||
| {ledger.name} | ||
| </h3> | ||
| <p className="text-sm text-light-secondary dark:text-dark-secondary font-mono"> | ||
| ID: {ledger.ledgerId} | ||
| </p> | ||
| <p className="text-sm text-light-secondary dark:text-dark-secondary font-mono"> | ||
| Tenant: {ledger.tenantId} | ||
| </p> | ||
| </div> | ||
| <div | ||
| className="text-sm text-light-secondary dark:text-dark-secondary" | ||
| > | ||
| <p>Users: {ledger.users.length}</p> | ||
| <p>Max shares: {ledger.maxShares}</p> | ||
| </div> | ||
| </div> | ||
| <div | ||
| className="mt-2 text-sm text-light-secondary dark:text-dark-secondary" | ||
| > | ||
| <p> | ||
| Created:{" "} | ||
| {new Date(ledger.createdAt).toLocaleDateString()} | ||
| </p> | ||
| </div> | ||
| {ledger.users.length > 0 && ( | ||
| <div | ||
| className="mt-3 pt-3 border-t border-light-decorative-01 dark:border-dark-decorative-01" | ||
| > | ||
| <p | ||
| className="text-xs font-medium text-light-secondary dark:text-dark-secondary mb-2" | ||
| > | ||
| User Access: | ||
| </p> | ||
| <div className="flex flex-wrap gap-2"> | ||
| {ledger.users.map((user, idx) => ( | ||
| <span | ||
| key={idx} | ||
| className="px-2 py-1 text-xs rounded bg-gray-100 dark:bg-gray-800 text-light-secondary dark:text-dark-secondary" | ||
| > | ||
| {user.name || user.userId} ({user.role}/ | ||
| {user.right}) | ||
| </span> | ||
| ))} | ||
| </div> | ||
| </div> | ||
| )} | ||
| </div> | ||
| )) | ||
| )} | ||
| </div> | ||
| )} | ||
| </div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tenants and ledgers UI sections are nearly identical in structure: both have loading/error/empty/list states, similar card styling, and similar list rendering. This duplication increases the chance of subtle drift (e.g. one gets improved skeletons or error handling and the other doesn’t) and makes future enhancements harder.
Given this dashboard is expected to evolve, centralizing the shared list behavior would make the codebase easier to maintain and reason about.
Suggestion
Extract a small reusable list/section component that encapsulates the common pattern—accepting props like title, query, and an renderItem callback—so you only define the loading/error/empty scaffolding once. I can propose a concrete DashboardSection abstraction using your existing Tailwind classes and React Query shape if you’d like—reply with "@CharlieHelps yes please" and I’ll add a refactor patch.
|
@jchris pls use the sts service from fireproof instead of doing evering again . |
| } | ||
|
|
||
| // Helper to convert Result monad to Promise for React Query | ||
| function wrapResultToPromise<T>(pro: () => Promise<Result<T>>, label: string) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
exception2Result from fireproof does that
| ); | ||
| return new DashboardApi({ | ||
| apiUrl, | ||
| fetch: window.fetch.bind(window), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you should be able to not to pass the fetch ---
| if (fpCloudToken) { | ||
| console.log("[Fireproof Dashboard] 🔑 Using existing fp-cloud-jwt."); | ||
| return { | ||
| type: "ucan" as const, // Corrected type here |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thats is nonsensen
|
|
||
| if (token) { | ||
| try { | ||
| const claims = decodeJwt(token); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use sts service from fireproof
|
|
||
| // Check KID match for debugging | ||
| const expectedKid = "ins_35qNS5Jwyc7z4aJRBIS7o205yzb"; | ||
| if (header.kid === expectedKid) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
never use constants in prod code
| const fetchFpCloudToken = async () => { | ||
| console.log("[Fireproof Dashboard] 🚀 Attempting to get fp-cloud-jwt..."); | ||
| try { | ||
| const result = await api.getCloudSessionToken({}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this logic is temp i will give you something better in the near future
|
|
||
|
|
||
| // Query to list all tenants for the logged-in user | ||
| const tenantsQuery = useQuery<ResListTenantsByUser>({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we make a dashboard-react packages so that this things are only once exist
7e6fcdd to
4978636
Compare
- Add loading state for Clerk auth initialization (!isLoaded) - Add userId to React Query keys for proper cache scoping - Keep using global fetch (not window.fetch) - Simplify to just ensureUser and show success Addresses feedback from @mabels on PR #659 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Fixes 'Illegal invocation' error by wrapping fetch call in an arrow function to maintain proper this binding when passed to DashboardApi. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
…and improve loading state UI

Summary
Initial proof-of-concept for Fireproof Dashboard integration at
/fireproofroute. This PR prepares the vibes.diy codebase to consume the Fireproof dashboard API (currently in unmerged PR in fireproof repo).What's Implemented
@tanstack/react-queryfor data fetchingQueryClientProviderto root layout (inside ClerkProvider, wrapping entire app)/fireproofroute with dashboard componentVITE_CONNECT_API_URLenvironment variable for API endpointCurrent Blocker
Build Error:
DashboardApiis not exported from@fireproof/core-protocols-dashboard@0.23.15The dashboard API client and types exist in the Fireproof codebase but are not yet published in the npm package. Once the Fireproof dashboard PR is merged and published, this will work.
Architecture Decisions
.Err()without wrappingFiles Changed
vibes.diy/pkg/app/root.tsx- Added QueryClientProvider setupvibes.diy/pkg/app/routes.ts- Added/fireproofroute configurationvibes.diy/pkg/app/routes/fireproof.tsx- New dashboard route componentvibes.diy/pkg/package.json- Added dependenciesNext Steps
Test Plan
/fireproof🤖 Generated with Claude Code