Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
14 changes: 8 additions & 6 deletions e2e/react-start/basic-auth/src/hooks/useMutation.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import * as React from 'react'

export function useMutation<TVariables, TData, TError = Error>(opts: {
fn: (variables: TVariables) => Promise<TData>
fn: (variables: TVariables) => Promise<TData | void>
onSuccess?: (ctx: { data: TData }) => void | Promise<void>
}) {
const [submittedAt, setSubmittedAt] = React.useState<number | undefined>()
Expand All @@ -20,11 +20,13 @@ export function useMutation<TVariables, TData, TError = Error>(opts: {
//
try {
const data = await opts.fn(variables)
await opts.onSuccess?.({ data })
setStatus('success')
setError(undefined)
setData(data)
return data
if (data) {
await opts.onSuccess?.({ data })
setStatus('success')
setError(undefined)
setData(data)
return data
}
Comment on lines +23 to +29
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Truthiness check will fail for valid falsy return values.

The if (data) check on line 23 uses JavaScript truthiness, which evaluates to false for valid data values like 0, false, "", or NaN. This will incorrectly skip success handling when TData is a falsy type.

🔎 Proposed fix using explicit undefined check
-        if (data) {
+        if (data !== undefined) {
           await opts.onSuccess?.({ data })
           setStatus('success')
           setError(undefined)
           setData(data)
           return data
         }

Additional concern: When data is undefined (redirect scenario), the status remains 'pending' indefinitely. Consider whether the status should transition to a different state (e.g., 'idle' or a new 'redirected' state) to accurately reflect the mutation's completion.

Note: This file is nearly identical to examples/react/start-supabase-basic/src/hooks/useMutation.ts. Consider consolidating these duplicate implementations into a shared utility.

🤖 Prompt for AI Agents
In e2e/react-start/basic-auth/src/hooks/useMutation.ts around lines 23 to 29,
the code uses a truthiness check (`if (data)`) which incorrectly treats valid
falsy values (0, false, "", NaN) as no-data and also leaves status as 'pending'
when data is undefined (redirect scenario); change the check to an explicit
undefined test (e.g., `if (data !== undefined)`) so valid falsy responses
trigger the onSuccess flow, set status to 'success' and setData for that case,
and add an explicit branch for `data === undefined` to transition status to a
suitable state (e.g., 'idle' or a new 'redirected' state) to mark the mutation
complete; also consider consolidating this file with the nearly identical
example at examples/react/start-supabase-basic/src/hooks/useMutation.ts into a
shared utility.

} catch (err: any) {
setStatus('error')
setError(err)
Expand Down
14 changes: 8 additions & 6 deletions examples/react/start-supabase-basic/src/hooks/useMutation.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import * as React from 'react'

export function useMutation<TVariables, TData, TError = Error>(opts: {
fn: (variables: TVariables) => Promise<TData>
fn: (variables: TVariables) => Promise<TData | void>
onSuccess?: (ctx: { data: TData }) => void | Promise<void>
}) {
const [submittedAt, setSubmittedAt] = React.useState<number | undefined>()
Expand All @@ -20,11 +20,13 @@ export function useMutation<TVariables, TData, TError = Error>(opts: {
//
try {
const data = await opts.fn(variables)
await opts.onSuccess?.({ data })
setStatus('success')
setError(undefined)
setData(data)
return data
if (data) {
await opts.onSuccess?.({ data })
setStatus('success')
setError(undefined)
setData(data)
return data
}
Comment on lines +23 to +29
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Truthiness check will fail for valid falsy return values.

The if (data) check on line 23 uses JavaScript truthiness, which evaluates to false for valid data values like 0, false, "", or NaN. This will incorrectly skip success handling when TData is a falsy type.

🔎 Proposed fix using explicit undefined check
-        if (data) {
+        if (data !== undefined) {
           await opts.onSuccess?.({ data })
           setStatus('success')
           setError(undefined)
           setData(data)
           return data
         }

Additional concern: When data is undefined (redirect scenario), the status remains 'pending' indefinitely. Consider whether the status should transition to a different state (e.g., 'idle' or a new 'redirected' state) to accurately reflect the mutation's completion.

📝 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
if (data) {
await opts.onSuccess?.({ data })
setStatus('success')
setError(undefined)
setData(data)
return data
}
if (data !== undefined) {
await opts.onSuccess?.({ data })
setStatus('success')
setError(undefined)
setData(data)
return data
}
🤖 Prompt for AI Agents
In examples/react/start-supabase-basic/src/hooks/useMutation.ts around lines 23
to 29, the current truthiness check "if (data)" will skip valid falsy results
(0, false, "", NaN); change the branch to explicitly check for undefined (e.g.,
"if (data !== undefined)") so success handling always runs for valid falsy
TData, call opts.onSuccess with the data, set error undefined and setData(data)
as before; additionally handle the case where data === undefined (redirect
scenario) by transitioning status out of 'pending' to an appropriate state (for
example setStatus('idle') or add/assign a 'redirected' status) so the mutation
does not remain stuck in 'pending'.

} catch (err) {
setStatus('error')
setError(err as TError)
Expand Down
4 changes: 2 additions & 2 deletions packages/react-start/src/useServerFn.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { isRedirect, useRouter } from '@tanstack/react-router'

export function useServerFn<T extends (...deps: Array<any>) => Promise<any>>(
serverFn: T,
): (...args: Parameters<T>) => ReturnType<T> {
): (...args: Parameters<T>) => ReturnType<T> | Promise<undefined> {
const router = useRouter()

return React.useCallback(
Expand All @@ -26,5 +26,5 @@ export function useServerFn<T extends (...deps: Array<any>) => Promise<any>>(
}
},
[router, serverFn],
) as any
)
}
Loading