-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
fix: useMutation example hook eslint no-shadow #6094
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.
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
fix: useMutation example hook eslint no-shadow #6094
Conversation
WalkthroughRenamed internal state identifiers and the public return properties of multiple useMutation hooks: React hooks now expose Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 7
🧹 Nitpick comments (2)
e2e/solid-start/basic-auth/src/hooks/useMutation.ts (1)
8-10: Consider aligning setter names with the renamed signals (setSignalVariables/setSignalData).
Line 8 and Line 10 renamed the signals, but the setters are stillsetVariables/setData, which is a bit misleading during maintenance.examples/solid/kitchen-sink-file-based/src/hooks/useMutation.tsx (1)
15-30: Drop the unused setter arg insetData((prev) => data).
On Line 25,previs unused—prefersetData(() => data)(orsetData(data)if you don’t need the functional form).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
e2e/react-start/basic-auth/src/hooks/useMutation.ts(2 hunks)e2e/solid-start/basic-auth/src/hooks/useMutation.ts(2 hunks)examples/react/kitchen-sink-file-based/src/hooks/useMutation.tsx(2 hunks)examples/react/kitchen-sink/src/useMutation.tsx(2 hunks)examples/react/start-basic-auth/src/hooks/useMutation.ts(2 hunks)examples/react/start-supabase-basic/src/hooks/useMutation.ts(2 hunks)examples/solid/kitchen-sink-file-based/src/hooks/useMutation.tsx(2 hunks)examples/solid/kitchen-sink/src/useMutation.tsx(3 hunks)examples/solid/start-basic-auth/src/hooks/useMutation.ts(2 hunks)examples/solid/start-supabase-basic/src/hooks/useMutation.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript strict mode with extensive type safety for all code
Files:
examples/solid/start-basic-auth/src/hooks/useMutation.tsexamples/react/start-supabase-basic/src/hooks/useMutation.tsexamples/react/kitchen-sink/src/useMutation.tsxexamples/solid/kitchen-sink-file-based/src/hooks/useMutation.tsxexamples/solid/start-supabase-basic/src/hooks/useMutation.tse2e/react-start/basic-auth/src/hooks/useMutation.tsexamples/react/kitchen-sink-file-based/src/hooks/useMutation.tsxexamples/solid/kitchen-sink/src/useMutation.tsxexamples/react/start-basic-auth/src/hooks/useMutation.tse2e/solid-start/basic-auth/src/hooks/useMutation.ts
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Implement ESLint rules for router best practices using the ESLint plugin router
Files:
examples/solid/start-basic-auth/src/hooks/useMutation.tsexamples/react/start-supabase-basic/src/hooks/useMutation.tsexamples/react/kitchen-sink/src/useMutation.tsxexamples/solid/kitchen-sink-file-based/src/hooks/useMutation.tsxexamples/solid/start-supabase-basic/src/hooks/useMutation.tse2e/react-start/basic-auth/src/hooks/useMutation.tsexamples/react/kitchen-sink-file-based/src/hooks/useMutation.tsxexamples/solid/kitchen-sink/src/useMutation.tsxexamples/react/start-basic-auth/src/hooks/useMutation.tse2e/solid-start/basic-auth/src/hooks/useMutation.ts
🧬 Code graph analysis (4)
examples/solid/start-basic-auth/src/hooks/useMutation.ts (1)
examples/solid/kitchen-sink/src/useMutation.tsx (1)
error(44-46)
examples/solid/kitchen-sink-file-based/src/hooks/useMutation.tsx (1)
examples/solid/kitchen-sink/src/useMutation.tsx (1)
error(44-46)
examples/solid/start-supabase-basic/src/hooks/useMutation.ts (1)
examples/solid/kitchen-sink/src/useMutation.tsx (1)
error(44-46)
e2e/solid-start/basic-auth/src/hooks/useMutation.ts (1)
examples/solid/kitchen-sink/src/useMutation.tsx (1)
error(44-46)
🔇 Additional comments (4)
examples/solid/kitchen-sink/src/useMutation.tsx (2)
8-10: LGTM! No-shadow fix correctly implemented.The internal signal variables are properly renamed to
signalVariablesandsignalData, eliminating the shadowing issue with thevariablesparameter in the mutate function (line 15).
37-49: Excellent! Backward compatibility maintained.The getters preserve the public API by exposing
variablesanddataproperties while internally using the renamed signals. This approach eliminates the ESLint no-shadow violation without breaking existing consumers.e2e/react-start/basic-auth/src/hooks/useMutation.ts (1)
8-10: No-shadow fix correctly implemented.The state variables are properly renamed to eliminate shadowing. The public API now exposes
stateVariablesandstateDatainstead ofvariablesanddata.Since this is an e2e test hook, verify that test files using this hook have been updated:
#!/bin/bash # Search for usage in e2e test files that may expect the old API rg -n --type=ts --type=tsx -C3 'useMutation' e2e/react-start/basic-auth/ | rg -C3 '\.variables|\.data'examples/react/kitchen-sink-file-based/src/hooks/useMutation.tsx (1)
8-10: No-shadow fix correctly implemented. Design choice differs from Solid pattern.The state variables are properly renamed to
stateVariablesandstateData, eliminating the shadowing issue. This is an intentional design difference from the Solid example: the React implementation directly exposes the renamed state properties, while the Solid implementation uses getters to maintain avariables/dataAPI surface. Both approaches are valid for example code; the React approach is simpler while Solid's is more abstracted. The no-shadow refactoring itself is correct.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
e2e/react-start/basic-auth/src/routes/signup.tsx (1)
92-94: LGTM! Property renaming correctly applied.The change from
signupMutation.datatosignupMutation.stateDatacorrectly implements the no-shadow fix and is consistent across both references.Optional type safety enhancement:
Consider adding optional chaining to line 94 for consistency and to satisfy strict TypeScript:
- <div className="text-red-400">{signupMutation.stateData.message}</div> + <div className="text-red-400">{signupMutation.stateData?.message}</div>While the logic ensures
stateDataexists when the condition on line 92 is truthy, TypeScript's type narrowing may not recognize this across the ternary operator, and the optional chaining makes the code more defensive.As per coding guidelines, extensive type safety is recommended for TypeScript code.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
e2e/react-start/basic-auth/src/components/Login.tsx(1 hunks)e2e/react-start/basic-auth/src/routes/signup.tsx(1 hunks)e2e/solid-start/basic-auth/src/components/Login.tsx(1 hunks)e2e/solid-start/basic-auth/src/routes/signup.tsx(1 hunks)examples/react/kitchen-sink-file-based/src/routes/dashboard.invoices.$invoiceId.tsx(1 hunks)examples/react/kitchen-sink/src/main.tsx(1 hunks)examples/react/start-basic-auth/src/components/Login.tsx(1 hunks)examples/react/start-basic-auth/src/routes/signup.tsx(1 hunks)examples/react/start-supabase-basic/src/components/Login.tsx(1 hunks)examples/react/start-supabase-basic/src/routes/signup.tsx(1 hunks)examples/solid/kitchen-sink-file-based/src/routes/dashboard.invoices.$invoiceId.tsx(1 hunks)examples/solid/start-basic-auth/src/components/Login.tsx(1 hunks)examples/solid/start-basic-auth/src/routes/signup.tsx(1 hunks)examples/solid/start-supabase-basic/src/components/Login.tsx(1 hunks)examples/solid/start-supabase-basic/src/routes/signup.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript strict mode with extensive type safety for all code
Files:
examples/react/start-supabase-basic/src/components/Login.tsxexamples/react/start-basic-auth/src/routes/signup.tsxe2e/solid-start/basic-auth/src/routes/signup.tsxe2e/react-start/basic-auth/src/components/Login.tsxexamples/react/start-basic-auth/src/components/Login.tsxexamples/solid/start-basic-auth/src/routes/signup.tsxexamples/solid/start-supabase-basic/src/components/Login.tsxexamples/solid/kitchen-sink-file-based/src/routes/dashboard.invoices.$invoiceId.tsxexamples/solid/start-supabase-basic/src/routes/signup.tsxe2e/solid-start/basic-auth/src/components/Login.tsxexamples/react/kitchen-sink/src/main.tsxexamples/react/kitchen-sink-file-based/src/routes/dashboard.invoices.$invoiceId.tsxe2e/react-start/basic-auth/src/routes/signup.tsxexamples/solid/start-basic-auth/src/components/Login.tsxexamples/react/start-supabase-basic/src/routes/signup.tsx
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Implement ESLint rules for router best practices using the ESLint plugin router
Files:
examples/react/start-supabase-basic/src/components/Login.tsxexamples/react/start-basic-auth/src/routes/signup.tsxe2e/solid-start/basic-auth/src/routes/signup.tsxe2e/react-start/basic-auth/src/components/Login.tsxexamples/react/start-basic-auth/src/components/Login.tsxexamples/solid/start-basic-auth/src/routes/signup.tsxexamples/solid/start-supabase-basic/src/components/Login.tsxexamples/solid/kitchen-sink-file-based/src/routes/dashboard.invoices.$invoiceId.tsxexamples/solid/start-supabase-basic/src/routes/signup.tsxe2e/solid-start/basic-auth/src/components/Login.tsxexamples/react/kitchen-sink/src/main.tsxexamples/react/kitchen-sink-file-based/src/routes/dashboard.invoices.$invoiceId.tsxe2e/react-start/basic-auth/src/routes/signup.tsxexamples/solid/start-basic-auth/src/components/Login.tsxexamples/react/start-supabase-basic/src/routes/signup.tsx
🔇 Additional comments (12)
examples/react/kitchen-sink-file-based/src/routes/dashboard.invoices.$invoiceId.tsx (1)
108-120: Rename looks correct; just ensure the hook’s public API really isstateVariables.This change is fine and preserves the conditional rendering logic, but it will break at runtime if
useMutationstill returnsvariablesinstead ofstateVariables(or if both exist andstateVariablesis not updated).examples/react/kitchen-sink/src/main.tsx (1)
449-462: LGTM; verify./useMutationreturnsstateVariablesin this example package too.The conditional is unchanged in intent and remains safe, but relies on the renamed field being present.
examples/solid/start-basic-auth/src/components/Login.tsx (1)
41-67: LGTM! Consistent API update across all references.All three references to
loginMutation.data()have been correctly updated tologinMutation.signalData(). The changes preserve the existing logic, maintain optional chaining, and align with the hook's API changes to fix ESLint no-shadow violations. ThesignalDatanaming is also more explicit for Solid.js's signal-based reactivity.examples/solid/kitchen-sink-file-based/src/routes/dashboard.invoices.$invoiceId.tsx (1)
107-107: LGTM! Property accessor correctly updated.The change from
variables()tosignalVariables()aligns with the PR's goal of eliminating variable shadowing in theuseMutationhook. The naming convention is semantically appropriate for Solid's signal-based reactive primitives, and the comparison logic remains functionally identical.examples/react/start-supabase-basic/src/components/Login.tsx (1)
41-45: LGTM! Consistent renaming eliminates variable shadowing.All references to
loginMutation.datahave been correctly updated tologinMutation.stateData, eliminating the ESLint no-shadow violation. The conditional logic and property accessors remain unchanged.e2e/solid-start/basic-auth/src/routes/signup.tsx (1)
92-94: LGTM! Consistent renaming for Solid signal pattern.Both references to
signupMutation.data()have been correctly updated tosignupMutation.signalData(), maintaining the signal accessor pattern while eliminating the variable shadowing issue.e2e/solid-start/basic-auth/src/components/Login.tsx (1)
41-44: LGTM! All signal data accessors correctly updated.All three references to
loginMutation.data()have been consistently updated tologinMutation.signalData(), following the Solid signal pattern and resolving the ESLint no-shadow violation.examples/react/start-supabase-basic/src/routes/signup.tsx (1)
54-56: LGTM! The property renaming fromdatatostateDatais consistent and correctly applied. TheuseMutationhook properly exportsstateData, and all references have been updated across the codebase. The optional chaining on line 54 is appropriate, and line 56 safely accesses the property within the truthy branch.examples/solid/start-supabase-basic/src/components/Login.tsx (1)
41-68: LGTM! The useMutation hook correctly provides the signalData() method.All references to
loginMutation.data()have been consistently updated tologinMutation.signalData(), which aligns with the PR's objective to fix variable shadowing in the Solid-based examples. The hook implementation exportssignalDataas a Solid signal (line 40 in useMutation.ts), and the code properly invokes it as a signal accessor. TypeScript strict mode is enabled in the project configuration, and ESLint router rules are configured.examples/solid/start-basic-auth/src/routes/signup.tsx (1)
89-93: Accessor rename is correct and complete.The change from
data()tosignalData()on lines 89 and 91 is verified as consistent with the refactoreduseMutationhook definition, which correctly exportssignalDataas a Solid.js signal. All Solid examples in the codebase have been properly updated with no remainingdata()calls. TypeScript strict mode is enabled and the usage complies with all type safety requirements.e2e/react-start/basic-auth/src/components/Login.tsx (1)
41-44: API rename applied consistently across the codebase.The changes correctly update all references from
loginMutation.datatologinMutation.stateDatain alignment with the useMutation hook refactor. The hook definition (lines 10, 42 in useMutation.ts) exports thestateDataproperty, and all component consumers in Login.tsx and signup.tsx correctly use the new API. The callback parameter at line 14 (ctx.data?.error) remains correct—it accesses the callback context parameter, which is a separate API from the mutation state property.examples/react/start-basic-auth/src/components/Login.tsx (1)
40-68: LGTM! Property renames are consistent.The updates correctly reference
loginMutation.stateDatainstead ofloginMutation.datathroughout the conditional rendering logic (lines 32, 34, 35). The useMutation hook implementation confirms it exports thestateDataproperty in its return object.Note: Line 5's
ctx.data?.errorcorrectly remains unchanged as it refers to the callback parameter, not the mutation state. The.dataproperties on lines 26 and 46 are form data variables, not mutation properties, so they appropriately remain unchanged.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
e2e/react-start/basic-auth/src/routes/signup.tsx(1 hunks)examples/react/start-basic-auth/src/routes/signup.tsx(1 hunks)examples/react/start-supabase-basic/src/routes/signup.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript strict mode with extensive type safety for all code
Files:
examples/react/start-supabase-basic/src/routes/signup.tsxe2e/react-start/basic-auth/src/routes/signup.tsxexamples/react/start-basic-auth/src/routes/signup.tsx
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Implement ESLint rules for router best practices using the ESLint plugin router
Files:
examples/react/start-supabase-basic/src/routes/signup.tsxe2e/react-start/basic-auth/src/routes/signup.tsxexamples/react/start-basic-auth/src/routes/signup.tsx
🔇 Additional comments (1)
e2e/react-start/basic-auth/src/routes/signup.tsx (1)
92-94: AI summary inconsistency detected.The AI-generated summary for this file incorrectly states that the changes replace
signupMutation.datawithsignupMutation.signalData. However, the actual code usessignupMutation.stateData(lines 92, 94), which is correct for React examples. The naming conventionsignalDatawould be appropriate for Solid framework examples, not React.
|
after review the eslint.config.js indicate that the rule for this project is off therefore i'm closing the issue |
this fix update the naming of two variable in the example code for the useMutation hook.
there is a shadowing of variable,
variableanddata:this fix aim to follow the eslint rules - no shadow
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.