Skip to content

fix: projects hook violation #3982

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.

Sign up for GitHub

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

Merged
chronark merged 1 commit into from
Sep 17, 2025
Merged

fix: projects hook violation #3982

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions apps/dashboard/app/(app)/projects/_components/create-project/create-project-dialog.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,12 @@ export const CreateProjectDialog = () => {
liveDeploymentId: null,
updatedAt: null,
id: "will-be-replace-by-server",
author: "will-be-replace-by-server",
branch: "will-be-replace-by-server",
commitTimestamp: Date.now(),
commitTitle: "will-be-replace-by-server",
domain: "will-be-replace-by-server",
regions: [],
});
await tx.isPersisted.promise;

Expand Down
113 changes: 39 additions & 74 deletions apps/dashboard/app/(app)/projects/_components/list/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { collection, collectionManager } from "@/lib/collections";
import { collection } from "@/lib/collections";
import { ilike, useLiveQuery } from "@tanstack/react-db";
import { BookBookmark, Dots } from "@unkey/icons";
import { Button, Empty } from "@unkey/ui";
Expand All @@ -17,32 +17,11 @@ export const ProjectsList = () => {
(q) =>
q
.from({ project: collection.projects })
.orderBy(({ project }) => project.updatedAt, "desc")
.where(({ project }) => ilike(project.name, `%${projectName}%`)),
[projectName],
);

// Get deployments and domains for each project
const deploymentQueries = projects.data.map((project) => {
const collections = collectionManager.getProjectCollections(project.id);
return useLiveQuery((q) => q.from({ deployment: collections.deployments }), [project.id]);
});

const domainQueries = projects.data.map((project) => {
const collections = collectionManager.getProjectCollections(project.id);
return useLiveQuery((q) => q.from({ domain: collections.domains }), [project.id]);
});

// Flatten the results
const allDeployments = deploymentQueries.flatMap((query) => query.data || []);
const allDomains = domainQueries.flatMap((query) => query.data || []);

const isLoading =
projects.isLoading ||
deploymentQueries.some((q) => q.isLoading) ||
domainQueries.some((q) => q.isLoading);

