Conversation
There was a problem hiding this comment.
Pull request overview
This pull request implements guest authentication support, allowing users to use the application without providing an email address. The feature enables users to start as guests and later upgrade to full accounts by linking an email or Google account while preserving their data.
Changes:
- Added guest sign-in functionality with InstantDB's guest authentication API
- Made
user.emailoptional in the schema and updated database operations to handle missing emails - Migrated permission checks from email-based to auth.id-based to support both guest and authenticated users
- Created account upgrade flow with email magic code and Google OAuth options
- Updated UI to disable trip sharing for guest users and show upgrade prompts
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| instant.schema.ts | Made user email field optional to support guest accounts |
| instant.perms.ts | Migrated all permission checks from auth.email to auth.id with $users link references |
| src/User/db.ts | Updated dbCreateUser and dbUpdateUser to accept optional email parameter |
| src/Auth/store.ts | Refactored user authentication flow to support guest users, including creation and upgrade paths |
| src/Auth/Auth.tsx | Added "Try as guest" button and improved error handling with finally blocks |
| src/Auth/UserAvatarMenu.tsx | Added "Upgrade account" menu item for guest users |
| src/Account/PageAccount.tsx | Disabled handle editing for guest users and updated email field display |
| src/Account/PageAccountUpgrade.tsx | New page implementing guest-to-full-user upgrade flow with email and Google options |
| src/Account/PageAccountUpgrade.module.css | Styling for upgrade page |
| src/Trip/TripMenu/TripMenu.tsx | Disabled trip sharing for guest users |
| src/Trip/TripHome/TripStatistics.tsx | Disabled trip sharing button for guest users |
| src/Trip/TripDialog/TripSharingDialog.tsx | Updated display logic to handle users without email |
| src/Trip/store/types.ts | Updated DbTripQueryReturnType to mark user email as optional |
| src/Routes/routes.ts | Added RouteAccountUpgrade route definition |
| src/App.tsx | Added route and lazy-loaded component for account upgrade page |
| notes/guest-auth-tech-design.md | Technical design document for the feature |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| console.log( | ||
| 'Fetched user data using auth user id:', | ||
| authResult.user.id, | ||
| userDataUsingAuthUserId, | ||
| ); |
There was a problem hiding this comment.
Console.log statements should be removed or replaced with proper logging in production code. These debug logs (lines 80-84, 212-216) are helpful during development but should either be removed or converted to use a proper logging framework that can be controlled via environment variables.
| console.log( | |
| 'Fetched user data using auth user id:', | |
| authResult.user.id, | |
| userDataUsingAuthUserId, | |
| ); | |
| if (process.env.NODE_ENV !== 'production') { | |
| console.debug( | |
| 'Fetched user data using auth user id:', | |
| authResult.user.id, | |
| userDataUsingAuthUserId, | |
| ); | |
| } |
| console.log( | ||
| 'Fetched user data using user email:', | ||
| userEmail, | ||
| userDataUsingEmail, | ||
| ); | ||
|
|
There was a problem hiding this comment.
Console.log statements should be removed or replaced with proper logging in production code. These debug logs are helpful during development but should either be removed or converted to use a proper logging framework that can be controlled via environment variables.
| console.log( | |
| 'Fetched user data using user email:', | |
| userEmail, | |
| userDataUsingEmail, | |
| ); |
| ) : ( | ||
| <> | ||
| {tripUser.email} <Text size="1">(not activated)</Text> | ||
| {tripUser.email || tripUser.handle || 'Guest user'}{' '} |
There was a problem hiding this comment.
The fallback chain tripUser.email || tripUser.handle || 'Guest user' works well, but could be clearer with explicit undefined checks. Since tripUser.email is now string | undefined, an empty string would still be truthy and might not fall through to the handle. Consider using nullish coalescing: tripUser.email ?? tripUser.handle ?? 'Guest user' to more accurately handle undefined/null values separately from empty strings.
| {tripUser.email || tripUser.handle || 'Guest user'}{' '} | |
| {tripUser.email ?? tripUser.handle ?? 'Guest user'}{' '} |
src/Auth/Auth.tsx
Outdated
| .then(() => { | ||
| setIsLoading(false); | ||
| }) | ||
| .then(() => {}) |
There was a problem hiding this comment.
The empty .then(() => {}) serves no purpose here. This was likely left in during refactoring when the setIsLoading logic was moved to the .finally() block. Consider removing this empty handler for cleaner code.
| .then(() => {}) |
src/Account/PageAccountUpgrade.tsx
Outdated
|
|
||
| db.auth | ||
| .signInWithMagicCode({ email: sentEmail, code }) | ||
| .then(() => {}) |
There was a problem hiding this comment.
Similar to the Auth.tsx issue, the empty .then(() => {}) serves no purpose. Consider removing this empty handler for cleaner code.
| .then(() => {}) |
src/Account/PageAccountUpgrade.tsx
Outdated
| void db | ||
| .queryOnce({ | ||
| user: { | ||
| $: { | ||
| where: { email }, | ||
| limit: 1, | ||
| }, | ||
| }, | ||
| }) | ||
| .then(({ data }) => { | ||
| const existingUser = data?.user?.[0] as { id: string } | undefined; | ||
| if (existingUser && existingUser.id !== currentUserId) { | ||
| setSentEmail(''); | ||
| publishToast({ | ||
| root: { duration: Number.POSITIVE_INFINITY }, | ||
| title: { children: 'Email already in use' }, | ||
| description: { | ||
| children: | ||
| 'This email is already linked to an existing account. Please log in with that account instead.', | ||
| }, | ||
| close: {}, | ||
| }); | ||
| setIsLoading(false); | ||
| return; | ||
| } | ||
|
|
||
| db.auth | ||
| .sendMagicCode({ email }) | ||
| .then(() => { | ||
| setSentEmail(email); | ||
| setScreen(UpgradeScreen.EmailVerify); | ||
| publishToast({ | ||
| root: { duration: Number.POSITIVE_INFINITY }, | ||
| title: { children: 'Email sent!' }, | ||
| description: { | ||
| children: `Please check your mailbox for ${email}`, | ||
| }, | ||
| close: {}, | ||
| }); | ||
| }) | ||
| .catch((err: unknown) => { | ||
| setSentEmail(''); | ||
| publishToast({ | ||
| root: { duration: Number.POSITIVE_INFINITY }, | ||
| title: { children: `Error sending email to ${email}` }, | ||
| description: { | ||
| children: (err as { body?: { message?: string } }).body | ||
| ?.message, | ||
| }, | ||
| close: {}, | ||
| }); | ||
| }) | ||
| .finally(() => { | ||
| setIsLoading(false); | ||
| }); | ||
| }) | ||
| .catch((err: unknown) => { | ||
| setSentEmail(''); | ||
| publishToast({ | ||
| root: { duration: Number.POSITIVE_INFINITY }, | ||
| title: { children: `Error validating ${email}` }, | ||
| description: { | ||
| children: (err as { message?: string }).message, | ||
| }, | ||
| close: {}, | ||
| }); |
There was a problem hiding this comment.
There's a potential race condition in the email verification logic. The code checks if an email is already in use before sending the magic code (lines 198-222), but another user could claim that email between the check and when the guest user verifies the code. The duplicate check in UpgradeEmailVerify (lines 332-355) helps mitigate this, but the user experience could be improved by consolidating the validation at verification time only, avoiding the false negative case where an email appears available but becomes unavailable before verification completes.
| void db | |
| .queryOnce({ | |
| user: { | |
| $: { | |
| where: { email }, | |
| limit: 1, | |
| }, | |
| }, | |
| }) | |
| .then(({ data }) => { | |
| const existingUser = data?.user?.[0] as { id: string } | undefined; | |
| if (existingUser && existingUser.id !== currentUserId) { | |
| setSentEmail(''); | |
| publishToast({ | |
| root: { duration: Number.POSITIVE_INFINITY }, | |
| title: { children: 'Email already in use' }, | |
| description: { | |
| children: | |
| 'This email is already linked to an existing account. Please log in with that account instead.', | |
| }, | |
| close: {}, | |
| }); | |
| setIsLoading(false); | |
| return; | |
| } | |
| db.auth | |
| .sendMagicCode({ email }) | |
| .then(() => { | |
| setSentEmail(email); | |
| setScreen(UpgradeScreen.EmailVerify); | |
| publishToast({ | |
| root: { duration: Number.POSITIVE_INFINITY }, | |
| title: { children: 'Email sent!' }, | |
| description: { | |
| children: `Please check your mailbox for ${email}`, | |
| }, | |
| close: {}, | |
| }); | |
| }) | |
| .catch((err: unknown) => { | |
| setSentEmail(''); | |
| publishToast({ | |
| root: { duration: Number.POSITIVE_INFINITY }, | |
| title: { children: `Error sending email to ${email}` }, | |
| description: { | |
| children: (err as { body?: { message?: string } }).body | |
| ?.message, | |
| }, | |
| close: {}, | |
| }); | |
| }) | |
| .finally(() => { | |
| setIsLoading(false); | |
| }); | |
| }) | |
| .catch((err: unknown) => { | |
| setSentEmail(''); | |
| publishToast({ | |
| root: { duration: Number.POSITIVE_INFINITY }, | |
| title: { children: `Error validating ${email}` }, | |
| description: { | |
| children: (err as { message?: string }).message, | |
| }, | |
| close: {}, | |
| }); | |
| void db.auth | |
| .sendMagicCode({ email }) | |
| .then(() => { | |
| setSentEmail(email); | |
| setScreen(UpgradeScreen.EmailVerify); | |
| publishToast({ | |
| root: { duration: Number.POSITIVE_INFINITY }, | |
| title: { children: 'Email sent!' }, | |
| description: { | |
| children: `Please check your mailbox for ${email}`, | |
| }, | |
| close: {}, | |
| }); | |
| }) | |
| .catch((err: unknown) => { | |
| setSentEmail(''); | |
| publishToast({ | |
| root: { duration: Number.POSITIVE_INFINITY }, | |
| title: { children: `Error sending email to ${email}` }, | |
| description: { | |
| children: (err as { body?: { message?: string } }).body | |
| ?.message, | |
| }, | |
| close: {}, | |
| }); | |
| }) | |
| .finally(() => { |
| await dbUpdateUser({ | ||
| id: userId, | ||
| handle: existingUser.handle || defaultHandle, | ||
| email: userEmail || undefined, |
There was a problem hiding this comment.
The expression userEmail || undefined on line 105 is redundant. If userEmail is falsy (null, undefined, or empty string), it will be converted to undefined. However, if userEmail is an empty string (which is truthy), it will pass through as an empty string rather than undefined. This could lead to subtle bugs. Consider using nullish coalescing instead: userEmail ?? undefined or simply userEmail since the parameter is already typed as optional.
| email: userEmail || undefined, | |
| email: userEmail ?? undefined, |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 31 out of 31 changed files in this pull request and generated 9 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| const handleSubmit = useCallback( | ||
| async (event: React.FormEvent<HTMLFormElement>) => { | ||
| async (event: React.SubmitEvent<HTMLFormElement>) => { |
There was a problem hiding this comment.
React.SubmitEvent is not a valid React type. The correct type for form submit events is React.FormEvent<HTMLFormElement>. This will cause TypeScript compilation errors. All occurrences of React.SubmitEvent<HTMLFormElement> should be changed to React.FormEvent<HTMLFormElement>.
| async (event: React.SubmitEvent<HTMLFormElement>) => { | |
| async (event: React.FormEvent<HTMLFormElement>) => { |
|
|
||
| const handleSubmit = useCallback( | ||
| async (event: React.FormEvent<HTMLFormElement>) => { | ||
| async (event: React.SubmitEvent<HTMLFormElement>) => { |
There was a problem hiding this comment.
React.SubmitEvent is not a valid React type. The correct type for form submit events is React.FormEvent<HTMLFormElement>. This will cause TypeScript compilation errors.
| async (event: React.SubmitEvent<HTMLFormElement>) => { | |
| async (event: React.FormEvent<HTMLFormElement>) => { |
|
|
||
| const handleSubmit = useCallback( | ||
| async (event: React.FormEvent<HTMLFormElement>) => { | ||
| async (event: React.SubmitEvent<HTMLFormElement>) => { |
There was a problem hiding this comment.
React.SubmitEvent is not a valid React type. The correct type for form submit events is React.FormEvent<HTMLFormElement>. This will cause TypeScript compilation errors.
|
|
||
| const handleSubmit = useCallback( | ||
| (e: React.FormEvent<HTMLFormElement>) => { | ||
| (e: React.SubmitEvent<HTMLFormElement>) => { |
There was a problem hiding this comment.
React.SubmitEvent is not a valid React type. The correct type for form submit events is React.FormEvent<HTMLFormElement>. This will cause TypeScript compilation errors.
| (e: React.SubmitEvent<HTMLFormElement>) => { | |
| (e: React.FormEvent<HTMLFormElement>) => { |
| .link({ | ||
| $users: defaultUserNamespaceId, | ||
| }), | ||
| db.tx.user[newUserId].create(attrs).link({ |
There was a problem hiding this comment.
Using .create() with optional schema fields may not work as expected in InstantDB. According to InstantDB patterns, .update() should be used instead of .create() for both create and update operations. The .create() method is typically for initializing with required fields only. Consider changing db.tx.user[newUserId].create(attrs) to db.tx.user[newUserId].update(attrs) to ensure compatibility with optional email field.
| db.tx.user[newUserId].create(attrs).link({ | |
| db.tx.user[newUserId].update(attrs).link({ |
|
|
||
| const handleForm = useCallback( | ||
| (e: React.FormEvent<HTMLFormElement>) => { | ||
| (e: React.SubmitEvent<HTMLFormElement>) => { |
There was a problem hiding this comment.
React.SubmitEvent is not a valid React type. The correct type for form submit events is React.FormEvent<HTMLFormElement>. This will cause TypeScript compilation errors.
| (e: React.SubmitEvent<HTMLFormElement>) => { | |
| (e: React.FormEvent<HTMLFormElement>) => { |
|
|
||
| const handleSubmit = useCallback( | ||
| (e: React.FormEvent<HTMLFormElement>) => { | ||
| (e: React.SubmitEvent<HTMLFormElement>) => { |
There was a problem hiding this comment.
React.SubmitEvent is not a valid React type. The correct type for form submit events is React.FormEvent<HTMLFormElement>. This will cause TypeScript compilation errors.
| (e: React.SubmitEvent<HTMLFormElement>) => { | |
| (e: React.FormEvent<HTMLFormElement>) => { |
| const [isLoading, setIsLoading] = useState(false); | ||
|
|
||
| const handleSubmit = useCallback( | ||
| (e: React.SubmitEvent<HTMLFormElement>) => { |
There was a problem hiding this comment.
React.SubmitEvent is not a valid React type. The correct type for form submit events is React.FormEvent<HTMLFormElement>. This will cause TypeScript compilation errors.
| (e: React.SubmitEvent<HTMLFormElement>) => { | |
| (e: React.FormEvent<HTMLFormElement>) => { |
| const [isLoading, setIsLoading] = useState(false); | ||
|
|
||
| const handleSubmit = useCallback( | ||
| (e: React.SubmitEvent<HTMLFormElement>) => { |
There was a problem hiding this comment.
React.SubmitEvent is not a valid React type. The correct type for form submit events is React.FormEvent<HTMLFormElement>. This will cause TypeScript compilation errors.
| (e: React.SubmitEvent<HTMLFormElement>) => { | |
| (e: React.FormEvent<HTMLFormElement>) => { |
Uh oh!
There was an error while loading. Please reload this page.