eslint setup and clean up #135
Conversation
✅ Deploy Preview for remix-forms canceled.
|
| @@ -339,8 +338,7 @@ function createField<Schema extends SomeZodObject>({ | |||
| radioWrapperComponent: RadioWrapper, | |||
| labelComponent: Label, | |||
| }), | |||
| // eslint-disable-next-line react-hooks/exhaustive-deps | |||
| [Input, Multiline, Select, Checkbox, Radio, RadioWrapper, Label], | |||
| [], | |||
There was a problem hiding this comment.
These variables are declared in an external scope so they never rerender the component, they are not needed here and eslint caught that.
| @@ -257,6 +256,7 @@ function createForm({ | |||
| radioWrapperComponent, | |||
| fieldErrorsComponent, | |||
| Error, | |||
| form.register, | |||
There was a problem hiding this comment.
This dependency was missing, this might result in a runtime change.
In my tests it didn't change the render count of forms with <Field component.
There was a problem hiding this comment.
I don't care too much for this linter rule but since it can help us to not miss important things adding form.register as a dependency is a small price to pay.
| @@ -422,8 +422,7 @@ function createForm({ | |||
| : transition.state === 'submitting' | |||
|
|
|||
| setDisabled(shouldDisable) | |||
| // eslint-disable-next-line react-hooks/exhaustive-deps | |||
| }, [transition.state, formState]) | |||
| }, [transition.state, formState, mode, isValid]) | |||
There was a problem hiding this comment.
I think this addition makes sense as one value is a string union and the other is a boolean, if they change this code should probably re-run... any ideas?
I realized the eslint config was faulty, there were dependencies missing thus
npm run lintwas failing.When I fixed it I also added the
@typescript-eslint/consistent-type-importsrule so it will enforce theimport typewhen it is due.It turns out some
eslint-disablecomments were unnecessary, others had to be added and few other issues that came up.I'll leave comments where actual code changes happened.