if (isLoading) {
if (projects.isLoading) {
return (
<div className="p-4">
<div
Expand All @@ -67,8 +46,9 @@ export const ProjectsList = () => {
<Empty.Icon className="w-auto" />
<Empty.Title>No Projects Found</Empty.Title>
<Empty.Description className="text-left">
There are no projects configured yet. Create your first project to start deploying and
managing your applications.
{projectName
? `No projects found matching "${projectName}". Try a different search term.`
: "There are no projects configured yet. Create your first project to start deploying and managing your applications."}
</Empty.Description>
<Empty.Actions className="mt-4 justify-start">
<a
Expand All @@ -88,55 +68,40 @@ export const ProjectsList = () => {
}

return (
<>
<div className="p-4">
<div
className="grid gap-4"
style={{
gridTemplateColumns: "repeat(auto-fit, minmax(325px, 370px))",
}}
>
{projects.data.map((project) => {
// Find active deployment and associated domain for this project
const activeDeployment = project.liveDeploymentId
? allDeployments.find((d) => d.id === project.liveDeploymentId)
: null;

// Find domain for this project
const projectDomain = allDomains.find((d) => d.projectId === project.id);

// Extract deployment regions for display
const regions = activeDeployment?.runtimeConfig?.regions?.map((r) => r.region) ?? [];

return (
<ProjectCard
projectId={project.id}
key={project.id}
name={project.name}
domain={projectDomain?.domain ?? "No domain configured"}
commitTitle={activeDeployment?.gitCommitMessage ?? "No deployments"}
commitTimestamp={activeDeployment?.gitCommitTimestamp}
branch={activeDeployment?.gitBranch ?? "—"}
author={activeDeployment?.gitCommitAuthorName ?? "—"}
regions={regions.length > 0 ? regions : ["No deployments"]}
repository={project.gitRepositoryUrl || undefined}
actions={
<ProjectActions projectId={project.id}>
<Button
variant="ghost"
size="icon"
className="mb-auto shrink-0"
title="Project actions"
>
<Dots size="sm-regular" />
</Button>
</ProjectActions>
}
/>
);
})}
</div>
<div className="p-4">
<div
className="grid gap-4"
style={{
gridTemplateColumns: "repeat(auto-fit, minmax(325px, 370px))",
}}
>
{projects.data.map((project) => (
<ProjectCard
projectId={project.id}
key={project.id}
name={project.name}
domain={project.domain}
commitTitle={project.commitTitle}
commitTimestamp={project.commitTimestamp}
branch={project.branch}
author={project.author}
regions={project.regions}
repository={project.gitRepositoryUrl || undefined}
actions={
<ProjectActions projectId={project.id}>
<Button
variant="ghost"
size="icon"
className="mb-auto shrink-0"
title="Project actions"
>
<Dots size="sm-regular" />
</Button>
</ProjectActions>
}
/>
))}
</div>
</>
</div>
);
};
2 changes: 1 addition & 1 deletion apps/dashboard/app/(app)/projects/_components/list/projects-card.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ type ProjectCardProps = {
name: string;
domain: string;
commitTitle: string;
commitTimestamp?: number;
commitTimestamp?: number | null;
branch: string;
author: string;
regions: string[];
Expand Down
8 changes: 8 additions & 0 deletions apps/dashboard/lib/collections/deploy/projects.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,14 @@ const schema = z.object({
gitRepositoryUrl: z.string().nullable(),
updatedAt: z.number().int().nullable(),
liveDeploymentId: z.string().nullable(),
// Flattened deployment fields for UI
commitTitle: z.string(),
branch: z.string(),
author: z.string(),
commitTimestamp: z.number().int().nullable(),
regions: z.array(z.string()),
// Domain field
domain: z.string(),
});

export const createProjectRequestSchema = z.object({
Expand Down
82 changes: 60 additions & 22 deletions apps/dashboard/lib/trpc/routers/deploy/project/list.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,30 +1,68 @@
import { db } from "@/lib/db";
import type { Deployment } from "@/lib/collections/deploy/deployments";
import type { Project } from "@/lib/collections/deploy/projects";
import { db, sql } from "@/lib/db";
import { ratelimit, requireUser, requireWorkspace, t, withRatelimit } from "@/lib/trpc/trpc";
import { TRPCError } from "@trpc/server";
import { deployments, domains, projects } from "@unkey/db/src/schema";

type ProjectRow = {
id: string;
name: string;
slug: string;
updated_at: number | null;
git_repository_url: string | null;
live_deployment_id: string | null;
git_commit_message: string | null;
git_branch: string | null;
git_commit_author_name: string | null;
git_commit_timestamp: number | null;
runtime_config: Deployment["runtimeConfig"] | null;
domain: string | null;
};

export const listProjects = t.procedure
.use(requireUser)
.use(requireWorkspace)
.use(withRatelimit(ratelimit.read))
.query(async ({ ctx }) => {
return await db.query.projects
.findMany({
where: (table, { eq }) => eq(table.workspaceId, ctx.workspace.id),
columns: {
id: true,
name: true,
slug: true,
updatedAt: true,
gitRepositoryUrl: true,
liveDeploymentId: true,
},
})
.catch((error) => {
console.error("Error querying projects:", error);
throw new TRPCError({
code: "INTERNAL_SERVER_ERROR",
message:
"Failed to retrieve projects due to an error. If this issue persists, please contact support.",
});
});
const result = await db.execute(sql`
Copy link
Collaborator

@chronark chronark Sep 17, 2025

Choose a reason for hiding this comment

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

what's the benefit of doing a raw sql, rather than a nice-to-read drizzle query?

Copy link
Contributor Author

@ogzhanolguncu ogzhanolguncu Sep 17, 2025

Choose a reason for hiding this comment

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

Domains were lacking in the connections for Project thats probably why I fallback to this and tbh this felt easier, but ifs a problem we can extend drizzle connections and Domains then do a drizzle query

SELECT
${projects.id},
${projects.name},
${projects.slug},
${projects.updatedAt},
${projects.gitRepositoryUrl},
${projects.liveDeploymentId},
${deployments.gitCommitMessage},
${deployments.gitBranch},
${deployments.gitCommitAuthorName},
${deployments.gitCommitTimestamp},
${deployments.runtimeConfig},
${domains.domain}
FROM ${projects}
LEFT JOIN ${deployments}
ON ${projects.liveDeploymentId} = ${deployments.id}
AND ${deployments.workspaceId} = ${ctx.workspace.id}
LEFT JOIN ${domains}
ON ${projects.id} = ${domains.projectId}
AND ${domains.workspaceId} = ${ctx.workspace.id}
Comment on lines +45 to +47
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 16, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue

Domain join can duplicate rows and pick a non‑active domain

Joining domains by projectId returns 1+ rows per project (multiple domains) and selects an arbitrary domain. Join by live deployment to get the “active” domain; consider a deterministic tie‑break (e.g., prefer custom over wildcard, newest first).

-      LEFT JOIN ${domains}
-        ON ${projects.id} = ${domains.projectId}
-        AND ${domains.workspaceId} = ${ctx.workspace.id}
+      LEFT JOIN ${domains}
+        ON ${deployments.id} IS NOT NULL
+        AND ${domains.deploymentId} = ${deployments.id}
+        AND ${domains.workspaceId} = ${ctx.workspace.id}

If you must support project‑level domains as a fallback, pull one deterministically via a subquery (ORDER BY type='custom' DESC, updated_at DESC LIMIT 1). I can draft that if desired.

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In apps/dashboard/lib/trpc/routers/deploy/project/list.ts around lines 45-47,
the current LEFT JOIN to domains by projectId can produce multiple rows per
project and return an arbitrary domain; change the query so it joins the
project's active/live deployment's domain (i.e., join through the
live_deployments/live_deployment_id) and use a deterministic tie-break when
multiple domains exist (prefer custom over wildcard, then newest), or, if
keeping a project-level fallback, replace the direct join with a subquery that
selects a single domain per project (ORDER BY (type='custom') DESC, updated_at
DESC LIMIT 1) and join that result instead so each project yields at most one
deterministic domain row.

WHERE ${projects.workspaceId} = ${ctx.workspace.id}
ORDER BY ${projects.updatedAt} DESC
`);

return (result.rows as ProjectRow[]).map(
(row): Project => ({
id: row.id,
name: row.name,
slug: row.slug,
updatedAt: row.updated_at,
gitRepositoryUrl: row.git_repository_url,
liveDeploymentId: row.live_deployment_id,
commitTitle: row.git_commit_message ?? "[DUMMY] Initial commit",
branch: row.git_branch ?? "main",
author: row.git_commit_author_name ?? "[DUMMY] Unknown Author",
commitTimestamp: row.git_commit_timestamp ?? Date.now() - 86400000,
regions: row.runtime_config?.regions?.map((r) => r.region) ?? ["us-east-1"],
Copy link
Collaborator

@chronark chronark Sep 17, 2025

Choose a reason for hiding this comment

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

this is bad, the default should absolutely not be some random region. The control plane already create a proper runtimeConfig

Copy link
Contributor Author

@ogzhanolguncu ogzhanolguncu Sep 17, 2025

Choose a reason for hiding this comment

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

That region and other [DUMMY ] data is just for nicer UI so we can find the stylistic issues on the frontend. Everything starts with [DUMMY ] or hardcoded stuff will be removed before demo.

Copy link
Collaborator

@chronark chronark Sep 17, 2025

Choose a reason for hiding this comment

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

the control plane sets the region correctly though, there is no need for default here

domain: row.domain ?? "project-temp.unkey.app",
Copy link
Collaborator

@chronark chronark Sep 17, 2025

Choose a reason for hiding this comment

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

this is flat out broken and no longer supports multiple domains per deployment

Copy link
Contributor Author

@ogzhanolguncu ogzhanolguncu Sep 17, 2025

Choose a reason for hiding this comment

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

For projects listing we only require 1 domain similar to Vercel. If you are saying we should return all the associated data and filter it on the frontend that we can do.

Copy link
Collaborator

@chronark chronark Sep 17, 2025

Choose a reason for hiding this comment

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

for projects listing maybe, but then you need a different trpc route to get more details about the deployment or whatever

if we just return dumb lists and join on the frontend, we can reuse the data on many pages

}),
);
});