Fix 4 code review findings on FRE-596

- clerk-provider.tsx: typed Clerk event listener with type guards (isClerkUserEvent, isClerkSignOutEvent) instead of (event as any) - service.ts: fixed signal propagation timing in updateProject, addCollaborator, removeCollaborator — capture updated project inside setProjects callback instead of reading stale signal after mutation - TeamManagement.tsx: added useAuth import and getAuthToken helper to replace raw localStorage reads; auth context now available in components - ProjectForm.tsx: added explicit null check on auth().user before accessing .id, replacing unsafe non-null assertion
2026-04-28 22:36:00 -04:00
parent 5dc59176bc
commit fc2b7fe970
4 changed files with 87 additions and 38 deletions
--- a/src/components/projects/ProjectForm.tsx
+++ b/src/components/projects/ProjectForm.tsx
@@ -19,11 +19,17 @@ export const ProjectForm: Component<any> = () => {
      return;
    }

+    const user = auth().user;
+    if (!user) {
+      setError('User not authenticated');
+      return;
+    }
+
    try {
      const project = await projectService.createProject(
        name().trim(),
        description().trim(),
-        auth().user!.id
+        user.id
      );
      navigate(`/projects/${project.id}`);
    } catch (err) {
--- a/src/components/teams/TeamManagement.tsx
+++ b/src/components/teams/TeamManagement.tsx
@@ -2,6 +2,7 @@ import { Component, createSignal, For, Show } from 'solid-js';
 import { A, useLocation } from '@solidjs/router';
 import { createTRPCClient } from '../../trpc/client';
 import { useQuery, useMutation, useQueryClient } from '@tanstack/react-query';
+import { useAuth } from '../../lib/auth';
 import type { Team as DbTeam, TeamMember as DbTeamMember } from '../../db/schema/teams';

 const API_URL = (import.meta as any).env?.VITE_API_URL || 'http://localhost:8080';
@@ -14,9 +15,15 @@ function formatDateTime(iso: Date | string): string {
  });
 }

