Skip to content
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

Resolve React Hook useEffect warning #92

Open
Xtrah opened this issue Jun 2, 2022 · 1 comment
Open

Resolve React Hook useEffect warning #92

Xtrah opened this issue Jun 2, 2022 · 1 comment
Labels
frontend ✨ Frontend tasks

Comments

@Xtrah
Copy link
Member

Xtrah commented Jun 2, 2022

Several places in the code has an ESLint warning react-hooks/exhaustive-deps that should be resolved. Run npm run check in the project root folder to find each instance.

warning React Hook useEffect has missing dependencies...

@Xtrah Xtrah added the frontend ✨ Frontend tasks label Jun 2, 2022
@eivindkopperud
Copy link
Member

I've had a look at this.

An example on how to fix it

For example, in ApplicationForm.tsx we have the following code:

	useEffect(() => {
	  if (form.values.text.length) {
		  form.validateField('text')
	  }
	  if (form.values.main_board_text.length) {
		  form.validateField('main_board_text')
	  }
	}, [form.values.text, form.values.main_board_text])

which gives a such warning. This can be fixed by changing it to:

	useEffect(() => {
		const validateForm = () => {
			if (form.values.text.length) {
				form.validateField('text')
			}
			if (form.values.main_board_text.length) {
				form.validateField('main_board_text')
			}
		}
		validateForm()
	}, [form.values.text, form.values.main_board_text])

See this stack overflow for reference

Another example on how to fix it

Use // eslint-disable-next-line react-hooks/exhaustive-deps to ignore the warning

Discussion

I don't really know if it has a practical consequenses having it as we have it? The first option seems more cluttered. On the other hand, it would be nice to summarize what the useEffect actually does, as they currently have a lot of code in them. Thoughts? I think the first option might be the way to go, if we're able to give meaningful names to the functions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
frontend ✨ Frontend tasks
Projects
None yet
Development

No branches or pull requests

2 participants