Skip to content

Commit 00ef6d3

Browse files
Copilothotlong
andcommitted
refactor: address code review feedback for auth client refactoring
- Simplify window origin detection into separate getWindowOrigin helper - Define proper ForgetPasswordFn type for better-auth method access - Throw error instead of returning empty AuthUser on updateUser failure - Replace nested ternary with if-else in test URL extraction Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
1 parent d0cb32e commit 00ef6d3

File tree

5 files changed

+35
-17
lines changed

5 files changed

+35
-17
lines changed

ROADMAP.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1305,7 +1305,7 @@ Plugin architecture refactoring to support true modular development, plugin isol
13051305
**Fix:**
13061306
1. Added `AuthPlugin` from `@objectstack/plugin-auth` to `objectstack.config.ts` for server mode (`pnpm dev:server`).
13071307
2. Created `authHandlers.ts` with in-memory mock implementations of better-auth endpoints for MSW mode (`pnpm dev`). Mock handlers are added to `customHandlers` in both `browser.ts` and `server.ts`.
1308-
3. Mock handlers support: sign-up/email, sign-in/email, get-session, sign-out, forgot-password, reset-password, update-user.
1308+
3. Mock handlers support: sign-up/email, sign-in/email, get-session, sign-out, forget-password (better-auth convention), reset-password, update-user.
13091309

13101310
**Tests:** 11 new auth handler tests, all existing MSW (7) and auth (24) tests pass.
13111311

apps/console/src/__tests__/authHandlers.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -143,8 +143,8 @@ describe('Mock Auth Handlers', () => {
143143
expect(sessionRes.status).toBe(401);
144144
});
145145

146-
it('should handle forgot-password', async () => {
147-
const res = await fetch(`${BASE_URL}/forgot-password`, {
146+
it('should handle forget-password', async () => {
147+
const res = await fetch(`${BASE_URL}/forget-password`, {
148148
method: 'POST',
149149
headers: { 'Content-Type': 'application/json' },
150150
body: JSON.stringify({ email: 'alice@example.com' }),

packages/auth/README.md

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ Authentication system for Object UI — AuthProvider, guards, login/register for
88
- 🛡️ **AuthGuard** - Protect routes and components from unauthenticated access
99
- 📝 **Pre-built Forms** - LoginForm, RegisterForm, and ForgotPasswordForm ready to use
1010
- 👤 **UserMenu** - Display authenticated user info with sign-out support
11-
- 🔑 **Auth Client Factory** - `createAuthClient` for pluggable backend integration
11+
- 🔑 **Auth Client Factory** - `createAuthClient` powered by official [better-auth](https://better-auth.com) client
1212
- 🌐 **Authenticated Fetch** - `createAuthenticatedFetch` for automatic token injection
1313
- 👀 **Preview Mode** - Auto-login with simulated identity for marketplace demos and app showcases
1414
- 🎯 **Type-Safe** - Full TypeScript support with exported types
@@ -30,8 +30,7 @@ import { AuthProvider, useAuth, AuthGuard } from '@object-ui/auth';
3030
import { createAuthClient } from '@object-ui/auth';
3131

3232
const authClient = createAuthClient({
33-
provider: 'custom',
34-
apiUrl: 'https://api.example.com/auth',
33+
baseURL: 'https://api.example.com/auth',
3534
});
3635

3736
function App() {

packages/auth/src/__tests__/createAuthClient.test.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,14 @@ import type { AuthClient } from '../types';
1313
function createMockFetch(handlers: Record<string, { status?: number; body: unknown }>) {
1414
const calls: Array<{ url: string; method: string; body: string | null }> = [];
1515
const mockFn = vi.fn(async (input: string | URL | Request, init?: RequestInit) => {
16-
const url = typeof input === 'string' ? input : input instanceof URL ? input.toString() : input.url;
16+
let url: string;
17+
if (typeof input === 'string') {
18+
url = input;
19+
} else if (input instanceof URL) {
20+
url = input.toString();
21+
} else {
22+
url = input.url;
23+
}
1724
calls.push({ url, method: init?.method ?? 'GET', body: init?.body as string | null });
1825
for (const [pattern, handler] of Object.entries(handlers)) {
1926
if (url.includes(pattern)) {

packages/auth/src/createAuthClient.ts

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -23,16 +23,23 @@ function resolveAuthURL(baseURL: string): { origin: string; basePath: string } {
2323
return { origin: url.origin, basePath: url.pathname.replace(/\/$/, '') };
2424
} catch {
2525
// Relative URL – resolve against the current origin when available
26-
const origin =
27-
typeof globalThis !== 'undefined' &&
28-
typeof (globalThis as Record<string, unknown>).window !== 'undefined' &&
29-
(globalThis as Record<string, unknown> & { window: { location?: { origin?: string } } }).window?.location?.origin
30-
? String((globalThis as Record<string, unknown> & { window: { location: { origin: string } } }).window.location.origin)
31-
: 'http://localhost';
26+
const origin = getWindowOrigin() ?? 'http://localhost';
3227
return { origin, basePath: baseURL.replace(/\/$/, '') };
3328
}
3429
}
3530

31+
/** Safely read window.location.origin when available (browser environments). */
32+
function getWindowOrigin(): string | undefined {
33+
try {
34+
if (typeof window !== 'undefined' && window.location?.origin) {
35+
return window.location.origin;
36+
}
37+
} catch {
38+
// window may be defined but accessing location can throw in some SSR environments
39+
}
40+
return undefined;
41+
}
42+
3643
/**
3744
* Create an auth client instance backed by the official better-auth client.
3845
*
@@ -104,8 +111,11 @@ export function createAuthClient(config: AuthClientConfig): AuthClient {
104111
},
105112

106113
async forgotPassword(email: string) {
107-
// better-auth spells this "forgetPassword"; cast to access it
108-
const forgetPw = (betterAuth as unknown as Record<string, (opts: unknown) => Promise<{ error: { message?: string; status: number } | null }>>).forgetPassword;
114+
// better-auth uses "forgetPassword" (without the "o"); the method
115+
// exists at runtime but is not present in the default TS types.
116+
type ForgetPasswordFn = (opts: { email: string; redirectTo: string }) =>
117+
Promise<{ error: { message?: string; status: number } | null }>;
118+
const forgetPw = (betterAuth as unknown as { forgetPassword: ForgetPasswordFn }).forgetPassword;
109119
const { error } = await forgetPw({ email, redirectTo: '/' });
110120
if (error) {
111121
throw new Error(error.message ?? `Auth request failed with status ${error.status}`);
@@ -124,10 +134,12 @@ export function createAuthClient(config: AuthClientConfig): AuthClient {
124134
if (error) {
125135
throw new Error(error.message ?? `Auth request failed with status ${error.status}`);
126136
}
137+
if (!data) {
138+
throw new Error('Update user returned no data');
139+
}
127140
// The server response may wrap the user in a `user` key or return it directly
128141
const raw = data as unknown as Record<string, unknown>;
129-
const user = (raw && typeof raw === 'object' && 'user' in raw ? raw.user : raw) as AuthUser;
130-
return user ?? ({} as AuthUser);
142+
return (raw && typeof raw === 'object' && 'user' in raw ? raw.user : raw) as AuthUser;
131143
},
132144
};
133145
}

0 commit comments

Comments
 (0)