Skip to content

Commit bbd8b8f

Browse files
bryphe-coderbryphe-coder
authored
refactor: Refactor FormTextField to not require a HoC (#70)
Prompted from discussion here: https://github.com/coder/coder/pull/60/files#r792124373 Our current FormTextField implementation requires a [higher-order component](https://reactjs.org/docs/higher-order-components.html), which can be complicated to understand. This experiments with moving it to not require being a HoC. The only difference in usage is that sometimes, you need to provide the type like `<FormTextField<FormValues> form={form} formFieldName="some-field-in-form" />` - but it doesn't require special construction.
1 parent 9cf4f7c commit bbd8b8f

File tree

3 files changed

+56
-66
lines changed

3 files changed

+56
-66
lines changed

‎site/components/Form/FormTextField.test.tsx

+2-4
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { act, fireEvent, render, screen } from "@testing-library/react"
22
import { useFormik } from "formik"
33
import React from "react"
44
import * as yup from "yup"
5-
import { formTextFieldFactory, FormTextFieldProps } from "./FormTextField"
5+
import { FormTextField, FormTextFieldProps } from "./FormTextField"
66

77
namespace Helpers {
88
export interface FormValues {
@@ -11,8 +11,6 @@ namespace Helpers {
1111

1212
export const requiredValidationMsg = "required"
1313

14-
const FormTextField = formTextFieldFactory<FormValues>()
15-
1614
export const Component: React.FC<Omit<FormTextFieldProps<FormValues>, "form" | "formFieldName">> = (props) => {
1715
const form = useFormik<FormValues>({
1816
initialValues: {
@@ -26,7 +24,7 @@ namespace Helpers {
2624
}),
2725
})
2826

29-
return <FormTextField {...props} form={form} formFieldName="name" />
27+
return <FormTextField<FormValues> {...props} form={form} formFieldName="name" />
3028
}
3129
}
3230

‎site/components/Form/FormTextField.tsx

+53-59
Original file line numberDiff line numberDiff line change
@@ -104,67 +104,61 @@ export interface FormTextFieldProps<T>
104104
* )
105105
* }
106106
*/
107-
export const formTextFieldFactory = <T,>(): React.FC<FormTextFieldProps<T>> => {
108-
const component: React.FC<FormTextFieldProps<T>> = ({
109-
children,
110-
disabled,
111-
displayValueOverride,
112-
eventTransform,
113-
form,
114-
formFieldName,
115-
helperText,
116-
isPassword = false,
117-
InputProps,
118-
onChange,
119-
type,
120-
variant = "outlined",
121-
...rest
122-
}) => {
123-
const isError = form.touched[formFieldName] && Boolean(form.errors[formFieldName])
107+
export const FormTextField = <T,>({
108+
children,
109+
disabled,
110+
displayValueOverride,
111+
eventTransform,
112+
form,
113+
formFieldName,
114+
helperText,
115+
isPassword = false,
116+
InputProps,
117+
onChange,
118+
type,
119+
variant = "outlined",
120+
...rest
121+
}: FormTextFieldProps<T>): React.ReactElement => {
122+
const isError = form.touched[formFieldName] && Boolean(form.errors[formFieldName])
124123

125-
// Conversion to a string primitive is necessary as formFieldName is an in
126-
// indexable type such as a string, number or enum.
127-
const fieldId = String(formFieldName)
124+
// Conversion to a string primitive is necessary as formFieldName is an in
125+
// indexable type such as a string, number or enum.
126+
const fieldId = String(formFieldName)
128127

129-
const Component = isPassword ? PasswordField : TextField
130-
const inputType = isPassword ? undefined : type
128+
const Component = isPassword ? PasswordField : TextField
129+
const inputType = isPassword ? undefined : type
131130

132-
return (
133-
<Component
134-
{...rest}
135-
variant={variant}
136-
disabled={disabled || form.isSubmitting}
137-
error={isError}
138-
helperText={isError ? form.errors[formFieldName] : helperText}
139-
id={fieldId}
140-
InputProps={isPassword ? undefined : InputProps}
141-
name={fieldId}
142-
onBlur={form.handleBlur}
143-
onChange={(e) => {
144-
if (typeof onChange !== "undefined") {
145-
onChange(e)
146-
}
131+
return (
132+
<Component
133+
{...rest}
134+
variant={variant}
135+
disabled={disabled || form.isSubmitting}
136+
error={isError}
137+
helperText={isError ? form.errors[formFieldName] : helperText}
138+
id={fieldId}
139+
InputProps={isPassword ? undefined : InputProps}
140+
name={fieldId}
141+
onBlur={form.handleBlur}
142+
onChange={(e) => {
143+
if (typeof onChange !== "undefined") {
144+
onChange(e)
145+
}
147146

148-
const event = e
149-
if (typeof eventTransform !== "undefined") {
150-
// TODO(Grey): Asserting the type as a string here is not quite
151-
// right in that when an input is of type="number", the value will
152-
// be a number. Type asserting is better than conversion for this
153-
// reason, but perhaps there's a better way to do this without any
154-
// assertions.
155-
event.target.value = eventTransform(e.target.value) as string
156-
}
157-
form.handleChange(event)
158-
}}
159-
type={inputType}
160-
value={displayValueOverride || form.values[formFieldName]}
161-
>
162-
{children}
163-
</Component>
164-
)
165-
}
166-
167-
// Required when using an anonymous factory function
168-
component.displayName = "FormTextField"
169-
return component
147+
const event = e
148+
if (typeof eventTransform !== "undefined") {
149+
// TODO(Grey): Asserting the type as a string here is not quite
150+
// right in that when an input is of type="number", the value will
151+
// be a number. Type asserting is better than conversion for this
152+
// reason, but perhaps there's a better way to do this without any
153+
// assertions.
154+
event.target.value = eventTransform(e.target.value) as string
155+
}
156+
form.handleChange(event)
157+
}}
158+
type={inputType}
159+
value={displayValueOverride || form.values[formFieldName]}
160+
>
161+
{children}
162+
</Component>
163+
)
170164
}

‎site/components/SignIn/SignInForm.tsx

+1-3
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { useSWRConfig } from "swr"
66
import * as Yup from "yup"
77

88
import { Welcome } from "./Welcome"
9-
import { formTextFieldFactory } from "../Form"
9+
import { FormTextField } from "../Form"
1010
import * as API from "./../../api"
1111
import { LoadingButton } from "./../Button"
1212

@@ -25,8 +25,6 @@ const validationSchema = Yup.object({
2525
password: Yup.string(),
2626
})
2727

28-
const FormTextField = formTextFieldFactory<BuiltInAuthFormValues>()
29-
3028
const useStyles = makeStyles((theme) => ({
3129
loginBtnWrapper: {
3230
marginTop: theme.spacing(6),

0 commit comments

Comments
 (0)