+function getAuthToken(): string | null {
+  const token = localStorage.getItem('clerk_token');
+  return token || null;
+}
+
 const TeamList: Component = () => {
  const queryClient = useQueryClient();
  const client = createTRPCClient(API_URL);
+  const auth = useAuth();
  const [showCreateDialog, setShowCreateDialog] = createSignal(false);
  const [newTeamName, setNewTeamName] = createSignal('');
  const [error, setError] = createSignal('');
@@ -24,9 +31,9 @@ const TeamList: Component = () => {
  const teamsQuery = useQuery({
    queryKey: ['team', 'listTeams'],
    queryFn: async () => {
-      const auth = localStorage.getItem('clerk_token');
+      const token = getAuthToken();
      const res = await fetch(`${API_URL}/trpc/team.listTeams`, {
-        headers: auth ? { Authorization: `Bearer ${auth}` } : {},
+        headers: token ? { Authorization: `Bearer ${token}` } : {},
      });
      if (!res.ok) throw new Error('Failed to fetch teams');
      const data = await res.json();
@@ -36,12 +43,12 @@ const TeamList: Component = () => {

  const createTeamMutation = useMutation({
    mutationFn: async (name: string) => {
-      const auth = localStorage.getItem('clerk_token');
+      const token = getAuthToken();
      const res = await fetch(`${API_URL}/trpc/team.createTeam`, {
        method: 'POST',
        headers: {
          'Content-Type': 'application/json',
-          ...(auth ? { Authorization: `Bearer ${auth}` } : {}),
+          ...(token ? { Authorization: `Bearer ${token}` } : {}),
        },
        body: JSON.stringify({ input: { name } }),
      });
@@ -61,12 +68,12 @@ const TeamList: Component = () => {

  const deleteTeamMutation = useMutation({
    mutationFn: async (id: string) => {
-      const auth = localStorage.getItem('clerk_token');
+      const token = getAuthToken();
      const res = await fetch(`${API_URL}/trpc/team.deleteTeam`, {
        method: 'POST',
        headers: {
          'Content-Type': 'application/json',
-          ...(auth ? { Authorization: `Bearer ${auth}` } : {}),
+          ...(token ? { Authorization: `Bearer ${token}` } : {}),
        },
        body: JSON.stringify({ input: { id } }),
      });
@@ -203,6 +210,7 @@ const TeamList: Component = () => {

 const TeamDetail: Component<{ teamId: string }> = ({ teamId }) => {
  const queryClient = useQueryClient();
+  const auth = useAuth();
  const [error, setError] = createSignal('');
  const [showInviteDialog, setShowInviteDialog] = createSignal(false);
  const [inviteUserId, setInviteUserId] = createSignal('');
@@ -211,9 +219,9 @@ const TeamDetail: Component<{ teamId: string }> = ({ teamId }) => {
  const teamQuery = useQuery({
    queryKey: ['team', 'getTeam', teamId],
    queryFn: async () => {
-      const auth = localStorage.getItem('clerk_token');
+      const token = getAuthToken();
      const res = await fetch(`${API_URL}/trpc/team.getTeam?input=${encodeURIComponent(JSON.stringify({ id: teamId }))}`, {
-        headers: auth ? { Authorization: `Bearer ${auth}` } : {},
+        headers: token ? { Authorization: `Bearer ${token}` } : {},
      });
      if (!res.ok) throw new Error('Failed to fetch team');
      const data = await res.json();
@@ -224,9 +232,9 @@ const TeamDetail: Component<{ teamId: string }> = ({ teamId }) => {
  const membersQuery = useQuery({
    queryKey: ['team', 'listMembers', teamId],
    queryFn: async () => {
-      const auth = localStorage.getItem('clerk_token');
+      const token = getAuthToken();
      const res = await fetch(`${API_URL}/trpc/team.listMembers?input=${encodeURIComponent(JSON.stringify({ teamId }))}`, {
-        headers: auth ? { Authorization: `Bearer ${auth}` } : {},
+        headers: token ? { Authorization: `Bearer ${token}` } : {},
      });
      if (!res.ok) throw new Error('Failed to fetch members');
      const data = await res.json();
@@ -236,12 +244,12 @@ const TeamDetail: Component<{ teamId: string }> = ({ teamId }) => {

  const updateRoleMutation = useMutation({
    mutationFn: async ({ userId, role }: { userId: number; role: string }) => {
-      const auth = localStorage.getItem('clerk_token');
+      const token = getAuthToken();
      const res = await fetch(`${API_URL}/trpc/team.updateMemberRole`, {
        method: 'POST',
        headers: {
          'Content-Type': 'application/json',
-          ...(auth ? { Authorization: `Bearer ${auth}` } : {}),
+          ...(token ? { Authorization: `Bearer ${token}` } : {}),
        },
        body: JSON.stringify({ input: { teamId, userId, role } }),
      });
@@ -256,12 +264,12 @@ const TeamDetail: Component<{ teamId: string }> = ({ teamId }) => {

  const removeMemberMutation = useMutation({
    mutationFn: async (userId: number) => {
-      const auth = localStorage.getItem('clerk_token');
+      const token = getAuthToken();
      const res = await fetch(`${API_URL}/trpc/team.removeMember`, {
        method: 'POST',
        headers: {
          'Content-Type': 'application/json',
-          ...(auth ? { Authorization: `Bearer ${auth}` } : {}),
+          ...(token ? { Authorization: `Bearer ${token}` } : {}),
        },
        body: JSON.stringify({ input: { teamId, userId } }),
      });
@@ -276,12 +284,12 @@ const TeamDetail: Component<{ teamId: string }> = ({ teamId }) => {

  const leaveTeamMutation = useMutation({
    mutationFn: async () => {
-      const auth = localStorage.getItem('clerk_token');
+      const token = getAuthToken();
      const res = await fetch(`${API_URL}/trpc/team.leaveTeam`, {
        method: 'POST',
        headers: {
          'Content-Type': 'application/json',
-          ...(auth ? { Authorization: `Bearer ${auth}` } : {}),
+          ...(token ? { Authorization: `Bearer ${token}` } : {}),
        },
        body: JSON.stringify({ input: { teamId } }),
      });
@@ -296,12 +304,12 @@ const TeamDetail: Component<{ teamId: string }> = ({ teamId }) => {

  const addMemberMutation = useMutation({
    mutationFn: async ({ userId, role }: { userId: number; role: string }) => {
-      const auth = localStorage.getItem('clerk_token');
+      const token = getAuthToken();
      const res = await fetch(`${API_URL}/trpc/team.addMember`, {
        method: 'POST',
        headers: {
          'Content-Type': 'application/json',
-          ...(auth ? { Authorization: `Bearer ${auth}` } : {}),
+          ...(token ? { Authorization: `Bearer ${token}` } : {}),
        },
        body: JSON.stringify({ input: { teamId, userId, role } }),
      });
--- a/src/lib/auth/clerk-provider.tsx
+++ b/src/lib/auth/clerk-provider.tsx
@@ -21,13 +21,38 @@ interface ClerkUser {
 interface ClerkSession {
  getId: () => string;
  getUser: () => ClerkUser;
+  getToken: (template?: string) => Promise<string | null>;
+}
+
+// Clerk's addListener emits resource instances, not typed events.
+// We model the possible event shapes for type-safe handling.
+type ClerkListenerEvent =
+  | { __type: 'userSignedIn'; user: object }
+  | { __type: 'userSignedOut' }
+  | { __type: 'sessionChanged'; session: object }
+  | { __type: 'userUpdated'; user: object };
+
+function isClerkUserEvent(event: unknown): event is { user: ClerkUser } {
+  if (event && typeof event === 'object' && 'user' in event) {
+    return true;
+  }
+  return false;
+}
+
+function isClerkSignOutEvent(event: unknown): boolean {
+  if (event && typeof event === 'object') {
+    const e = event as Record<string, unknown>;
+    return e.__type === 'userSignedOut' || e.__type === 'sessionRemoved';
+  }
+  return false;
 }

 interface ClerkClient {
-  user: () => any;
-  session: () => any;
+  user: () => ClerkUser | undefined;
+  session: () => ClerkSession | null;
  isLoading: boolean;
  signOut: () => Promise<void>;
+  addListener: (handler: (event: unknown) => void) => void;
 }

 const AuthContext = createContext<Accessor<AuthState> | undefined>(undefined);
@@ -81,8 +106,8 @@ export function ClerkProvider(props: { children: JSX.Element }) {
      }

      const wrappedClient: ClerkClient = {
-        user: () => client.user,
-        session: () => (client.session as any) || null,
+        user: () => client.user || undefined,
+        session: () => (client.session as unknown as ClerkSession) || null,
        isLoading: false,
        signOut: async () => {
          await client.signOut();
@@ -93,12 +118,15 @@ export function ClerkProvider(props: { children: JSX.Element }) {
            error: null,
          });
        },
+        addListener: (handler: (event: unknown) => void) => {
+          client.addListener((event: unknown) => handler(event));
+        },
      };

      setClerkClient(wrappedClient);

      if (client.user) {
-        const session = client.session as any;
+        const session = client.session as unknown as ClerkSession;
        if (session) {
          const token = await session.getToken();
          if (token) {
@@ -117,15 +145,15 @@ export function ClerkProvider(props: { children: JSX.Element }) {
        setAuthToken(null);
      }

-      client.addListener((event) => {
-        if ((event as any).type === 'user' && (event as any).user) {
+      client.addListener((event: unknown) => {
+        if (isClerkUserEvent(event) && event.user) {
          setState({
-            user: clerkUserToUser((event as any).user),
+            user: clerkUserToUser(event.user),
            isLoading: false,
            isAuthenticated: true,
            error: null,
          });
-        } else if ((event as any).type === 'signOut') {
+        } else if (isClerkSignOutEvent(event)) {
          setState({
            user: null,
            isLoading: false,
--- a/src/lib/projects/service.ts
+++ b/src/lib/projects/service.ts
@@ -63,15 +63,18 @@ export function createProjectService(): ProjectService {
    updates: Partial<Project>
  ): Promise<Project> => {
    setLoading(true);
+    let updated: Project | undefined;
    setProjects((prev) =>
-      prev.map((p) =>
-        p.id === id
-          ? { ...p, ...updates, updatedAt: new Date().toISOString() }
-          : p
-      )
+      prev.map((p) => {
+        if (p.id === id) {
+          updated = { ...p, ...updates, updatedAt: new Date().toISOString() };
+          return updated;
+        }
+        return p;
+      })
    );
    setLoading(false);
-    return projects().find((p) => p.id === id)!;
+    return updated!;
  };

  const deleteProject = async (id: string): Promise<void> => {
@@ -86,12 +89,13 @@ export function createProjectService(): ProjectService {
    role: UserRole
  ): Promise<Project> => {
    setLoading(true);
+    let updated: Project | undefined;
    setProjects((prev) =>
      prev.map((p) => {
        if (p.id !== projectId) return p;
        const existing = p.collaborators.find((c) => c.userId === userId);
        if (existing) return p;
-        return {
+        updated = {
          ...p,
          collaborators: [
            ...p.collaborators,
@@ -99,10 +103,11 @@ export function createProjectService(): ProjectService {
          ],
          updatedAt: new Date().toISOString(),
        };
+        return updated;
      })
    );
    setLoading(false);
-    return projects().find((p) => p.id === projectId)!;
+    return updated!;
  };

  const removeCollaborator = async (
@@ -110,18 +115,20 @@ export function createProjectService(): ProjectService {
    userId: string
  ): Promise<Project> => {
    setLoading(true);
+    let updated: Project | undefined;
    setProjects((prev) =>
      prev.map((p) => {
        if (p.id !== projectId) return p;
-        return {
+        updated = {
          ...p,
          collaborators: p.collaborators.filter((c) => c.userId !== userId),
          updatedAt: new Date().toISOString(),
        };
+        return updated;
      })
    );
    setLoading(false);
-    return projects().find((p) => p.id === projectId)!;
+    return updated!;
  };

  const archiveProject = async (id: string): Promise<Project> => {