3
\$\begingroup\$

I've created a component that validates an email address.

It works, but since I'm new to hooks I would like some validation of what I've done before I let what I've done loose on the rest of the components I'm creating.

In particular I'm concerned I'm not managing the state correctly, it seems verbose, and I've got a lot of variables I am tracking.

If anyone has any pointers on the Fade Component as well I'd appreciate them.

I still need to handle errors back from useQuery too, I realise.

And of course, I don't know what I don't know, so feel free to point me in the right direction.

Thank you!

import React, { useState } from 'react'
import { InputAdornment, Typography } from '@material-ui/core'
import Fade from '@material-ui/core/Fade'
import CircularProgress from '@material-ui/core/CircularProgress'
import { MdCheckCircle } from 'react-icons/md'
import { ErrorMessage } from 'formik'
import TextField from 'Components/TextField'
import { string } from 'yup'
import { useAsyncFn } from 'react-use'
import { useApolloClient } from '@apollo/react-hooks'
import { FIND_BY_IDENTIFIER } from 'Queries/FindByIdentifier.query'
import { not } from 'ramda'

const schema = string()
  .ensure()
  .required('Oops, email is required')
  .email('Oops, check your email format')

const LoginInstead = () =>  { return (<div>Would you like to login instead?</div>) }

function FormEmailAsync ({
  name,
  email,
  handleChange,
  label,
  setFieldError,
  setFieldTouched,
  onBlurred,
  ...rest
}) {
 const [emailValid, setValidEmail] = useState('')
 const client = useApolloClient()
 const [loading, setLoading] = useState(false)
 const [error, setError] = useState(false)
 const [emailPresent, setEmailPresent] = useState(false)

  //eslint-disable-next-line no-unused-vars
  const [theState, asyncFunc] = useAsyncFn(async value => {
    const returnedSchema = await schema.validate(value)
    if (returnedSchema !== value) { return returnedSchema }
    setLoading(true)
    const { data } = await client.query({
      query: FIND_BY_IDENTIFIER,
      variables: { identifier: value },
      context: { uri: process.env.REACT_APP_GRAPHQL_PUBLIC_API }
    })
    setValidEmail(not(data.findByIdentifier.present))
    setEmailPresent(data.findByIdentifier.present)    
    setLoading(false)
    return value
  }, [])

  const onBlurFunc = async (name, email) => {
    setFieldTouched(name)
    let returnFromAsync = await asyncFunc(email)    
    if (returnFromAsync === email) {
      onBlurred(returnFromAsync)
    } else {
      setError(true)
      setFieldError(name, returnFromAsync.message)
    }
  }

  const onChangeFunc = (evt) => {
    setValidEmail(null)
    setEmailPresent(false)    
    setError(false)
    handleChange(evt)    
  }
  return (
    <div>
      <TextField
        error={error}
        name={name}
        variant='outlined'
        value={email}
        placeholder={'email@example.com'}
        onChange={evt => onChangeFunc(evt)}
        label={label}
        onBlur={event => onBlurFunc(name, event.target.value)}
        InputProps={{
          endAdornment: (
            <InputAdornment position='end'>
              <Fade
                in={loading}
                style={{
                  transitionDelay: loading ? '800ms' : '0ms'
                }}
                unmountOnExit
              >
                <CircularProgress size={16} />
              </Fade>
             { emailValid === true && 
              <Fade
                in={emailValid}
              >
                <MdCheckCircle
                  color='green'
                  data-testid={'success'}
                  size={16}
                />
             </Fade> }
            </InputAdornment>
          )
        }}
        {...rest}
      />
      { emailPresent ? LoginInstead() :
      <ErrorMessage
        name='email'
        render={msg => <Typography color='error'>{msg}</Typography> }
      />}
    </div>
  )
}

export default FormEmailAsync
```
\$\endgroup\$
2
\$\begingroup\$

While hooks are amazing, there is no point in using them when your function component becomes overly complex. Better to just create a class component.

Some reasons why switching to a class component is better:

  1. Defining functions within your functions will cause those variables to be redefined after each re-render which is a waste of efficiency and performance
  2. Destructuring too many variables from your useState makes it exceedingly hard to maintain in the future and hard to read

The general rule of thumb is that class components are "smart components" and function components are "dummy components". Reason why is that class components have a state and handle logic while "dummy components" are simply there to create views.

I would recommend creating a class component handling your validation, and simplifying your render method by creating "dummy components" with simple hook usage (so that they stay dumb).

\$\endgroup\$
  • \$\begingroup\$ Thanks for your review. I've spent quite some time on this hooks implementation so understandably I'm not keen to move to a class implementation. I do see some value though in creating more components to handle the "dumb" aspects. Perhaps a class might drop out of that. \$\endgroup\$ – Justin Lane Sep 11 '19 at 1:49
1
\$\begingroup\$

I feel your component mixes a lot of concerns: API requests and responses, form field validations and error display, and presentational concerns such as styling and animations.

In my opinion, a React component works best if it concerns itself either:

  • with one concern only (API or validation or presentation)
  • with the coordination of multiple components

I think your component (and by extension, your codebase) could profit from splitting this component into multiple components or hooks that each address one of the issues. You already apply hooks very well and I don't see a problem with that – no need to switch back to class components. Hooks can even help you better with extracting logic that is not bound to a specific level in the component hierarchy, such as repeating tasks like API requests and validations.

Since you're thinking about "unleashing" this on the rest of your codebase, this is a perfect moment to think about which aspects of the component you want to reuse and how you want to use these tools, informing the design of your components.

\$\endgroup\$
  • \$\begingroup\$ Yes, I think extracting some of the components will "dry" things up and make it easier to understand what is going on. I shall try extracting the logic as you suggest into separate components and see how this looks. Thank you. :) \$\endgroup\$ – Justin Lane Sep 11 '19 at 1:54

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.