-
Notifications
You must be signed in to change notification settings - Fork 78
refactor: remove RentGpusForm #2634
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
base: main
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughThis PR removes GPU rental functionality from the deploy web application. It eliminates the RentGpusForm component, RentGpu page, and RentGpusFormValuesType references across the codebase, narrowing form control types in SDL components to use only SdlBuilderFormValuesType. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2634 +/- ##
==========================================
- Coverage 50.91% 50.53% -0.38%
==========================================
Files 1055 1018 -37
Lines 29781 28716 -1065
Branches 6713 6546 -167
==========================================
- Hits 15162 14511 -651
+ Misses 14335 13979 -356
+ Partials 284 226 -58
*This pull request uses carry forward flags. Click here to find out more.
🚀 New features to boost your workflow:
|
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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/deploy-web/src/components/sdl/EnvFormModal/EnvFormModal.tsx (1)
26-31:⚠️ Potential issue | 🟡 MinorRemove the second
anyparameter from Control type.
Line 30 usesControl<SdlBuilderFormValuesType, any>; this violates the coding guideline against usingany. Since context is not used in this component, omit the second parameter entirely, as the rest of the codebase does.✅ Suggested fix
- control: Control<SdlBuilderFormValuesType, any>; + control: Control<SdlBuilderFormValuesType>;apps/deploy-web/src/components/sdl/MountedStorageFormControl.tsx (1)
34-42:⚠️ Potential issue | 🟠 MajorRemove explicit
anyfromControltype parameter.
Line 39 usesControl<SdlBuilderFormValuesType, any>; the second type parameter should be omitted to avoid explicit use ofany. UseControl<SdlBuilderFormValuesType>instead.Suggested fix
- control: Control<SdlBuilderFormValuesType, any>; + control: Control<SdlBuilderFormValuesType>;As per coding guidelines:
**/*.{ts,tsx,js}: Never use typeanyor cast to typeany. Always define the proper TypeScript types.
🤖 Fix all issues with AI agents
In `@apps/deploy-web/src/components/sdl/ImageSelect.tsx`:
- Around line 21-24: In Props in ImageSelect.tsx, remove the unnecessary any
type parameter on Control: change control: Control<SdlBuilderFormValuesType,
any> to use the single generic Control<SdlBuilderFormValuesType> (follow the
TokenFormControl.tsx pattern) so the resolver context generic is omitted; update
the Props type declaration accordingly to eliminate the use of any and keep
proper typing for control.
In `@apps/deploy-web/src/components/sdl/RegionSelect.tsx`:
- Around line 15-17: The prop type RegionSelectProps currently uses
Control<FieldValues, any> which introduces an unwanted any; update the type to
omit the second generic so it reads Control<FieldValues> (or just Control if you
prefer a fully generic control) in the RegionSelectProps declaration to remove
the no-any violation; look for the Control usage in the RegionSelectProps type
definition and replace Control<FieldValues, any> with Control<FieldValues>.
| type Props = { | ||
| children?: ReactNode; | ||
| control: Control<SdlBuilderFormValuesType | RentGpusFormValuesType, any>; | ||
| control: Control<SdlBuilderFormValuesType, any>; | ||
| currentService: ServiceType; |
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.
🧩 Analysis chain
🏁 Script executed:
cd /repo && git ls-files | head -20Repository: akash-network/console
Length of output: 120
🏁 Script executed:
# First, let me check if the repo exists and the file location
find . -name "ImageSelect.tsx" -type fRepository: akash-network/console
Length of output: 117
🏁 Script executed:
# Check the content of the file and understand the Control type
cat -n apps/deploy-web/src/components/sdl/ImageSelect.tsx | head -50Repository: akash-network/console
Length of output: 2868
🏁 Script executed:
# Search for imports to understand where Control comes from
rg "import.*Control" apps/deploy-web/src/components/sdl/ImageSelect.tsxRepository: akash-network/console
Length of output: 158
🏁 Script executed:
rg "Control<" apps/deploy-web/src --type ts --type tsx -A 1 -B 1Repository: akash-network/console
Length of output: 92
🏁 Script executed:
# Also check the react-hook-form documentation or type definitions in node_modules if available
find . -path "*/node_modules/react-hook-form/*" -name "*.d.ts" | head -5Repository: akash-network/console
Length of output: 47
🏁 Script executed:
rg "Control<" apps/deploy-web/src -A 1 -B 1Repository: akash-network/console
Length of output: 12993
🏁 Script executed:
# Search for all Control type definitions and usages
rg "control:" apps/deploy-web/src -A 0 -B 0 | head -20Repository: akash-network/console
Length of output: 2243
🏁 Script executed:
# Let's check the react-hook-form package to understand the Control type signature
# First check if it's in package.json
cat package.json | grep -A 2 "react-hook-form"Repository: akash-network/console
Length of output: 47
🏁 Script executed:
cat -n apps/deploy-web/src/components/sdl/ImageSelect.tsx | tail -100Repository: akash-network/console
Length of output: 4312
🏁 Script executed:
# Also check the react-hook-form Control type documentation
web_search
react-hook-form Control generic parameters documentationRepository: akash-network/console
Length of output: 169
🌐 Web query:
react-hook-form Control type generic parameters second parameter context
💡 Result:
In react-hook-form, Control is generic over:
Control<TFieldValues extends FieldValues = FieldValues, TContext = any>TFieldValues: your form’s values shape.TContext: the type of the “form context” object you optionally pass touseForm({ context }), which is then forwarded to yourresolveras itscontextparameter. This is why you’ll see it wired throughUseFormProps(context: TContext,resolver: Resolver<TFieldValues, TContext>) andUseFormReturn(control: Control<TFieldValues, TContext>). [1]
Example:
type FormValues = { email: string };
type MyContext = { locale: "en" | "es" };
const form = useForm<FormValues, MyContext>({
context: { locale: "en" },
resolver: async (values, context) => {
// context is MyContext here
return { values, errors: {} };
},
});
// control is Control<FormValues, MyContext>
const { control } = form;If you’re not using useForm({ context }) / resolver context, you can usually ignore the 2nd generic and just use Control<FormValues>. [1]
Sources:
[1] React Hook Form – TypeScript Support (shows control: Control<TFieldValues, TContext> and UseFormProps with context / resolver typed by TContext) (react-hook-form.com)
Citations:
🏁 Script executed:
# Find where the control is created - search for useForm calls in the codebase
rg "useForm" apps/deploy-web/src --type js -B 3 -A 10 | grep -A 10 "resolver\|context" | head -50Repository: akash-network/console
Length of output: 47
🏁 Script executed:
# Check the specific component hierarchy - trace back where control comes from
# Look for the main form setup
rg "useForm<SdlBuilderFormValuesType" apps/deploy-web/src -A 15Repository: akash-network/console
Length of output: 16298
🏁 Script executed:
# Count how many files omit vs use the second parameter
echo "Files using Control with second parameter:"
rg "Control<[^>]+, " apps/deploy-web/src | wc -l
echo ""
echo "Files using Control without second parameter:"
rg "Control<[^>]+>(?!.*,)" apps/deploy-web/src | wc -lRepository: akash-network/console
Length of output: 415
🏁 Script executed:
# Check TokenFormControl to see the pattern
cat -n apps/deploy-web/src/components/sdl/TokenFormControl.tsxRepository: akash-network/console
Length of output: 2324
Remove the any type parameter from Control.
Line 23 uses Control<SdlBuilderFormValuesType, any>, which violates the TypeScript guideline against using any. The second parameter represents the resolver context type, which is unused in this component since no context is passed to useForm. Follow the pattern in TokenFormControl.tsx and omit the second parameter entirely.
✅ Suggested fix
- control: Control<SdlBuilderFormValuesType, any>;
+ control: Control<SdlBuilderFormValuesType>;As per coding guidelines: **/*.{ts,tsx,js}: Never use type any or cast to type any. Always define the proper TypeScript types.
🤖 Prompt for AI Agents
In `@apps/deploy-web/src/components/sdl/ImageSelect.tsx` around lines 21 - 24, In
Props in ImageSelect.tsx, remove the unnecessary any type parameter on Control:
change control: Control<SdlBuilderFormValuesType, any> to use the single generic
Control<SdlBuilderFormValuesType> (follow the TokenFormControl.tsx pattern) so
the resolver context generic is omitted; update the Props type declaration
accordingly to eliminate the use of any and keep proper typing for control.
| type RegionSelectProps = { | ||
| control: Control<RentGpusFormValuesType, any>; | ||
| control: Control<FieldValues, any>; | ||
| className?: string; |
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.
🧩 Analysis chain
🏁 Script executed:
cat -n apps/deploy-web/src/components/sdl/RegionSelect.tsx | head -30Repository: akash-network/console
Length of output: 1380
🏁 Script executed:
rg "type RegionSelectProps" -A 5 apps/deploy-web/src/components/sdl/RegionSelect.tsxRepository: akash-network/console
Length of output: 257
🏁 Script executed:
rg "import.*Control" apps/deploy-web/src/components/sdl/RegionSelect.tsxRepository: akash-network/console
Length of output: 171
🏁 Script executed:
rg "Control<" apps/deploy-web/src/components/sdl/ -A 2 -B 2Repository: akash-network/console
Length of output: 11183
🏁 Script executed:
cat -n apps/deploy-web/src/components/sdl/RegionSelect.tsxRepository: akash-network/console
Length of output: 4152
🏁 Script executed:
rg "useFormContext|context" apps/deploy-web/src/components/sdl/RegionSelect.tsxRepository: akash-network/console
Length of output: 47
🏁 Script executed:
rg "control\." apps/deploy-web/src/components/sdl/RegionSelect.tsxRepository: akash-network/console
Length of output: 47
🏁 Script executed:
# Check if there are any existing patterns with unknown for Control context
rg "Control<.*unknown" apps/deploy-web/Repository: akash-network/console
Length of output: 47
🏁 Script executed:
# Verify Control type signature in react-hook-form types
rg "export.*Control" node_modules/react-hook-form/dist/types/form.d.ts 2>/dev/null | head -5Repository: akash-network/console
Length of output: 47
Omit unnecessary context generic from Control type.
Line 16 uses Control<FieldValues, any>, which violates the no-any guideline. Since the component doesn't use form context, omit the second generic parameter entirely.
✅ Suggested fix
- control: Control<FieldValues, any>;
+ control: Control<FieldValues>;📝 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.
| type RegionSelectProps = { | |
| control: Control<RentGpusFormValuesType, any>; | |
| control: Control<FieldValues, any>; | |
| className?: string; | |
| type RegionSelectProps = { | |
| control: Control<FieldValues>; | |
| className?: string; |
🤖 Prompt for AI Agents
In `@apps/deploy-web/src/components/sdl/RegionSelect.tsx` around lines 15 - 17,
The prop type RegionSelectProps currently uses Control<FieldValues, any> which
introduces an unwanted any; update the type to omit the second generic so it
reads Control<FieldValues> (or just Control if you prefer a fully generic
control) in the RegionSelectProps declaration to remove the no-any violation;
look for the Control usage in the RegionSelectProps type definition and replace
Control<FieldValues, any> with Control<FieldValues>.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.