Skip to content

feat: introduce branded user identifiers #7

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

Open
leynos wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: introduce branded user identifiers #7

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
13 changes: 9 additions & 4 deletions frontend-pwa/src/api/client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,17 @@
import { z } from 'zod';
import { customFetchParsed } from './fetcher';

export type UserId = string & { readonly brand: 'UserId' };

Comment on lines +7 to +8
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Aug 14, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Prefer Zod .brand() and derive UserId from a schema; drop the manual intersection brand

Eliminate the hand-rolled brand and derive the type from a dedicated schema to keep runtime validation and types in lockstep and avoid the no-op transform later.

Apply this diff to replace the manual brand with a schema-derived type:

-export type UserId = string & { readonly brand: 'UserId' };
+/** Branded user identifier (nominal type), parsed by userIdSchema. */
+export type UserId = z.infer<typeof userIdSchema>;

Then, add this schema near the imports (outside this hunk):

// Reusable branded ID schema
export const userIdSchema = z.string().brand<'UserId'>();

export interface User {
id: UserId;
display_name: string;
}

Comment on lines +9 to +13
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Aug 14, 2025

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Document and freeze the public User shape; mark fields readonly

Document the export and make fields readonly to discourage mutation of parsed data in consumers.

-export interface User {
-  id: UserId;
-  display_name: string;
-}
+/** Public user shape parsed from the API. Invariant: id is a branded identifier. */
+export interface User {
+  readonly id: UserId;
+  readonly display_name: string;
+}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export interface User {
id: UserId;
display_name: string;
}
/** Public user shape parsed from the API. Invariant: id is a branded identifier. */
export interface User {
readonly id: UserId;
readonly display_name: string;
}
🤖 Prompt for AI Agents 
In frontend-pwa/src/api/client.ts around lines 9 to 13, the exported User
interface lacks documentation and allows consumers to mutate parsed data; add a
JSDoc comment describing the public User shape and intended usage, and make its
properties readonly (e.g., readonly id: UserId; readonly display_name: string)
so callers are discouraged from mutating instances returned by the API.

const userSchema = z.object({
id: z.string(),
id: z.string().transform(id => id as UserId),
display_name: z.string(),
});

export type User = z.infer<typeof userSchema>;
}) satisfies z.ZodType<User>;

export const listUsers = (signal?: AbortSignal) =>
customFetchParsed('/api/users', z.array(userSchema), { signal });